Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

doc: add instructions for creating package tests #120534

Merged
merged 1 commit into from
May 1, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions doc/contributing/coding-conventions.chapter.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,3 +512,73 @@ If you do need to do create this sort of patch file, one way to do so is with gi
```ShellSession
$ git diff > nixpkgs/pkgs/the/package/0001-changes.patch
```

## Package tests {#sec-package-tests}

Tests are important to ensure quality and make reviews and automatic updates easy.

Nix package tests are a lightweight alternative to [NixOS module tests](https://nixos.org/manual/nixos/stable/#sec-nixos-tests). They can be used to create simple integration tests for packages while the module tests are used to test services or programs with a graphical user interface on a NixOS VM. Unittests that are included in the source code of a package should be executed in the `checkPhase`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

module tests are used to test services or programs with a graphical user interface on a NixOS VM

We can handle graphical test just fine using Xvfb. The NixOS tests are primarily integration tests that depend on other services or to test NixOS modules themselves. passthru.tests should really be preferred whenever possible since the NixOS tests really are heavy.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The invocation is

xvfb-run -s '-screen 0 800x600x24' dbus-run-session \
--config-file=${dbus.daemon}/share/dbus-1/session.conf \
meson test --print-errorlogs

Sometimes the dbus session part can be omitted.

can it replace https://github.com/NixOS/nixpkgs/blob/master/nixos/tests/shattered-pixel-dungeon.nix ?

Yeah, that should not be NixOS test.

But xvfb-run just creates a virtual frame buffer so that you can start graphical programs – you will still need to write your own tests or use upstream ones. We should probably create some helpers for passthru tests so that we can use tesseract like in the NixOS test.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be installCheckPhase is you want to e.g. test the final result? (IIRC that was wan of the ideas behind the NixCon 2017 talk as well).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification on that is added with #119731.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jtojnar thanks!

We should probably create some helpers for passthru tests so that we can use tesseract like in the NixOS test.

that's probably what #36842 tried, but it was rejected because it duplicated nixos tests... maybe it can be generalized somehow so it works for both


### Writing package tests {#ssec-package-tests-writing}

This is an example using the `phoronix-test-suite` package with the current best practices.

Add the tests in `passthru.tests` to the package definition like this:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i would like to link checkPhase and passthru.tests inside the document, but i don't know how and if its worth adding now when the build pipeline changes soon and it might brake

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, current level of interlinking in the manual leaves a lot to be desired.

If you just use the anchor name, it will work now and we will migrate it:

Suggested change
Add the tests in `passthru.tests` to the package definition like this:
Add the tests in [`passthru.tests`](#var-meta-tests) to the package definition like this:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rember the first time I saw passthru, I went like: "?!?!!?!? (It's not even an english word)"

Maybe one small phrase to introduce that concept here would be in order

By convention, we use the passthru attribute which allows us to store arbitrary data, ...

(If that makes sense)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@blaggacao yeah, that's true. there is reference documentation at https://nixos.org/manual/nixpkgs/stable/#sec-standard-meta-attributes


```nix
{ stdenv, lib, fetchurl, callPackage }:

stdenv.mkDerivation {

passthru.tests = {
simple-execution = callPackage ./tests.nix { };
};

meta = { … };
}
```

Create `tests.nix` in the package directory:

```nix
{ runCommand, phoronix-test-suite }:

let
inherit (phoronix-test-suite) pname version;
in

runCommand "${pname}-tests" { meta.timeout = 3; }
''
# automatic initial setup to prevent interactive questions
${phoronix-test-suite}/bin/phoronix-test-suite enterprise-setup >/dev/null
# get version of installed program and compare with package version
if [[ `${phoronix-test-suite}/bin/phoronix-test-suite version` != *"${version}"* ]]; then
echo "Error: program version does not match package version"
exit 1
fi
# run dummy command
${phoronix-test-suite}/bin/phoronix-test-suite dummy_module.dummy-command >/dev/null
# needed for Nix to register the command as successful
touch $out
''
```

### Running package tests {#ssec-package-tests-running}

davidak marked this conversation as resolved.
Show resolved Hide resolved
You can run these tests with:

```ShellSession
$ cd path/to/nixpkgs
$ nix-build -A phoronix-test-suite.tests
```

### Examples of package tests {#ssec-package-tests-examples}

Here are examples of package tests:

- [Jasmin compile test](https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/jasmin/test-assemble-hello-world/default.nix)
- [Lobster compile test](https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/compilers/lobster/test-can-run-hello-world.nix)
- [Spacy annotation test](https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/spacy/annotation-test/default.nix)
- [Libtorch test](https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/science/math/libtorch/test/default.nix)
- [Multiple tests for nanopb](https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/nanopb/default.nix)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make sense to link to a specific git revision here. Otherwise we'll risk ending up with a bunch of dead links.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense for the long-term perspective where someone reads this manual version in 20 years.

All examples are terrible and will be replaced tho.