jmeter has an installCheck that checks each executable file. I think this could be good for a few of the packages I have added to nixpkgs but I don’t see it used often. In some cases it is explicitly turned off. Would it be ok to add this check to all packages I maintain. The check would likely get the usage output and check for the version string. Would it be reasonable to do this in one PR?
2 Likes
Yes, please do! More tests are always good, especially “integration” (install) test. There was an attempt at a more generic solution to the tests you describe here, but unfortunately that didn’t go anywhere. Still hoping for something like that in the future.
4 Likes
More tests are not always good. In particular integration tests tend to be very brittle and cause a lot of false alarms (see the NixOS VM tests, which frequently fail due to obscure timing-related problems).
1 Like
Another counter-example to adding more tests is sageWithDoc which I think takes over an hour to complete all the tests. (makes it harder to review it’s upstream dependencies).
Ideally, you want to have a balance of making sure that the package is valid, and that it usable by other software (libraries) or users.
2 Likes
That proposal looked particular interesting.
Though I do agree with @edolstra sentiment on brittle integration tests not being helpful, but I can think in several places where tests could help out with nixpkgs development. For example NetworkManager has no integration tests (for nixos) and it’s a crucial component for many users.
Which I think makes releasing NixOS hard, we could much better determine we’re ready for release if we had a base release criteria. And perhaps the tested jobset is this, but that jobset doesn’t have nearly enough coverage for what’s actually being used.
2 Likes
I’d say the contrary: It would be essentially impossible to reliably guarantee all the functionality of sage without its extensive test suite. As a side-effect, it indirectly tests a lot of the math ecosystem as well (many of those packages don’t have many tests of their own).
Also the tests the OP was talking about here (“executable --help does not crash”) are more in the realm of sanity checks and really should not be brittle. The package test approach in general should be less brittle and less susceptible to timing issues than VM tests, but that is somewhat out of scope for this question.
2 Likes
I’m not familiar with sage itself, but it is very painful when a change causes it to rebuild, I have passed over reviewing many PRs when i see that the last remaining package is sage.
I do agree that if an application can’t even do a --help, then it is fundamentally broken, and shouldn’t be included into nixpkgs.
I’m not familiar with sage itself, but it is very painful when a change causes it to rebuild, I have passed over reviewing many PRs when i see that the last remaining package is sage.
For direct Sage dependencies the test suite is sometimes relevant… I wonder if it is a good idea to have a list of dependencies which are likely to break Sage, and document that in other cases a build without tests is enough.
So to get back to topic: does it make sense for maintainers to add these kind of “smoke tests” to existing packages?
I have been wondering this as well. For instance, I manually check spaCy on updates, but I have written a small NixOS test that downloads a small model, annotates a sentence and verifies the output. It’s all pretty quick to run.
But should this actually be a NixOS test or an install check?
1 Like