diff options
-rw-r--r-- | doc/manual/rl-next/pytest-suite.md | 10 | ||||
-rw-r--r-- | meson.build | 1 | ||||
-rw-r--r-- | package.nix | 8 | ||||
-rw-r--r-- | src/libstore/build/derivation-goal.cc | 1 | ||||
-rw-r--r-- | src/libstore/build/worker.cc | 2 | ||||
-rw-r--r-- | src/libstore/machines.cc | 2 | ||||
-rw-r--r-- | src/libstore/ssh-store.cc | 8 | ||||
-rw-r--r-- | tests/functional/dependencies.nix | 27 | ||||
-rw-r--r-- | tests/functional/hash-check.nix | 21 | ||||
-rw-r--r-- | tests/functional2/README.md | 24 | ||||
-rw-r--r-- | tests/functional2/__init__.py | 0 | ||||
-rw-r--r-- | tests/functional2/conftest.py | 8 | ||||
-rw-r--r-- | tests/functional2/meson.build | 29 | ||||
-rw-r--r-- | tests/functional2/test_eval_trivial.py | 4 | ||||
-rw-r--r-- | tests/functional2/testlib/__init__.py | 0 | ||||
-rw-r--r-- | tests/functional2/testlib/fixtures.py | 121 | ||||
-rw-r--r-- | tests/nixos/remote-builds-ssh-ng.nix | 3 |
17 files changed, 232 insertions, 37 deletions
diff --git a/doc/manual/rl-next/pytest-suite.md b/doc/manual/rl-next/pytest-suite.md new file mode 100644 index 000000000..f4dbda1e8 --- /dev/null +++ b/doc/manual/rl-next/pytest-suite.md @@ -0,0 +1,10 @@ +--- +synopsis: "The beginnings of a new pytest-based functional test suite" +category: Development +cls: [2036, 2037] +credits: jade +--- + +The existing integration/functional test suite is based on a large volume of shell scripts. +This often makes it somewhat challenging to debug at the best of times. +The goal of the pytest test suite is to make tests have more obvious dependencies on files and to make tests more concise and easier to write, as well as making new testing methods like snapshot testing easy. diff --git a/meson.build b/meson.build index 2cf86e985..7a5f28d6b 100644 --- a/meson.build +++ b/meson.build @@ -607,6 +607,7 @@ endif if enable_tests subdir('tests/unit') subdir('tests/functional') + subdir('tests/functional2') endif subdir('meson/clang-tidy') diff --git a/package.nix b/package.nix index ff862bf92..2d485be93 100644 --- a/package.nix +++ b/package.nix @@ -170,6 +170,7 @@ let functionalTestFiles = fileset.unions [ ./tests/functional + ./tests/functional2 ./tests/unit (fileset.fileFilter (f: lib.strings.hasPrefix "nix-profile" f.name) ./scripts) ]; @@ -243,6 +244,8 @@ stdenv.mkDerivation (finalAttrs: { nativeBuildInputs = [ python3 + python3.pkgs.pytest + python3.pkgs.pytest-xdist meson ninja cmake @@ -474,6 +477,11 @@ stdenv.mkDerivation (finalAttrs: { pythonPackages = ( p: [ + # FIXME: these have to be added twice due to the nix shell using a + # wrapped python instead of build inputs for its python inputs + p.pytest + p.pytest-xdist + p.yapf p.python-frontmatter p.requests diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7f72efa6a..96140e10b 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1211,6 +1211,7 @@ HookReply DerivationGoal::tryBuildHook() else { s += "\n"; writeLogsToStderr(s); + logger->log(lvlInfo, s); } } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index d1c1abdf8..10f58f5d3 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -239,7 +239,7 @@ Worker::Results Worker::run(std::function<Targets (GoalFactory &)> req) auto onInterrupt = kj::newPromiseAndCrossThreadFulfiller<Result<Results>>(); auto interruptCallback = createInterruptCallback([&] { - return result::failure(std::make_exception_ptr(makeInterrupted())); + onInterrupt.fulfiller->fulfill(result::failure(std::make_exception_ptr(makeInterrupted()))); }); auto promise = runImpl(std::move(topGoals)) diff --git a/src/libstore/machines.cc b/src/libstore/machines.cc index d0897b81f..7314e3177 100644 --- a/src/libstore/machines.cc +++ b/src/libstore/machines.cc @@ -68,11 +68,11 @@ ref<Store> Machine::openStore() const { Store::Params storeParams; if (storeUri.starts_with("ssh://")) { + storeParams["log-fd"] = "4"; storeParams["max-connections"] = "1"; } if (storeUri.starts_with("ssh://") || storeUri.starts_with("ssh-ng://")) { - storeParams["log-fd"] = "4"; if (sshKey != "") storeParams["ssh-key"] = sshKey; if (sshPublicHostKey != "") diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index fb60326c1..5c1fc0c1f 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -30,11 +30,6 @@ struct SSHStoreConfig : virtual RemoteStoreConfig, virtual CommonSSHStoreConfig class SSHStore : public virtual SSHStoreConfig, public virtual RemoteStore { public: - // Hack for getting remote build log output. - // Intentionally not in `SSHStoreConfig` so that it doesn't appear in - // the documentation - const Setting<int> logFD{(StoreConfig*) this, -1, "log-fd", "file descriptor to which SSH's stderr is connected"}; - SSHStore(const std::string & scheme, const std::string & host, const Params & params) : StoreConfig(params) , RemoteStoreConfig(params) @@ -49,8 +44,7 @@ public: sshPublicHostKey, // Use SSH master only if using more than 1 connection. connections->capacity() > 1, - compress, - logFD) + compress) { } diff --git a/tests/functional/dependencies.nix b/tests/functional/dependencies.nix index be1a7ae9a..0ede76b71 100644 --- a/tests/functional/dependencies.nix +++ b/tests/functional/dependencies.nix @@ -1,8 +1,7 @@ { hashInvalidator ? "" }: with import ./config.nix; -let { - +let input0 = mkDerivation { name = "dependencies-input-0"; buildCommand = "mkdir $out; echo foo > $out/bar"; @@ -32,17 +31,15 @@ let { outputHashAlgo = "sha256"; outputHash = "1dq9p0hnm1y75q2x40fws5887bq1r840hzdxak0a9djbwvx0b16d"; }; - - body = mkDerivation { - name = "dependencies-top"; - builder = ./dependencies.builder0.sh + "/FOOBAR/../."; - input1 = input1 + "/."; - input2 = "${input2}/."; - input1_drv = input1; - input2_drv = input2; - input0_drv = input0; - fod_input_drv = fod_input; - meta.description = "Random test package"; - }; - +in +mkDerivation { + name = "dependencies-top"; + builder = ./dependencies.builder0.sh + "/FOOBAR/../."; + input1 = input1 + "/."; + input2 = "${input2}/."; + input1_drv = input1; + input2_drv = input2; + input0_drv = input0; + fod_input_drv = fod_input; + meta.description = "Random test package"; } diff --git a/tests/functional/hash-check.nix b/tests/functional/hash-check.nix index f029f0cc9..6095bc57f 100644 --- a/tests/functional/hash-check.nix +++ b/tests/functional/hash-check.nix @@ -1,5 +1,4 @@ -let { - +let input1 = derivation { name = "dependencies-input-1"; system = "i086-msdos"; @@ -16,14 +15,12 @@ let { outputHashAlgo = "md5"; outputHash = "ffffffffffffffffffffffffffffffff"; }; - - body = derivation { - name = "dependencies"; - system = "i086-msdos"; - builder = "/bar/sh"; - args = ["-e" "-x" (./dummy + "/FOOBAR/../.")]; - input1 = input1 + "/."; - inherit input2; - }; - +in +derivation { + name = "dependencies"; + system = "i086-msdos"; + builder = "/bar/sh"; + args = ["-e" "-x" (./dummy + "/FOOBAR/../.")]; + input1 = input1 + "/."; + inherit input2; } diff --git a/tests/functional2/README.md b/tests/functional2/README.md new file mode 100644 index 000000000..b0440fb3a --- /dev/null +++ b/tests/functional2/README.md @@ -0,0 +1,24 @@ +# functional2 tests + +This uncreatively named test suite is a Pytest based replacement for the shell framework used to write traditional Nix integration tests. +Its primary goal is to make tests more concise, more self-contained, easier to write, and to produce better errors. + +## Goals + +- Eliminate implicit dependencies on files in the test directory as well as the requirement to copy the test files to the build directory as is currently hacked in the other functional test suite. + - You should be able to write a DirectoryTree of files for your test declaratively. +- Reduce the amount of global environment state being thrown around in the test suite. +- Make tests very concise and easy to reuse code for, and hopefully turn more of what is currently code into data. + - Provide rich ways of calling `nix` with pleasant syntax. + +## TODO: Intended features + +- [ ] Expect tests ([pytest-expect-test]) or snapshot tests ([pytest-insta]) or, likely, both! + We::jade prefer to have short output written in-line as it makes it greatly easier to read the tests, but pytest-expect doesn't allow for putting larger stuff in external files, so something else is necessary for those. +- [ ] Web server fixture: we don't test our network functionality because background processes are hard and this is simply goofy. + We could just test it. +- [ ] Nix daemon fixture. +- [x] Parallelism via pytest-xdist. + +[pytest-expect-test]: https://pypi.org/project/pytest-expect-test/ +[pytest-insta]: https://pypi.org/project/pytest-insta/ diff --git a/tests/functional2/__init__.py b/tests/functional2/__init__.py new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/functional2/__init__.py diff --git a/tests/functional2/conftest.py b/tests/functional2/conftest.py new file mode 100644 index 000000000..406f97b75 --- /dev/null +++ b/tests/functional2/conftest.py @@ -0,0 +1,8 @@ +import pytest +from pathlib import Path +from .testlib import fixtures + + +@pytest.fixture +def nix(tmp_path: Path): + return fixtures.Nix(tmp_path) diff --git a/tests/functional2/meson.build b/tests/functional2/meson.build new file mode 100644 index 000000000..b18d7035d --- /dev/null +++ b/tests/functional2/meson.build @@ -0,0 +1,29 @@ +xdist_opts = [ + # auto number of workers, max 12 jobs + '-n', 'auto', '--maxprocesses=12', + # group tests by module or class; ensures that any setup work occurs as little as possible + '--dist=loadscope', +] + +# surprisingly, this actually works even if PATH is set to something before +# meson gets hold of it. neat! +functional2_env = environment() +functional2_env.prepend('PATH', bindir) + +test( + 'functional2', + python, + args : [ + '-m', 'pytest', + '-v', + xdist_opts, + meson.current_source_dir() + ], + env : functional2_env, + # FIXME: Although we can trivially use TAP here with pytest-tap, due to a meson bug, it is unusable. + # (failure output does not get displayed to the console. at all. someone should go fix it): + # https://github.com/mesonbuild/meson/issues/11185 + # protocol : 'tap', + suite : 'installcheck', + timeout : 300, +) diff --git a/tests/functional2/test_eval_trivial.py b/tests/functional2/test_eval_trivial.py new file mode 100644 index 000000000..8bde9d22c --- /dev/null +++ b/tests/functional2/test_eval_trivial.py @@ -0,0 +1,4 @@ +from .testlib.fixtures import Nix + +def test_trivial_addition(nix: Nix): + assert nix.eval('1 + 1').json() == 2 diff --git a/tests/functional2/testlib/__init__.py b/tests/functional2/testlib/__init__.py new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/tests/functional2/testlib/__init__.py diff --git a/tests/functional2/testlib/fixtures.py b/tests/functional2/testlib/fixtures.py new file mode 100644 index 000000000..bbaaae51d --- /dev/null +++ b/tests/functional2/testlib/fixtures.py @@ -0,0 +1,121 @@ +import os +import json +import subprocess +from typing import Any +from pathlib import Path +import dataclasses + + +@dataclasses.dataclass +class CommandResult: + cmd: list[str] + rc: int + """Return code""" + stderr: bytes + """Outputted stderr""" + stdout: bytes + """Outputted stdout""" + + def ok(self): + if self.rc != 0: + raise subprocess.CalledProcessError(returncode=self.rc, + cmd=self.cmd, + stderr=self.stderr, + output=self.stdout) + return self + + def json(self) -> Any: + self.ok() + return json.loads(self.stdout) + + +@dataclasses.dataclass +class NixSettings: + """Settings for invoking Nix""" + experimental_features: set[str] | None = None + + def feature(self, *names: str): + self.experimental_features = (self.experimental_features + or set()) | set(names) + return self + + def to_config(self) -> str: + config = '' + + def serialise(value): + if type(value) in {str, int}: + return str(value) + elif type(value) in {list, set}: + return ' '.join(str(e) for e in value) + else: + raise ValueError( + f'Value is unsupported in nix config: {value!r}') + + def field_may(name, value, serialiser=serialise): + nonlocal config + if value is not None: + config += f'{name} = {serialiser(value)}\n' + + field_may('experimental-features', self.experimental_features) + return config + + +@dataclasses.dataclass +class Nix: + test_root: Path + + def hermetic_env(self): + # mirroring vars-and-functions.sh + home = self.test_root / 'test-home' + home.mkdir(parents=True, exist_ok=True) + return { + 'NIX_STORE_DIR': self.test_root / 'store', + 'NIX_LOCALSTATE_DIR': self.test_root / 'var', + 'NIX_LOG_DIR': self.test_root / 'var/log/nix', + 'NIX_STATE_DIR': self.test_root / 'var/nix', + 'NIX_CONF_DIR': self.test_root / 'etc', + 'NIX_DAEMON_SOCKET_PATH': self.test_root / 'daemon-socket', + 'NIX_USER_CONF_FILES': '', + 'HOME': home, + } + + def make_env(self): + # We conservatively assume that people might want to successfully get + # some env through to the subprocess, so we override whatever is in the + # global env. + d = os.environ.copy() + d.update(self.hermetic_env()) + return d + + def call(self, cmd: list[str], extra_env: dict[str, str] = {}): + """ + Calls a process in the test environment. + """ + env = self.make_env() + env.update(extra_env) + proc = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=self.test_root, + env=env, + ) + (stdout, stderr) = proc.communicate() + rc = proc.returncode + return CommandResult(cmd=cmd, rc=rc, stdout=stdout, stderr=stderr) + + def nix(self, + cmd: list[str], + settings: NixSettings = NixSettings(), + extra_env: dict[str, str] = {}): + extra_env = extra_env.copy() + extra_env.update({'NIX_CONFIG': settings.to_config()}) + return self.call(['nix', *cmd], extra_env) + + def eval( + self, expr: str, + settings: NixSettings = NixSettings()) -> CommandResult: + # clone due to reference-shenanigans + settings = dataclasses.replace(settings).feature('nix-command') + + return self.nix(['eval', '--json', '--expr', expr], settings=settings) diff --git a/tests/nixos/remote-builds-ssh-ng.nix b/tests/nixos/remote-builds-ssh-ng.nix index 8deb9a504..ec12f9066 100644 --- a/tests/nixos/remote-builds-ssh-ng.nix +++ b/tests/nixos/remote-builds-ssh-ng.nix @@ -97,7 +97,8 @@ in builder.wait_for_unit("sshd.service") out = client.fail("nix-build ${expr nodes.client 1} 2>&1") - assert "error: failed to start SSH connection to 'root@builder': Host key verification failed" in out, f"No host verification error in {out}" + assert "Host key verification failed." in out, f"No host verification error:\n{out}" + assert "warning: SSH to 'root@builder' failed, stdout first line: '''" in out, f"No details about which host:\n{out}" client.succeed(f"ssh -o StrictHostKeyChecking=no {builder.name} 'echo hello world' >&2") |