python3Packages.raylib-python-cffi.tests: fix the eval#480960
Conversation
nixpkgs-ci
bot
added
10.rebuild-darwin: 0
nixpkgs-ci
bot
removed
the
9.needs: reviewer
Without the chnage the eval fails:
$ nix-instantiate -A python3Packages.raylib-python-cffi.tests
error:
… from call site
at pkgs/development/python-modules/raylib-python-cffi/default.nix:52:20:
51|
52| passthru.tests = import ./passthru-tests.nix {
| ^
53| inherit writers;
error: function 'anonymous lambda' called without required argument 'src'
at pkgs/development/python-modules/raylib-python-cffi/passthru-tests.nix:1:1:
1| {
| ^
2| src,
trofi
force-pushed
the
python3Packages.raylib-python-cffi.tests-fix-eval
branch
from
bace3f9 to
77884b9
Compare
|
As an FYI, this test could use some cleanup. For example, this is IFD: } (builtins.readFile (src + path));importing a file from inside the In this case, we don't need to read that file at eval time only to use it to create another derivation. Instead we can write a derivation that copies, symlinks, or directly references the relevant file at build time. |
nixpkgs-ci
bot
added
the
12.approvals: 1
For reference, the test was added in #387993 and hasn't been touched since. In addition to the IFD issue mentioned in my last comment, AFAICT the tests don't actually test anything.1 All they do is repackage some scripts from cc original author @Sigmanificient and reviewer @GaetanLepage Footnotes
|
|
Thanks for fixing this issue, i'm definitely not be best at doing nix 😅 @MattSturgeon I am opened for suggestions to improve theses tests, but I don't see anything other than going all in with a nixosTests + screenshot, which seems super heavy ...and still require you to actually check nothing has gone wrong. I also remember seeing this pattern within another package, so I'm pretty such this isn't the only package doing a test you need to actually run 🤔 |
| writers, | ||
| }: | ||
| let | ||
| src = raylib-python-cffi.src; |
There was a problem hiding this comment.
More a curiosity question than anything, but would something like inherit (raylib-python-cffi) src; work or does it cause something weird to happen that I am unaware of?
There was a problem hiding this comment.
Yeah, inherit (foo) bar and bar = foo.bar are exactly the same thing. I usually prefer the inherit style, but I try to avoid nitpicking such trivial details when I can restrain myself.
There was a problem hiding this comment.
The idea is not to nitpick on this, just to know if there is some notable behavior! I do prefer the inherit style too, but this will go when the opportunity comes. I prefer to challenge my perception, in case i could be wrong :)
I think half the issue is that it is unclear how you expect the tests to be used. If I see a package's When I read the test's impl I was super surprised to see them not validating anything, but instead just re-packaging some scripts from the package's src. I did guess that maybe you intended the test derivation's outputs to be actually ran, but that concept is so unusual it feels like a bug. All of this is separate to the IFD issue; we should never import the content of derivation output files into a nix evaluation, because nixpkgs has a policy against IFD. It's less bad within passthru tests that aren't typically evaluated, but it is still totally unnecessary when re-packaging can be done in a build script (e.g. using |
3 participants
Without the chnage the eval fails:
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.