This is a decomposition of the testing-python.nix and build-vms.nix
files into modules.
By refactoring the glue, we accomplish the following:
- NixOS tests can now use `imports` and other module system features.
- Network-wide test setup can now be reusable; example:
- A setup with all VMs configured to use a DNS server
- Split long, slow tests into multiple tests that import a
common module that has most of the setup.
- Type checking for the test arguments
- (TBD) "generated" options reference docs
- Aspects that had to be wired through all the glue are now in their
own files.
- Chief example: interactive.nix.
- Also: network.nix
In rewriting this, I've generally stuck as close as possible to the
existing code; copying pieces of logic and rewiring them, without
changing the logic itself.
I've made two exceptions to this rule
- Introduction of `extraDriverArgs` instead of hardcoded
interactivity logic.
- Incorporation of https://github.com/NixOS/nixpkgs/pull/144110
in testScript.nix.
I might revert the latter and split it into a new commit.
The involved test was nixosTests.nextcloud.basic21.
It has a testScript that is strict in nodes.nextcloud.config.system.build.vm,
in assertions about imagemagick being in the system closure.
The recursion was introduced in 329a4461a7 from
https://github.com/NixOS/nixpkgs/pull/140792
Make sure the all derivations referenced by the test script are
available on the nodes. Accessing these derivations works just fine
without this change when using 9p to mount the host's store, but when
an image is built (virtualisation.buildRootImage), the dependencies
need to be copied to the image. We don't want to copy the script
itself, though, since that would trigger unnecessary image rebuilds.
This commit encapsulates the involved domain into classes and
defines explicit and typed arguments where untyped dicts where used.
It preserves backwards compatibility through legacy wrappers.
The current name is misleading: it doesn't contain cli arguments,
but several constants and utility functions related to qemu.
This commit also removes the use of `with import ...` for clarity.
Previous to this commit, the entire test driver environment was shared
with the actual python test environment.
This is a hefty api surface. This commit selectively exposes only those
symbols to the test environment that are actually meant to be used by
tests.
This is the case when the test-script is empty. `nixos-build-vms(8)` is
primarily supposed to be used as tool to test changes or to reproduce
bugs (IMHO) where "just spinning up a few VMs" is the primary use-case.
In the ongoing discussion about these changes[1] it was suggested to
only expose it when needed (i.e. in the case I described above) to keep
the API surface as slim as possible.
[1] https://github.com/NixOS/nixpkgs/pull/133675#discussion_r688112485
Previously the driver was configured exclusively through convoluted
environment variables.
Now the driver's defaults are configured through env variables.
Some additional concerns are in the github comments of this PR.
the use of python further restricts possible RFC1035 host labels since
dash is not allowed for use in python identifiers.
The previous implementation of this check was flawed, since it did not
check the `hostName` value that is actually used to construe the
identifier, but the node name, which can be anything, e.g. just `machine`.
The previous implementation, by further restricting RFC1035 labels, only
for the sake of testing seems to be an unacceptable restriction and should
be addressed separately.
Apparently this looks like it was forgotten when doing commit
3884ff70ba, which refactored the test
runner and driver a bit.
The passthru argument actually was correctly reintroduced in
setupDriverForTest, but the actual makeTest function didn't use it.
This fixes the nixpkgs tarball job, which previously failed with:
attribute 'elkPackages' missing, at /build/source/pkgs/tools/misc/logstash/6.x.nix:58:30
Signed-off-by: aszlig <aszlig@nix.build>
Acked-by: David Arnold <dar@xoe.solutions>
Fixes: https://github.com/NixOS/nixpkgs/issues/127274
Merges: https://github.com/NixOS/nixpkgs/pull/127346
Less nesting, where that improves readability. More nesteing, where
that improves readability, but most importantly:
Expose individual functions separately so that they can be more easily
built directly, eg.:
`nix build --impure --expr '(import ./testing-python.nix {system = builtins.currentSystem;}).mkTestDriver'`
At nixpkgs root:
`rg redirectSerial ./` does not result in any other match
nor does
`rg USE_SERIAL ./` except for an unrelated match in:
pkgs/tools/graphics/argyllcms/default.nix
When importing Nixpkgs within Nixpkgs, we should not consider aliases
to ensure we don't rely on them internally.
There are probably more places that need to be converted.
Our test driver exposes a bunch of variables and functions, which
pyflakes doesn't recognise by default because it assumes that the test
script is executed standalone. In reality however the test driver script
is using exec() on the testScript.
Fortunately pyflakes has $PYFLAKES_BUILTINS, which are the attributes
that are globally available on all modules to be checked. Since we only
have one module, using this environment variable is fine as opposed to
my first approach to this, which tried to use the unstable internal API
of pyflakes.
The attributes are gathered by the main derivation of the test driver,
because we don't want to end up defining a new attribute in the test
driver module just to being confused why using it in a test will result
in an error.
Another way we could have gathered these attributes would be in
mkDriver, which is where the linting takes place. However, we do have a
different set of Python dependencies in scope and duplicating these will
again just cause confusion over having it at one location only.
Signed-off-by: aszlig <aszlig@nix.build>
Co-Authored-By: aszlig <aszlig@nix.build>
So far, we have used "black" for formatting the test code, which is
rather strict and opinionated and when used inline in Nix expressions it
creates all sorts of trouble.
One of the main annoyances is that when using strings coming from Nix
expressions (eg. store paths or option definitions from NixOS modules),
completely unrelated changes could cause tests to fail, since eg. black
wants lines to be broken.
Another downside of enforcing a certain kind of formatting is that it
makes the Nix expression code inconsistent because we're mixing two
spaces of indentation (common in nixpkgs) with four spaces of
indentation as defined in PEP-8. While this is perfectly fine for
standalone Python files, it really looks ugly and inconsistent IMO when
used within Nix strings.
What we actually want though is a linter that catches problems early on
before actually running the test, because this is *actually* helping in
development because running the actual VM test takes much longer.
This is the reason why I switched from black to pyflakes, because the
latter actually has useful checks, eg. usage of undefined variables,
invalid format arguments, duplicate arguments, shadowed loop vars and
more.
Signed-off-by: aszlig <aszlig@nix.build>
Closes: https://github.com/NixOS/nixpkgs/issues/72964
The hydra tarball step would fail due to the nodes attribute not being
properly inherited. Since we can't execute all the tests and release
steps locally anymore (thanks to the JSONification and faster hydra
eval) these errors will probably keep in appearing.
This is hopefully the last of those introduced by me test runner
refactoring.
Error was seen on hydra (https://hydra.nixos.org/build/129282411):
> unpacking sources
> unpacking source archive /nix/store/bp95x52h6nv3j8apxrryyj2rviw682k1-source
> source root is source
> patching sources
> autoconfPhase
> No bootstrap, bootstrap.sh, configure.in or configure.ac. Assuming this is not an GNU Autotools package.
> configuring
> release name is nixpkgs-21.03pre249116.1088f059401
> git-revision is 1088f05940
> building
> no Makefile, doing nothing
> running tests
> warning: you did not specify '--add-root'; the result might be removed by the garbage collector
> warning: you did not specify '--add-root'; the result might be removed by the garbage collector
> checking Nixpkgs on i686-linux
> checking Nixpkgs on x86_64-linux
> checking Nixpkgs on x86_64-darwin
> checking eval-release.nix
> trace: `mkStrict' is obsolete; use `mkOverride 0' instead.
> trace: `lib.nixpkgsVersion` is deprecated, use `lib.version` instead!
> trace: warning: lib.readPathsFromFile is deprecated, use a list instead
> trace: Warning: `showVal` is deprecated and will be removed in the next release, please use `traceSeqN`
> trace: lib.zip is deprecated, use lib.zipAttrsWith instead
> checking find-tarballs.nix
> trace: `mkStrict' is obsolete; use `mkOverride 0' instead.
> trace: `lib.nixpkgsVersion` is deprecated, use `lib.version` instead!
> trace: warning: lib.readPathsFromFile is deprecated, use a list instead
> trace: Warning: `showVal` is deprecated and will be removed in the next release, please use `traceSeqN`
> trace: lib.zip is deprecated, use lib.zipAttrsWith instead
> error: while evaluating anonymous function at /build/source/maintainers/scripts/find-tarballs.nix:6:1, called from undefined position:
> while evaluating 'operator' at /build/source/maintainers/scripts/find-tarballs.nix:27:16, called from undefined position:
> while evaluating 'immediateDependenciesOf' at /build/source/maintainers/scripts/find-tarballs.nix:39:29, called from /build/source/maintainers/scripts/find-tarballs.nix:27:44:
> while evaluating anonymous function at /build/source/lib/attrsets.nix:234:10, called from undefined position:
> while evaluating anonymous function at /build/source/maintainers/scripts/find-tarballs.nix:40:37, called from /build/source/lib/attrsets.nix:234:16:
> while evaluating 'derivationsIn' at /build/source/maintainers/scripts/find-tarballs.nix:42:19, called from /build/source/maintainers/scripts/find-tarballs.nix:40:40:
> while evaluating 'canEval' at /build/source/maintainers/scripts/find-tarballs.nix:48:13, called from /build/source/maintainers/scripts/find-tarballs.nix:43:9:
> while evaluating the attribute 'nodes' at /build/source/nixos/lib/testing-python.nix:195:23:
> attribute 'nodes' missing, at /build/source/nixos/lib/testing-python.nix:193:16
> build time elapsed: 0m0.122s 0m0.043s 17m51.526s 0m56.668s
> builder for '/nix/store/96rk3c74vrk6m3snm7n6jhis3j640pn4-nixpkgs-tarball-21.03pre249116.1088f059401.drv' failed with exit code 1
In 5500dc8 we introduced the --keep-vm-state flag and defaulted to that
flag not being set. This lead to the `runInMachine` tests not longer
working and that going unnoticed for quite some time now.
Previously you would be able to override only the QEMU package to be
used in the test runner. Frankly that doesn't help a lot if you are
trying to get a graphical session. The graphical session requires the
option in the NixOS module system to bet set to the correct QEMU
package.
In this commit I moved most of the test node configuration and
transformations into the `mkDriver` function (previously called
`driver`). The motivation was to be able to create a `driver` instance
with a given QEMU package that will be used consistently througout the
test expression.