-
Notifications
You must be signed in to change notification settings - Fork 0
ci: add devenv test to code checks #16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRenames and updates CI workflows, adds Nix/Cachix-based devenv install and a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/codeChecks.yml (1)
44-52: Nice addition: devenv integration into CI.The Nix and devenv setup steps are correctly configured. The workflow will now validate the devenv environment and run pre-commit hooks.
Optional: Consider adding Nix store caching and a timeout
To improve CI performance and reliability:
- Add Nix store caching to speed up subsequent runs:
- uses: cachix/install-nix-action@v31 + with: + extra_nix_config: | + access-tokens = github.com=${{ secrets.GITHUB_TOKEN }} - uses: cachix/cachix-action@v16 with: name: devenv
- Add a timeout to prevent indefinite hangs:
- name: Build the devenv shell and run any pre-commit hooks run: devenv test + timeout-minutes: 15
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/codeChecks.yml(2 hunks)devenv.nix(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 5
File: devenv.nix:569-569
Timestamp: 2025-11-15T17:16:55.075Z
Learning: In the xenOs76/https-wrench repository, the devenv.nix test environment is local-only without GitHub CI integration, so automatic dependency updates in enterTest are acceptable for the maintainer's workflow.
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 10
File: internal/requests/requests_test.go:918-997
Timestamp: 2025-11-29T14:05:35.000Z
Learning: In the https-wrench tool test suite (xenOs76/https-wrench repository), the user wants to be reminded in the future about potentially adding MinVersion enforcement to TLS configurations in test code, once legacy server compatibility becomes less critical.
📚 Learning: 2025-11-15T17:16:55.075Z
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 5
File: devenv.nix:569-569
Timestamp: 2025-11-15T17:16:55.075Z
Learning: In the xenOs76/https-wrench repository, the devenv.nix test environment is local-only without GitHub CI integration, so automatic dependency updates in enterTest are acceptable for the maintainer's workflow.
Applied to files:
.github/workflows/codeChecks.ymldevenv.nix
🔇 Additional comments (4)
.github/workflows/codeChecks.yml (2)
17-17: LGTM: Simplified to Go 1.25 only.The matrix now tests exclusively with Go 1.25, streamlining the CI workflow.
44-45: Bothcachix/install-nix-action@v31andcachix/cachix-action@v16are already at current stable versions. Major tags like v31 point to the latest minor/patch release in line with how most GitHub actions manage releases, and v31.4.0 is the latest release. v16 is the latest release for cachix-action.devenv.nix (2)
151-157: LGTM: Clean cert refresh before nginx starts.The task correctly removes stale test artifacts and regenerates TLS certificates before nginx processes start. The conditional directory check (
test -d ... &&) prevents errors when the directory doesn't exist.
626-626: LGTM: Avoids duplicate Go test execution.Commenting out
run-go-testsin theenterTestscript is appropriate since the GitHub Actions workflow already runsgo test -v ./...(line 35 of codeChecks.yml). TheenterTestscript now focuses on integration and CLI tests without redundancy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
21-23: Simplify single-version matrix to direct version string.The matrix strategy with a single Go version
['1.25']adds unnecessary complexity. Consider using a direct version string instead.🔎 Apply this diff to simplify the configuration:
runs-on: ubuntu-latest - strategy: - matrix: - go-version: ['1.25'] -- name: Set up Go uses: actions/setup-go@v6 with: - go-version: ${{ matrix.go-version }} + go-version: '1.25'Also applies to: 51-51
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/codeChecks.yml(2 hunks).github/workflows/release.yml(2 hunks)devenv.nix(3 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 5
File: devenv.nix:569-569
Timestamp: 2025-11-15T17:16:55.075Z
Learning: In the xenOs76/https-wrench repository, the devenv.nix test environment is local-only without GitHub CI integration, so automatic dependency updates in enterTest are acceptable for the maintainer's workflow.
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 10
File: internal/requests/requests_test.go:918-997
Timestamp: 2025-11-29T14:05:35.000Z
Learning: In the https-wrench tool test suite (xenOs76/https-wrench repository), the user wants to be reminded in the future about potentially adding MinVersion enforcement to TLS configurations in test code, once legacy server compatibility becomes less critical.
📚 Learning: 2025-11-15T17:16:55.075Z
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 5
File: devenv.nix:569-569
Timestamp: 2025-11-15T17:16:55.075Z
Learning: In the xenOs76/https-wrench repository, the devenv.nix test environment is local-only without GitHub CI integration, so automatic dependency updates in enterTest are acceptable for the maintainer's workflow.
Applied to files:
devenv.nix
🪛 actionlint (1.7.9)
.github/workflows/release.yml
39-39: the runner of "docker/setup-qemu-action@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
.github/workflows/codeChecks.yml (2)
6-7: LGTM! Path triggers correctly include workflow and devenv files.Adding the workflow file itself and
devenv.*patterns to the path triggers ensures CI runs when the test environment or workflow configuration changes.
59-61: LGTM! devenv test integration looks good.The
devenv teststep with a 15-minute timeout appropriately integrates the Nix-based test environment into CI, aligning with the PR objective.devenv.nix (1)
526-526: Verify if disabling k3s test is intentional.The
test-requests-k3shas been commented out. If this is because k3s infrastructure is unavailable in the CI environment, consider adding a comment explaining why it's disabled.Is k3s infrastructure available in the CI environment? If not, consider adding a comment:
- #test-requests-k3s + # k3s test disabled - requires k3s cluster not available in CI.github/workflows/release.yml (1)
34-36: The Nix installation step is necessary for this workflow. GoReleaser's configuration explicitly includes a Nix build step (see.goreleaser.yamllines 167 and 174 which referencenix:andpkgs/https-wrench/default.nix). Thecachix/install-nix-actionprovides the Nix environment required for GoReleaser to execute this build configuration.Likely an incorrect or invalid review comment.
…ct token permissions on code_checks job
Summary by CodeRabbit
Infrastructure
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.