feat(nix): add integration tests, refactor file structure #135
Conversation
This leaves the required toplevel files alone, and moves all other Nix files into a dedicated `nix/` directory. This facilitates adding more Nix code, such as tests, to the directory without polluting the toplevel with too many files, and also consolidates all flake-compat usage.
WalkthroughRefactors flake composition to use flake-parts, moves many Nix files under Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer workspace
participant Flake as flake.nix (mkFlake)
participant Parts as flake-parts
participant Nixpkgs as inputs.nixpkgs
participant Tests as nix/tests/default.nix
rect rgba(0,128,128,0.06)
Dev->>Flake: edit flake.nix (use flake-parts, add perSystem)
Flake->>Parts: call lib.mkFlake(perSystem -> packages, devShells, imports)
Parts->>Nixpkgs: resolve system-specific packages (via inputs)
end
rect rgba(128,0,128,0.06)
Dev->>Tests: add tests/*.test.nix
Tests->>Tests: findTests("./.") => map *.test.nix -> mkTest(test)
Tests->>Parts: tests can be wired into perSystem checks via flake-module.nix
end
sequenceDiagram
participant shell.nix as shell.nix
participant compat as nix/flake-compat.nix
participant lock as flake.lock
shell.nix->>compat: import ./nix/flake-compat.nix
compat->>lock: read lock metadata (rev, narHash)
compat->>GitHub: fetchTarball using lock metadata
compat->>shell.nix: expose .shellNix from fetched flake-compat outputs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
nix/flake-compat.nix (1)
1-11: flake-compat pinning via flake.lock looks good; optional readability tweakThis matches the usual
flake.lock-driven flake-compat pattern and should work fine as-is. If you want to DRY the long attr paths a bit, you could factor out the locked node:-import -( - let - lock = builtins.fromJSON (builtins.readFile ../flake.lock); - in - fetchTarball { - url = "https://github.com/edolstra/flake-compat/archive/${lock.nodes.flake-compat.locked.rev}.tar.gz"; - sha256 = lock.nodes.flake-compat.locked.narHash; - } -) -{src = ../.;} +import +( + let + lock = builtins.fromJSON (builtins.readFile ../flake.lock); + compat = lock.nodes.flake-compat.locked; + in + fetchTarball { + url = "https://github.com/edolstra/flake-compat/archive/${compat.rev}.tar.gz"; + sha256 = compat.narHash; + } +) +{ src = ../.; }Purely optional; current code is correct.
nix/tests/example.nix (1)
1-15: Example NixOS test is wired correctly to the flake moduleThe test shape (
{ pkgs, self, }: pkgs.testers.runNixOSTest { ... }) matches the newmkTestcontract, importsself.nixosModules.nixos-clias expected, and exercises thenixosCLI in a VM, which is appropriate for an example test. You can later extendtestScriptwith concrete output checks if you want stronger assertions.nix/tests/default.nix (1)
1-32: Test discovery logic is solid; consider keeping values as pathsThe
findTestspipeline correctly:
- walks
./.recursively,- filters
*.test.nix,- normalizes names by dropping
.test.nixand replacing/with-,- and
mkTest’s{ inherit pkgs self; }contract matches the new test files.One small improvement: you don't need to stringify the file paths for the values. Keeping them as paths makes the intent clearer and avoids relying on
importof a string path:entries = lib.genAttrs' (lib.filesystem.listFilesRecursive dir) (name: lib.nameValuePair (builtins.toString name) name);The rest of the pipeline can stay the same, since only the attribute names depend on string operations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
README.md(2 hunks)default.nix(1 hunks)flake-compat.nix(0 hunks)flake.nix(1 hunks)nix/flake-compat.nix(1 hunks)nix/package.nix(1 hunks)nix/tests/default.nix(1 hunks)nix/tests/example.nix(1 hunks)nix/tests/example.test.nix(1 hunks)nix/tests/flake-module.nix(1 hunks)shell.nix(1 hunks)
💤 Files with no reviewable changes (1)
- flake-compat.nix
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~64-~64: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...on the filename. Since these tests are quite expensive to run, passing them is not an automati...
(EN_WEAK_ADJECTIVE)
⏰ 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)
- GitHub Check: Build/Test
🔇 Additional comments (7)
nix/package.nix (1)
14-25: fileset-basedsrclooks correct; be aware of maintenance overheadThe
lib.fileset.toSourcesetup cleanly restricts the Go build context to exactly what you need (go.mod,cmd/,internal/, docs, etc.), which is a nice replacement fornix-gitignore. The only caveat is that adding new top-level Go directories in future will require updating this list, so just something to keep in mind.README.md (1)
110-112: Updated package.nix path is consistent with the new nix/ layoutPointing the versioning docs at
[package.nix](./nix/package.nix)matches the refactor that moved Nix files under./nix/. This keeps the documentation aligned with the new file structure.flake.nix (1)
12-71: flake-parts migration looks solid; just confirmnix-options-docsupports allsystemsThe move to
flake-parts.lib.mkFlakewithsystems = lib.systems.flakeExposed;, per-systempackages/devShells, andflake.nixosModules.nixos-cliwired vialib.modules.importApplyall look consistent and idiomatic.One thing worth double‑checking:
devShells.defaultunconditionally doesnix-options-doc = inputs.nix-options-doc.packages.${system}.default;for every
systeminlib.systems.flakeExposed. Ifnix-options-docdoesn’t publish apackages.${system}.defaultfor some of those systems,nix develop(or flake evaluation) will fail there. If that’s a concern, you could either narrowsystemsor guard this withlib.optionalAttrs/ similar.nix/tests/example.test.nix (1)
1-16: Example NixOS test is well-structured and matches the described test harnessThe test function signature, use of
pkgs.testers.runNixOSTest, importingself.nixosModules.nixos-cli, and the simplemachine1.succeed("nixos features")smoke check all line up nicely with the new test scaffolding described in the README and flake module. Looks like a good minimal example for further tests to copy.nix/tests/flake-module.nix (1)
1-5: Test flake module wiring looks correct
perSystem.checkscorrectly delegates to./.with{ inherit pkgs self; }, which matches the new tests/default.nix interface and keeps the flake surface minimal.default.nix (1)
2-14: Path refactor preserves default.nix behaviorSwitching to
./nix/flake-compat.nixforflakeSelfand./nix/module.nixformodulekeeps the default.nix interface intact while matching the new directory layout; theself/useFlakePkgwiring is unchanged.shell.nix (1)
1-1: Delegating to nix/flake-compat.nix simplifies shell.nixPointing
shell.nixdirectly at(import ./nix/flake-compat.nix).shellNixnicely centralizes flake-compat logic without changing the external entrypoint.
This uses a fileset to restrict the amount of rebuilds that one has to perform. For example, if a test is changed, we do not want to rebuild the `nixos-cli` package, since that's wasteful. Using an explicit opt-in fileset for src works here because there are only a few files/directories required to compile the actual package, and they can be specified manually without much trouble.
water-sucks
force-pushed
the
refactor-nix-structure
branch
from
304901e to
a72666e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(2 hunks)flake.nix(1 hunks)nix/package.nix(1 hunks)nix/tests/default.nix(1 hunks)nix/tests/example.nix(1 hunks)nix/tests/example.test.nix(1 hunks)nix/tests/flake-module.nix(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- flake.nix
🚧 Files skipped from review as they are similar to previous changes (3)
- nix/tests/example.test.nix
- nix/tests/flake-module.nix
- nix/tests/default.nix
🧰 Additional context used
🪛 LanguageTool
README.md
[style] ~64-~64: As an alternative to the over-used intensifier ‘quite’, consider replacing this phrase.
Context: ...on the filename. Since these tests are quite expensive to run, passing them is not an automati...
(EN_WEAK_ADJECTIVE)
⏰ 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)
- GitHub Check: Build/Test
🔇 Additional comments (4)
README.md (3)
44-45: LGTM! Helpful documentation for legacy workflows.The addition of legacy
nix-buildandnix-shelldocumentation is valuable for users who haven't migrated to flakes yet.
47-67: Excellent test infrastructure documentation.The Tests section provides clear, comprehensive guidance on:
- Test location and naming conventions
- How to run tests in both flake and non-flake contexts
- Expectations around test coverage and CI requirements
The documentation correctly references
pkgs.testers.runNixOSTestand usesnix-buildconsistently for the non-flake workflow, addressing the concerns from previous reviews.
111-111: Path update correctly reflects the new directory structure.The updated reference to
./nix/package.nixaligns with the refactored file organization.nix/tests/example.nix (1)
1-15: LGTM! Well-structured example test.This example test provides a clear template for future tests:
- Uses the correct
pkgs.testers.runNixOSTestAPI- Demonstrates proper node configuration with module imports
- Includes a basic sanity check that verifies the service enables and the CLI works
The minimal scope is appropriate for an example/scaffold test.
1 participant
Description
This adds a convenient scaffold for orchestrating NixOS tests in the
nix/tests/directory for testing various scenarios.Any file created there with the suffix
.test.nixis now added as a flake check. Each test MUST usepkgs.runNixOSTestto spin up VM(s) to perform various test cases, which will come in later pull requests.This also moves all Nix-related files into a dedicated
nix/directory to avoid polluting the toplevel with various Nix files.Summary by CodeRabbit
Documentation
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.