Replace collectRunComponents with haskellLib.check #316

hamishmack · 2019-11-14T01:18:25Z

In order to make it easier to run tests this change adds a
collectRunComponents function (like collectComponents but
with it returns the passthru.run derivation for each component).

This change also disables checkPhase for all components to avoid
running the tests twice. To run a test or benchmark you now have to
use the the passthru.run derivation.

In order to make it easier to run tests this change adds a collectRunComponents function (like collectComponents but with it returns the `passthru.run` derivation for each component). This change also disables `checkPhase` for all components to avoid running the tests twice. To run a test or benchmark you now have to use the the `passthru.run` derivation.

angerman

LGTM! With lots of comments 🎉

rvl

Looks better, but I would prefer to leave collectComponents as is, and make a more specialised runTestComponents.

builder/comp-builder.nix

rvl · 2019-11-14T05:52:16Z

builder/comp-builder.nix

+    # This run derivation can be used to execute test, benchmark (or even
+    # exe) components.  The $out of the derivation is a file containing
+    # the resulting stdout output.
+    run = stdenv.mkDerivation ({


Looks better

lib/default.nix

rvl · 2019-11-14T06:01:23Z

builder/comp-builder.nix

+    run = stdenv.mkDerivation ({
+      name = (fullName + "-run");
+
+      src = drv;


What if the test suite has a dependency on data files in the source directory, will it work?

The executable is not copied out of the location it is in. I checked that echo $src was the same as echo ${drv} in the checkPhase. (if that is what you were concerned about).

rvl · 2019-11-14T06:03:29Z

builder/comp-builder.nix

+      checkPhase = ''
+        runHook preCheck
+
+        ${toString component.testWrapper} $src/${installedExe} ${lib.concatStringsSep " " component.testFlags} | tee $out


Is there a good reason that installedExe is not bin/${componentId.cname}?

I am not sure. See

haskell.nix/builder/comp-builder.nix

Lines 120 to 123 in 6e95ba5

# exe components are in /bin, but test and benchmarks are not. Perhaps to avoid

# them being from being added to the PATH when the all component added to an env.

# TODO revist this to find out why and document or maybe change this.

installedExeDir = if haskellLib.isTest componentId || haskellLib.isBenchmark componentId

Yeah I think it should be bin. Having the component name as a directory is inconvenient.

Co-Authored-By: Rodney Lorrimar <rodney.lorrimar@iohk.io>

rvl · 2019-11-14T10:07:57Z

Because having a passthru derivation doesn't work well with mapTestOn, and because, .run is hidden quite deeply, could we change the layout?

Could you move it to hsPkgs.<package>.checks.<name>? This will execute the tests for hsPkgs.<package>.components.tests.<name>.

michaelpj · 2019-11-14T10:12:05Z

Could you move it to hsPkgs..checks.?

What about test-runners instead of checks? A bit more descriptive, perhaps.

rvl · 2019-11-19T01:18:07Z

builder/hspkg-builder.nix

  components = haskellLib.applyComponents buildComp config;
+  run = builtins.mapAttrs (_: es: builtins.mapAttrs (_: d: d.run) es)
+    { inherit (components) exes tests benchmarks; };


I can't really see why users would want to run exes and benchmarks like this.

It could be simpler with a single test-runners attribute.

An exe might be pure generator (for instance it could build an html file or a log table). I think making sure it runs could be meaningful test.

Checking that a benchmarks runs seems like it could also be a meaningful test. You might not care about the performance result when run in nix (since it is hard to know what load the machine will be under), but you could also have it exitFailure if the result was very poor.

caches `configure-flags`, `cabal.config`, and `ghc-environment` files per library in the nix store, so the `make-config-files` script just has to append them into the same named files for the package being built instead of using `ghc-pkg dump` and `ghc-pkg register` for each dependency, copies the `.conf` files from the dependencies `package.conf.d` directory and then uses a single `ghc-pkg recache` eliminate the call of `ghc-pkg check` suppress some output of `ghc-pkg` (not related to speed improvements, I'm flexible on rolling this bit back) in my tests, the make-config-files scripts now only take seconds, even for packages that have many many dependencies - prior to this change, this script took more than a minute for such packages

This is what most people will want and it means using collecting all the derivations with `release-lib.nix` will work without having to filter out `exes` and `benchmarks`.

`normalizeRelativeDir` adds a slash on to the end of `dataDir`. Adding another one here results in `//` and files are left out by mistake.

hamishmack requested review from angerman and rvl November 14, 2019 01:18

hamishmack added 3 commits November 14, 2019 14:27

Reduce duplicate code

349abfa

Add comment for passthru.run

cfb1699

Update docs for collectRunComponents

de8c4ad

angerman approved these changes Nov 14, 2019

View reviewed changes

rvl suggested changes Nov 14, 2019

View reviewed changes

hamishmack and others added 2 commits November 14, 2019 20:07

Update builder/comp-builder.nix

b29de00

Co-Authored-By: Rodney Lorrimar <rodney.lorrimar@iohk.io>

collectBuildComponents to collectComponents

47e0bb7

hamishmack requested a review from rvl November 14, 2019 08:34

hamishmack added 3 commits November 18, 2019 17:15

Relocate binary for benchmarks/tests (not in all)

cee580c

Add package.run.(exes/tests/benchmaks).${name}

42c7cce

Try ln -s instead of cp for DLLs

a51248c

rvl reviewed Nov 19, 2019

View reviewed changes

Richard Wallace and others added 7 commits November 19, 2019 21:21

use ld.gold by default on linux

b68c60e

Merge remote-tracking branch 'origin/master' into hkm/passthru-run-2

e6505d5

Just include checks for compontent.tests

4d619a7

This is what most people will want and it means using collecting all the derivations with `release-lib.nix` will work without having to filter out `exes` and `benchmarks`.

Add ifdInputsOnly and set it to true in relase.nix

9b9fa13

Reduce CI times by excluding older ghc versions

a43d302

Enable all tests again

a9d867d

rvl mentioned this pull request Nov 21, 2019

Wallet Backend builds and tests on Windows cardano-foundation/cardano-wallet#703

Closed

27 tasks

hamishmack added 4 commits November 21, 2019 19:52

Test IFD inputs only

c54a039

haskellLib.check instead of passthru.run

d8314a7

Enable all tests again

b6975b5

Default doCheck=true. Filter checks on doCheck

cf15c36

hamishmack requested a review from rvl November 22, 2019 00:58

hamishmack added 27 commits November 23, 2019 23:10

Improve ifdInputsOnly to include more ifd inputs

64afed9

Improve ifdInputsOnly to include more ifd inputs

3ce1b29

Improve ifdInputsOnly to include more ifd inputs

64689a4

Improve ifdInputsOnly to include more ifd inputs

bf5dc77

Improve ifdInputsOnly to include more ifd inputs

79a7f9f

Merge remote-tracking branch 'origin/master' into hkm/passthru-run-2

bb5daae

Improve ifdInputsOnly to include more ifd inputs

37b9e11

Enable all tests again

ec4a4eb

ifdInputs only

e8fae60

Include build inputs of ifd nix derivations

c6ab443

Try using ifd level to better control hydra eval

1dc1708

Set preferLocalBuild=false instead of withInputs

a1e0b92

Only build tests when ifdLevel>1

a8a044f

ifdLevel = 1

e861f35

ifdLevel = 2

fff24e4

Run check-hydra on all the IFD levels

d8199c4

Run check-hydra on all the IFD levels

5a321d3

Only build ghc-extra-packages for ghc865

6021f5d

Only build ghc-extra-packages for ghc865

b79da5f

Fix maintainer scripts

4cf064f

Skip check-hydra on windows

192737c

Skip check-hydra on windows

27ce94c

ifdLevel = 3

c2be4ab

Fix data-dir: when cleaning components

344abd7

`normalizeRelativeDir` adds a slash on to the end of `dataDir`. Adding another one here results in `//` and files are left out by mistake.

Make test component source available in check

fa15c3a

Remove collectRunComponents. Doc haskellLib.check

46cd1a8

Merge remote-tracking branch 'origin/master' into hkm/passthru-run-2

d635a22

hamishmack changed the title ~~Add collectRunComponents. Move checkPhase to .run~~ Replace collectRunComponents with haskellLib.check Nov 29, 2019

hamishmack merged commit a9b0122 into master Nov 29, 2019

hamishmack deleted the hkm/passthru-run-2 branch November 29, 2019 10:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace collectRunComponents with haskellLib.check #316

Replace collectRunComponents with haskellLib.check #316

hamishmack commented Nov 14, 2019

angerman left a comment

rvl left a comment

rvl Nov 14, 2019

rvl Nov 14, 2019

hamishmack Nov 14, 2019

rvl Nov 14, 2019

hamishmack Nov 14, 2019

rvl Nov 14, 2019

rvl commented Nov 14, 2019

michaelpj commented Nov 14, 2019

rvl Nov 19, 2019

hamishmack Nov 19, 2019

	# exe components are in /bin, but test and benchmarks are not. Perhaps to avoid
	# them being from being added to the PATH when the all component added to an env.
	# TODO revist this to find out why and document or maybe change this.
	installedExeDir = if haskellLib.isTest componentId \|\| haskellLib.isBenchmark componentId

Replace collectRunComponents with haskellLib.check #316

Replace collectRunComponents with haskellLib.check #316

Conversation

hamishmack commented Nov 14, 2019

angerman left a comment

Choose a reason for hiding this comment

rvl left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvl commented Nov 14, 2019

michaelpj commented Nov 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment