haskell-modules/generic-builder.nix: Allow running Haskell tests as passthru.tests #305958
haskell-modules/generic-builder.nix: Allow running Haskell tests as passthru.tests #305958TeofilC wants to merge 1 commit intoNixOS:haskell-updatesfrom
Conversation
github-actions
bot
added
the
6.topic: haskell
TeofilC
changed the title
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
b6e26a3 to
79a1239
Compare
ofborg
bot
added
10.rebuild-darwin: 501+
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
5 times, most recently
from
76311e4 to
8eb06a3
Compare
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
8eb06a3 to
cf7cae6
Compare
|
I like the idea of this, but I think we need a bit more discussion on the design. I am not certain whether this design as is will make anything easier. Concretely can you name a kind of iteration on the tests which will not invalidate building the original package? |
@maralorn Thanks for taking a look! That's a good point. By default they will have the same source and whenever the main build rebuilds, this will also rebuild. I also find that tests need more maintenance than regular libraries. They tend to require more of an environment such as extra exes, resources (eg, the time zone database), or other services. This means they tend to need a lot of small tweaks and it's nice to only have to rebuild the test derivation when that happens. Also when you have other serivces it's quite nice to be able to have this as a separate derivation cause then you can turn it into a VM based build and have things set up in a more principled way. And means your main derivation won't rebuild when these potentially loosely coupled things change. The main advantage of this approach though isn't in the relationship between an individual main derivation and the test deriviation, but in its effects on the overall build graph of a larger project. As it stands downstream libraries will not be built if the test suite fails. This is quite annoying if your builds are slow and leads to slow feedback loops when using the nixpkgs infrastructure to build things in CI. You end up fixing a test suite, only to find that something else downstream in the build graph was also broken but hadn't been tried. Making it so that as much of the build graph is attempted to be built even if they fail means you can cut out a lot of waiting. |
wegank
added
the
2.status: merge conflict
wegank
added
the
2.status: stale
Additionally, stuff can be parallelized better, because downstream packages can already start building while upstream tests are still running. |
stale
bot
removed
the
2.status: stale
There was a problem hiding this comment.
Maybe it's better to put the logic into doCheck = ..?
The current approach imho leads to having the default checkPhase, so at a minimum we'll get some log output, but depending on the source repo, possibly even something that is run via Makefile or so?
There was a problem hiding this comment.
doCheck is about whether we want to enable test suites at configure time. I have simply removed this line instead. While strange, a user could conceivably want to both run the checks in the main derivation and the separate one. So if they don't want it run in the main checkPhase then they should disable that separately. I'm not 100% sure if this is the right way to go, but it seems like the option that makes the fewest assumptions
|
I'll try to pick this back up and integrate folks comments soon. Given that we don't want to depend on |
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
cf7cae6 to
998953a
Compare
ofborg
bot
removed
the
2.status: merge conflict
|
Thanks for your comments @maralorn and @wolfgangwalther. I have done some more work on this. I have reworked this to not depend on the list of test suites at eval time. Instead of having one passthru deriviation for each test suite, we have just one that runs all the test suites. And if no I have updated the PR description and explained the motivation a bit more. I hope this should address your comments. Let me know if you have some more thoughts. I haven't tested this in depth yet or added new utilities to |
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
998953a to
d26733a
Compare
maralorn
left a comment
maralorn
left a comment
There was a problem hiding this comment.
I think I am convinced this could be useful and am in favor of moving forward.
I still feel that some tests will fail in this rudimentary separate derivation because of a multitude of unexpected reasons. But we will have to work on that iteratively when we encounter it.
Besides that I still feel like we should try our bests to decouple the derivations as much as possible. e.g. with doCheck = false we possibly need to null the checkPhases stuff in the original derivation so that modifying it does not invalidate the derivation?
Would it be possible to do the building of the tests also in the test derivation?
|
Thanks for taking a look @maralorn! I'll put some more time into this PR in the next few weeks.
Sounds reasonable.
I think the principled option here would be to just have better support for per-component builds in the nixpkgs infra. I think we are already most of the way there with the At some point, I want to migrate the nix code at $work from using haskell.nix to the nixpkgs infra, and I'm hoping to get per-component builds working for that along with the sort of stuff in this PR (and ideally I'd like to upstream as much as possible). |
nixpkgs-ci
bot
added
the
2.status: merge conflict
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
d26733a to
28bdc2e
Compare
nixpkgs-ci
bot
removed
the
2.status: merge conflict
…assthru.tests We add an `enableSeperateTestOutput` option to the Haskell builder. When enabled, we install the test suites into a `tests` output and create a passthru tests.checks deriviation that will run the test suites. We take the list of test suites from `testTargets` if it is defined, and probe Cabal for all the test suites otherwise.
TeofilC
force-pushed
the
wip/haskell-separate-tests
branch
from
28bdc2e to
5fb1172
Compare
|
This should now be ready for another look (sorry! it took so long). @maralorn |
|
Appears sensible to me. Have you already experimented on a few packages whether the separate test works? |
4 participants
Description of changes
This adds a flag to the Haskell generic builder to allow running the test suites through a
passthru.tests.checksderivation.This is quite helpful for large/complex test suites attached to large libraries. Having these as separate derivations makes it a lot quicker to debug why a test suite is failing. It's also quite nice for end-users cause there's a clearer separation between "this build failed" and "this test-suite failed".
It also means that if just the test suite fails then the overall build graph isn't blocked. So you can potentially build more packages and get more information from one feedback cycle. This makes a big difference when building large package graphs.
If a test requires KVM, this is also helpful for just making the test derivation require that.
This is a feature in haskell.nix and I'm taking inspiration from their implementation. (I'll add credit in the commit message once I write up a proper one). For reference here's the equivalent bit of haskell.nix: https://github.com/input-output-hk/haskell.nix/blob/master/lib/check.nix, which has a few more bells and whistles.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.