modules.nixosTest.setPackage: init

[roberth]

Description of changes

Add overriding methods to the NixOS test package attrset.

• Originally I wanted to define overriding as part of nixosTest matrix (prev also modularity, option docs) #176557, but it's never too late to make progress.
• Triggered by tests.trivial-builders.references: provide passthru attributes and override interface on non-Linux platforms #273183 (comment)

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
  • (Package updates) Added a release notes entry if the change is major or breaking
  • (Module updates) Added a release notes entry if the change is significant
  • (Module addition) Added a release notes entry if adding a new NixOS module
• Fits CONTRIBUTING.md.

---

Add a 👍 reaction to pull requests you find important.

[RaitoBezarius]

Thanks for the ping, I will review this when I have some time.

[roberth]

I've applied this to two tests and packages I maintain, which led me to develop the set-package.nix module for the test framework.
The new setPackage isn't as general as extendNixOS, but it is more guided and it takes care of the unsupported intersection of passthru tests, overriding and a non-Linux virtualization host, and does so in a mostly transparent way.

This has increased the diff size a fair bit, but most of the changes to the two packages are trivial.

The commit history also shows the non-set-package.nix solution which wouldn't support e.g. ghostunnel.tests.nixos on Darwin. That's not essential for review.

---

Tested happy flow with:

(set -x; for package in cassandra_4 ghostunnel; do for system in x86_64-linux aarch64-darwin; do nix-build -A $package.tests.nixos --argstr system $system; nix-build -A nixosTests.$package --argstr system $system; done; done)

---

Tested expected error with:

nix-repl> with import ./. { system = "aarch64-darwin"; }; (ghostunnel.overrideAttrs { name = "yolotunnel"; }).tests.nixos

<snip>

       … while evaluating the option `nodes.service.services.ghostunnel.package':

       … while evaluating definitions from `/home/user/h/nixpkgs/nixos/tests/ghostunnel.nix, via option setPackage.<function body>':

       error: It seems that you are trying to run a NixOS test which is part of a package that has been overridden.
       This can be done - just not through the package `tests` attribute, because this package only knows about its own overrides, not the ones that should be applied to the Linux variation of it.
       You can construct the correct test, e.g.:

           let
             # Let's assume you have package sets like this:
             pkgsForLinux = nixpkgs { system = ...; }; # e.g. aarch64-linux
             nativePkgs = nixpkgs { system = ...; }; # e.g. aarch64-darwin

             myOverrides = ...;

             # You just invoked something like myFoo.tests where
             #   myFoo = nativePkgs.foo.overrideAttrs myOverrides;
             # or it might have been:
             #   nativePkgs.nixosTests.foo.setPackage myFoo

             # Instead, create
             myFooForLinux = pkgsLinux.foo.overrideAttrs myOverrides;
           in
             # So that you can construct the correct test like this:
             nativePkgs.nixosTests.foo.setPackage myFooForLinux

       Or if you're defining the test file outside of Nixpkgs, it might look like this:

           (nativePkgs.testers.runNixOSTest ./foo-test.nix).setPackage myFooForLinux

[wolfgangwalther]

This could work well for the callPackageWith case, too.

[wolfgangwalther]

Not sure about the name.. but I needed "override existing option" already as well, when prototyping a module-based system argument for #402298 (comment). Good thing.

[wolfgangwalther]

Ah, this will probably also help with nixosTests.postgresql, which currently is a matrix of tests for each major version.

Maybe we can then just have nixosTests.postgresql be the tests for v17 (latest) and the other versions are tested via passthru only.

[wolfgangwalther]

This is 🚀

[wolfgangwalther]

Some back & forth between commits with formatting of ghostunnel here. Looking at the commit messages, I assume you don't want to squash this, so this should be cleaned up.

[wolfgangwalther]

setPackage is odd to me. I would expect it to be called overridePackage, that's what it does, right? Aka, without setPackage I still get a test with the default package?

Although the whole thing... looks a lot like there is a test function which needs a package argument, which has a default value. On the nixpkgs-side, that's just .override { package = finalAttrs.finalPackage; }. I see similarity to what .callPackage does, here. But callPackage does it for many packages, more generalized.

Can this be generalized, too?

[roberth]

The tests aren't callPackage-based anymore, to simplify two "dependency injectors" into just one, which is the module system with its module arguments.

setPackage "takes" that general functionality and narrows it down to a single use case in a way that supports running the tests on macOS VM hosts. And it tries to do so in a developer friendly way, with a workaround for a common error scenario and a descriptive error when the combination of features (overriding and cross-platform passthru tests) is not supported.

So the easy answer is it doesn't need to be generalized. Contributors can use module arguments or options directly.
OTOH yes, it could be generalized to support multiple packages at once.

[roberth]

I'm not so jsure that the way we run NixOS tests on VM hosts like darwin is the right way.
The unpleasant workaround to partially recover the interaction with overriding is proof of that.

Perhaps the nixosTests should not be available on non-NixOS platforms, and instead, macOS users should call a (TBD) attribute like (import ./. { system = "aarch64-linux"; }).nixosTests.acme.onVMHost.aarch64-darwin.
In other words, the test framework would be responsible for onVMHost.
By "composing" the various choices this way, we don't need to magically translate Darwin packages to Linux packages. All the overriding happens in the Linux world, and the alternate virtualizer is added last, as nature intended.

Should I remove the setPackage stuff for now? extendNixOS or a simpler setPackage would get the job done within the outlined alternate framework, at the cost of NixOS development on Darwin having a more different developer experience.
I do lean that way, because we can only stack so much complexity on top of unnecessary complexity.

[wolfgangwalther]

I don't claim to understand the details and implications of this, but I think one important property for nixpkgs-review is to be able to build the same nixosTests.<...> attributes on all 4 systems the same way. Otherwise, that part would become annoying, quickly. Also <pkg>.passthru.tests should work the same for all 4.

[roberth]

Right. At least setPackage tries its best for all that, but regardless of this PR, we'll have to accept that either

• pkg.tests.nixos is a bit of a lie and doesn't support (pkg.override* foo).tests.nixos at all on non-Linux platforms,
• or all packages need to only add NixOS tests in an // optionalAttrs hostPlatform.isLinux

The latter is more correct, manages expectations better, doesn't lead to subtle impossibilities, and keeps our "infra code" simpler, but it does burden the package authors and the nixosTests.<...> and pkg.tests users a bit.

[roberth]

I've split out the first part into

• NixOS test framework: add overriding methods #421480

[wolfgangwalther]

Right. At least setPackage tries its best for all that, but regardless of this PR, we'll have to accept that either

• pkg.tests.nixos is a bit of a lie and doesn't support (pkg.override* foo).tests.nixos at all on non-Linux platforms,

• or all packages need to only add NixOS tests in an // optionalAttrs hostPlatform.isLinux

The latter is more correct, manages expectations better, doesn't lead to subtle impossibilities, and keeps our "infra code" simpler, but it does burden the package authors and the nixosTests.<...> and pkg.tests users a bit.

OK, I'm starting to understand the problem space a bit more...

pkg.tests.nixos on darwin is not testing the darwin package. Thus, it shouldn't be there. Otherwise nixpkgs-review output is confusing. Not only is the (pkg.override* foot).tests.nixos a lie, but also the regular pkg.tests.nixos - it suggests that the darwin-build had been tested, but that's not true.

The whole point of enabling nixosTests on darwin is to allow nixpkgs developers on darwin to actually develop for NixOS - and run the tests for that. Correct?

Thus, nixosTests should work on darwin, but pkg.tests.nixos should not. That would be the best UX/DX.

all packages need to only add NixOS tests in an // optionalAttrs hostPlatform.isLinux

Can we maybe make this part of what's currently called setPackage?

Aka, turns this function into a "do everything that needs to be done to run this test via pkg.passthru.tests sensibly". Set the right package, return an empty attrset for darwin, .. maybe something else (in the future).

[roberth]

all packages need to only add NixOS tests in an // optionalAttrs hostPlatform.isLinux

Can we maybe make this part of what's currently called setPackage?

Unfortunately this optionalAttrs needs to go in passthru.tests. It's the part that would implement the idea "pkg.tests.nixos on darwin is not testing the darwin package. Thus, it shouldn't be there."

[wolfgangwalther]

Unfortunately this optionalAttrs needs to go in passthru.tests. It's the part that would implement the idea "pkg.tests.nixos on darwin is not testing the darwin package. Thus, it shouldn't be there."

I don't understand why it needs to be in passthru.tests. passthru.tests needs to call .setPackage, so the conditional could sure also be in there? If .setPackage just returns an empty attrset, there will be an "empty passthru test", which is exactly what we want on darwin, right? What am I missing?

[roberth]

Rebased onto merged #421480 and cassandra changes, by dropping the cassandra changes.
I'm not convinced that setPackage is the right solution at this point. See earlier comment.

[roberth]

Alternative direction:

• nixos/testing: Add onVMHost attribute to all tests #423276

(also independently useful, I'd say)

[wolfgangwalther]

I'd say it's not really an alternative, both are orthogonal. I think we'd want both:

• the ability to use (pkg.override*).tests to work on linux and pkg.tests.nixos to not be available on darwin (this PR) and
• the ability to run (pkg.override*).tests for a linux package from a darwin host (nixos/testing: Add onVMHost attribute to all tests #423276)