Since the debut of the test-driver, we didn't obtain
a race timer with the test execution to ensure that tests doesn't run beyond
a certain amount of time.
This is particularly important when you are running into hanging tests
which cannot be detected by current facilities (requires more pvpanic wiring up, QMP
API stuff, etc.).
Two easy examples:
- Some QEMU tests may get stuck in some situation and run for more than 24 hours → we default to 1 hour max.
- Some QEMU tests may panic in the wrong place, e.g. UEFI firmware or worse → end users can set a "reasonable" amount of time
And then, we should let the retry logic retest them until they succeed and adjust
their global timeouts.
Of course, this does not help with the fact that the timeout may need to be
a function of the actual busyness of the machine running the tests.
This is only one step towards increased reliability.
Fixes the problem introduced by 12b3066aae
which caused nixos/release.nix to return the wrong attributes, while
intending to only affect nixos/lib's runTest.
This also removes callTest from the test options, because callTest is
only ever invoked by all-tests.nix.
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
