Disable server tests in the nix build #1123
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| # Additional flags passed to the cargo test binary, see `cargo test -- --help` | ||
| checkFlags = [ | ||
| # Registration tests require a postgres server | ||
| "--skip=registration" |
There was a problem hiding this comment.
| "--skip=registration" | |
| "--lib --bins" |
Or similar may work better? If that's exactly what is passed to cargo test, then it will ensure we do not run any integration tests here
There was a problem hiding this comment.
Merging as-is for now, I'll look at changing or updating this when we add more integration tests!
There was a problem hiding this comment.
The call to cargo test is this invocation. The arguments passed to checkFlags are passed into the binary with cargo test -- --lib --bins, so that won't work, and it doesn't look like there's a way to pass those arguments without co-opting some other feature.
If that way of calling cargo makes more sense for the tests here, it's probably clearer to just override the checkPhase and write the testing script ourselves. It's just a bash script.
# Override the checkPhase script. This is run after building to verify correctness.
# Uses release mode to avoid building in debug mode just for testing
checkPhase = ''
cargo test --release --lib --bins
'';
There was a problem hiding this comment.
Ah I see, thanks for explaining! Will figure out which makes the most sense when we add more tests
There was a problem hiding this comment.
These tests are run in a sandbox with no network access, so it's fairly common that projects with complex test cases just won't work within it very easily.
I see! That explains a lot, thank you 🙏
Alternately we could just disable tests entirely in the nix build by using doCheck = false; instead of checkFlags. See docs for guidance. That may be preferable if you don't expect to get any value out of running the tests inside the nix sandbox.
I'll leave the tests running as part of the build for now, but may disable them in future
For a few reasons 1. This step is really, really slow. I don't think there's sufficient value in a slow CI step to keep it 2. Whenever we add an integration test it needs to be added to the ignore list. I want to keep friction on adding such tests as low as is possible. 3. We already run tests in a bunch of places, so I don't think this is needed Ref: #1123
For a few reasons 1. This step is really, really slow. I don't think there's sufficient value in a slow CI step to keep it 2. Whenever we add an integration test it needs to be added to the ignore list. I want to keep friction on adding such tests as low as is possible. 3. We already run tests in a bunch of places, so I don't think this is needed Ref: #1123
The recent #1096 broke the Nix CI, this change fixes the nix build by disabling those tests. New tests that also use the postgres server will need to be excluded as well using additional '--skip' options or by keeping them all in the same excluded module. Adding a postgres server to make these pass is technically possible, but probably not worth the maintenance burden for you.
For background, the
nix buildstep runs the tests by default in an attempt to verify the correctness of the build. These tests are run in a sandbox with no network access, so it's fairly common that projects with complex test cases just won't work within it very easily.Alternately we could just disable tests entirely in the nix build by using
doCheck = false;instead ofcheckFlags. See docs for guidance. That may be preferable if you don't expect to get any value out of running the tests inside the nix sandbox.(I noticed this because I do actually use the nix flake to run a bleeding edge version of the client on my own systems, so this flow is tested by at least 1 person :))