tests: run more of them in ci and hydra#428706
tests: run more of them in ci and hydra#428706jopejoe1 wants to merge 3 commits intoNixOS:masterfrom
Conversation
jopejoe1
force-pushed
the
run-more-tests
branch
from
4213747 to
c69add5
Compare
wolfgangwalther
left a comment
wolfgangwalther
left a comment
There was a problem hiding this comment.
Eval fails with no helpful error message, at least I can't see any. Might be more helpful when run locally, but not sure.
|
The failure is due to the CI running out of memory. |
jopejoe1
force-pushed
the
run-more-tests
branch
from
c69add5 to
24f4921
Compare
|
Oh, that's interesting. Is this failure random or introduced because eval for this PR now takes much more memory? In either case, we'd have to adjust the chunkSize for CI. Related: #431459. |
|
(CI was somehow not triggered correctly on the last push (?)) Edit: Still not running the PR workflow, I have no idea why... Edit2: The new PR run is pending, because the old one, that ran out of memory has somehow not been cancelled correctly. |
|
The old PR run seems to be stuck here: https://github.com/NixOS/nixpkgs/actions/runs/16829073601/job/47672217906 I tried cancelling it, but that didn't change a thing. I guess we'll have to wait until this is killed after... 6 hours (?). (This reminds me of putting more reasonable time-limits in place for all the CI jobs...) |
|
So, I guess the big question is whether we really want to add all these calls to An alternative would be to explicitly not recurse into these attrsets, but add comments explaining why. (I do think the change to an attrset instead of a list is a good one in any case, we should keep that) |
|
With #432286 merged, a rebase of this should not cause OOM anymore. |
jopejoe1
force-pushed
the
run-more-tests
branch
from
24f4921 to
81be307
Compare
| pkgs.spike | ||
| ]; | ||
|
|
||
| sanity = lib.recurseIntoAttrs ( |
There was a problem hiding this comment.
I don't think we can actually do this recursion, because:
- This takes too many resources to Eval, and
- some of these use "variants" (pkgsLLVM, etc.), which are disabled for CI. The long-term goal is to not allow any variants used within nixpkgs.
There was a problem hiding this comment.
some of these use "variants" (pkgsLLVM, etc.), which are disabled for CI. The long-term goal is to not allow any variants used within nixpkgs.
If that is the case, I'm not sure that we should have those tests in their current state at all.
There was a problem hiding this comment.
This was introduced in #243321.
Using them via ofborg still potentially makes sense, right? I'm not sure whether that's possible without recurseIntoAttrs, though. Now I wonder how recurseForDerivations behaves exactly - what happens when I do nix-build -A tests.cross.sanity and the sanity attrset does not have recurseForDerivations set? Would it still go through that attrset, because I specified it explicitly or would this already require the recurse attribute?
|
Seems like it even OOMs with the swap enabled. This is not going to work with all the |
nixpkgs-ci
bot
added
the
2.status: merge conflict
nixpkgs-ci
bot
added
the
2.status: stale
2 participants
Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.