kernel: fix passthru.tests #247520
kernel: fix passthru.tests #247520
Conversation
discoverTests tries to discover some sort of internal function and tries to call it with the arguments for that internal function. This poses an issue when you want to expose some other functions (i.e. a parameterisation for a test) in nixosTests. This commit allows a test to pass through arbitrary values via `.passthru` without them having discovery applied to them; including functions.
NixOS#191540 indirectly broke kernel passthru.tests; calling the testsForLinuxPackages and testsForKernel functions with some args intended for some other exposed test-internal function. Organise the passed-through functions under `passthru` to prevent this from happening.
Atemu
added
6.topic: kernel
github-actions
bot
added
the
6.topic: nixos
ofborg
bot
added
10.rebuild-darwin: 0
| @@ -21,7 +21,7 @@ let | |||
| if isAttrs val | |||
| then | |||
| if hasAttr "test" val then callTest val | |||
| else mapAttrs (n: s: discoverTests s) val | |||
| else mapAttrs (n: s: if n == "passthru" then s else discoverTests s) val | |||
There was a problem hiding this comment.
iirc this code is used by both release.nix and nixosTests, and it has to deal with all tests, which are still written in a number of styles.
So this is hard to review; can't really tell the implications without a lot of manual testing.
In my opinion release.nix should be rewritten to follow the more easily understood structure of nixosTests, but until that's done, we need to consider that both code paths used this ill-conceived semi-generic code.
All that aside, we should really try to avoid the proliferation of the term "passthru". Unfortunately it's the mechanism we have to use with mkDerivation, but it is by itself meaningless, and its presence in the package attribute set is an implementation detail. So if we're going to accept a fix for the problem it would be great for it not to use the passthru attribute.
As for rewriting release.nix, ideally we'd have something like #176557 which has a nice and general solution for this particular problem of overriding a test to use a specific package. But the actual implementation was a bit messy, which is why I think it should be rewritten. It would probably still resemble the documented example.
There was a problem hiding this comment.
I agree this isn't a great solution. It's really more of a workaround and I don't like adding special cases like this but I don't really see another way without refactoring the entire tests infrastructure which I don't feel comfortable doing and even you doing it would take quite some time (see #176557).
By using "passthru" as the special attribute's name, I hope to make it somewhat clear that it's intended for passing through values rather than exposing tests. Do you have a better name for it?
All this should change is that anything organised under a passthru isn't discovered as a test. My hope is that no Nixpkgs contributor, knowing the meaning of passthru in mkDerivation, would organise an actual test under "passthru", only values for, well, passing through.
|
Successfully created backport PR for |
|
Thanks! |
The .overrideAttrs part is ok now, but a "passthru' workaround is necessary now. See NixOS#247520
Description of changes
#191540 indirectly broke kernel
passthru.tests; calling the testsForLinuxPackages and testsForKernel functions
with some args intended for some other exposed test-internal function.
Organise the passed-through functions under
passthruto prevent this fromhappening.
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)