Test support
This PR implements support for emulating spago test from nix's building environment.
N.B. It's currently built on top of several other branches. It will be rebased and declared ready for a proper code review once those land. This PR is sent out to facilitate a concrete albeit preliminary discussion about alternatives.
The only relevant commit is ba1b2c12f5e33133d6be0cd2fc9e70e5195c5fd2.
Motivation
Being able to run an equivalent of spago test from within nix makes CI easy.
- Writing CI pipelines is simplified because there is no need for special testing steps. They can just be part of a single
nix flake check/nix build .#my_website. - CIs are more reliable. The alternative of building with nix and testing separately is brittle because it's not clear that the artifacts/dependencies match in the two cases.
Design
- Compile the test modules in
checkPhaseofbuildSpagoLockby invokingpurs, similarly to how it's today invoked frombuildPhase. This is the right place for it because the compilation cache is readily available. The nix package will refuse to build if tests fail. - Require user to explicitly turn on testing of the packages in their workspace by passing the names of those packages to
buildSpagoLock. - Rely on
slimlockto buildnpmDependenciesfor testing. Expose them by changing the user-facing API ofbuildSpagoLockto return{ jsArtifacts, npmDependencies }, where-
jsArtifacts: map from package names to derivations with
output/directory. This is whatbuildSpagoLockreturns today. -
npmDependencies: the output of
slimlockran on$src/package-lock.json.
-
jsArtifacts: map from package names to derivations with
Considerations
- It's good to expose npmDependencies to the user, so that they can readily use them in the bundling step. This increases confidence that the node module used in tests and in the bundle are identical.
- If the user doesn't reference
npmDependenciesand doesn't enable tests, they won't be built due to nix's laziness. - OTOH, if they are to be built, the current implementation expects to find
slimlockin scope, which requires the user to have installed it via overlay. This is to allow user not to depend on it, when it's not needed. -
super.callPackageis changed toself.callPackageto allow the user to inject theslimlock. Seems correct to have it this way for other reasons too (e.g. allow user to override the node version), but I'm not 100% confident this won't lead to looping in some edge case. -
Test.Mainis currently hardcoded as the main module of the test because we have no way of usingfromYAMLonspago.yaml(it may contain comments).spagocould be modified to include this information in thespago.lock.
Alternatives
- Instead of calling
slimlockallow the user to pass in the node dependencies explicitly.- Pros
- More flexible for the user.
- Doesn't break the API.
- No confusion about errors stemming from
slimlockmissing from the overlays, when testing is enabled.
- Cons
- Requires more boilerplate.
- Possible source of bugs, if the node modules are built erroneously (e.g. node modules only relevant for the test dependencies are missing for some reason). This is a contrived point.
- The exact directory of the node modules becomes magical and brittle: e.g. slimlock puts them into
js, node2nix useslib.
- Pros
- Use
backend-optimizer- Pros
- Faster tests
- May uncover bugs in backend-optimizer (unlikely)
- Cons
- Slower builds
- Complexity
- Pros
Call for reviewers! Per our preliminary conversation with @thomashoneyman the way npm dependencies are handled might need to change. This PR is currently producing the node dependencies internally using slimlock (which should be simple for the end user), but a more flexible alternative is to require the user to explicitly pass them to the buildSpagoLock function.
That being said, I sent the PR for review as is, to make sure I collect all the feedback before addressing the reviewer comments.
Thanks for submitting this, @bakhtiyarneyman. A couple thoughts right away:
Instead of calling slimlock allow the user to pass in the node dependencies explicitly.
I prefer this; having worked across a number of PureScript projects, there are many different ways to build node_modules, and we shouldn't force slimlock. For example, the registry-dev project uses slimlock, but this overlay uses prefetch-npm-deps and stores the deps hash.
[con] The exact directory of the node modules becomes magical and brittle: e.g. slimlock puts them into js, node2nix uses lib
Hm. Perhaps we could accept either a) a path to a directory containing node_modules, or b) simply node_modules itself, which would sidestep this con?
we have no way of using fromYAML on spago.yaml (it may contain comments).
We probably will have to stop using fromYAML and switch to proper YAML parsing. Purifix, for example, uses yaml2json.