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
Improve release-cross tests #21414
Improve release-cross tests #21414
Conversation
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @domenkozar and @rbvermaa to be potential reviewers. |
94c76f8
to
621e26a
Compare
Eventually we'll want to test cross-compiling *from* various platforms. For now, its good to be consistent.
621e26a
to
cc68039
Compare
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.
The first commit was a little hard to review so I only glanced over it. The rest looks good, though I did point out one nit.
|
||
testEqual = path: systems: forAllSupportedSystems systems (testEqualOne path); | ||
|
||
mapTestEqual = lib.mapAttrsRecursive (path: systems: testEqual path systems); |
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.
The expression in parentheses is really just a verbose way of writing testEqual
.
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.
Oh hah! A few refactors in and I totally missed that worthless eta expansion. Thanks!
These derivations do not care about the target platform, and thus should not be affected by cross-compiling. Currently, these tests *fail*, but they will be fixed soon by a latter PR. The release-cross job doesn't block a channel, so this should be no problem.
cc68039
to
1d0e918
Compare
http://hydra.nixos.org/eval/1317573 The good news is I didn't break any existing jobs, the bad news is that my |
Ah, I guess it has to be |
Those assertions should be failing, so I figured Hydra would have no idea whether there is a derivation behind them. Maybe it has some magic, but in any event I don't want the job to dissapear if the assert isn't thrown. I'll try something like |
FWIW they do show up in http://hydra.nixos.org/jobset/nixpkgs/cross-trunk#tabs-errors. But yes, you do need some sort of dummy derivations so they show up like the others. |
Oh, that's a good start. It's weird that that tab is there with all the evaluations, rather than per-evaluation. |
From this PR:
Equality comparison of derivations seems to be special and different from equality comparison of a normal set. When you make the changes necessary to get those tests to pass, it proves that a derivation with a |
Ahead of #21388 or #21268, it would be nice to test cross-compiling better. This PR cleans up the existing tests, and adds some new ones. Those new ones fail, but that's OK: they won't hold up any channel, and they'll remind us to merge one of the two PRs above, which fixes the new tests.
@DavidEGrayson What do you think? I'd add you as a reviewer on the side panel, but It won't let me. If you like this I'll go ahead and merge it.