phoronix-test-suite: add tests

[davidak]

Motivation for this change

Add tests to make reviews easier and the package more stable.

This is my attempt to implement package testing like discussed in #73076 (comment).

Please provide feedback!

Some aspects might be controversial like with phoronix-test-suite;. It allows to simply use "${version}" in the tests, but using with was discouraged for some reason.

Also what about naming?

  passthru.tests = {
    tests = callPackage ./tests.nix { };
  };

tests tests tests don't read nicely. should we call it "simple" or "basic"?

When it is approved i would like to add tests to more packages and document it.

Things done

• Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
• Built on platform(s)
  • NixOS
  • macOS
  • other Linux distributions
• Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
• Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
• Tested execution of all binary files (usually in ./result/bin/)
• Determined the impact on package closure size (by running nix path-info -S before and after)
• Ensured that relevant documentation is up to date
• Fits CONTRIBUTING.md.

Notify maintainers

cc @jtojnar @worldofpeace @Profpatsch @7c6f434c

[jtojnar]

To be honest, I do not see much value in these trivial execution tests. I would say that the application builds but does not execute after update is pretty small. And the reviewer still needs to at least run the program and try some action so it will be already covered by this.

Even a trivial actions would be enough for this approach to be worth in my eyes (e.g. testing that calculator returns 4 for 2+2). But perhaps this is a rather hard program to test, as it probably relies on real hardware.

[davidak]

@jtojnar thanks for your feedback!

To be honest, I do not see much value in these trivial execution tests. I would say that the application builds but does not execute after update is pretty small. And the reviewer still needs to at least run the program and try some action so it will be already covered by this.

My plan is to automate this manual tests to check this boxes:

• Built on platform(s) NixOS
• Tested execution of all binary files (usually in ./result/bin/)

This is especially useful for automated updates with nixpkgs-update, but also for easier review with ofborg and nixpkgs-review.

Even a trivial actions would be enough for this approach to be worth in my eyes (e.g. testing that calculator returns 4 for 2+2). But perhaps this is a rather hard program to test, as it probably relies on real hardware.

Yes, this specific program downloads tests from the internet (about 1 MB or more) and run a benchmark which uses 100% CPU on one core for several minutes. Nothing you want to run on a CI.

So i have choosen this dummy command to test if basic functionality is given.

For other programs i would test at least one common task.

Do you see a difference in testing with this automated tests and testing manual in a real terminal?

[7c6f434c]

I would say that the application builds but does not execute after update is pretty small.

In general, when packaging, I think «builds and fails to load a library on start» is more typical than «builds and runs but doesn't work even on simplest examples»…

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review-may-2019/3032/108

[davidak]

All remarks are resolved so this is ready to merge.

There were no further comments, so this seems to be fine.

I already reviewed 2 updates where this would have been helpful.

[nixos-discourse]

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/how-many-people-are-paid-to-work-on-nix-nixpkgs/8307/51