diff --git a/nixos/doc/manual/development/developing-the-test-driver.chapter.md b/nixos/doc/manual/development/developing-the-test-driver.chapter.md index 4b70fe00af4..d64574fa62a 100644 --- a/nixos/doc/manual/development/developing-the-test-driver.chapter.md +++ b/nixos/doc/manual/development/developing-the-test-driver.chapter.md @@ -25,6 +25,8 @@ These include `pkgs.nixosTest`, `testing-python.nix` and `make-test-python.nix`. ## Testing changes to the test framework {#sec-test-the-test-framework} +We currently have limited unit tests for the framework itself. You may run these with `nix-build -A nixosTests.nixos-test-driver`. + When making significant changes to the test framework, we run the tests on Hydra, to avoid disrupting the larger NixOS project. For this, we use the `python-test-refactoring` branch in the `NixOS/nixpkgs` repository, and its [corresponding Hydra jobset](https://hydra.nixos.org/jobset/nixos/python-test-refactoring). diff --git a/nixos/doc/manual/development/writing-nixos-tests.section.md b/nixos/doc/manual/development/writing-nixos-tests.section.md index 3de46fda3df..486a4b64a26 100644 --- a/nixos/doc/manual/development/writing-nixos-tests.section.md +++ b/nixos/doc/manual/development/writing-nixos-tests.section.md @@ -130,6 +130,11 @@ starting them in parallel: start_all() ``` +If the hostname of a node contains characters that can't be used in a +Python variable name, those characters will be replaced with +underscores in the variable name, so `nodes.machine-a` will be exposed +to Python as `machine_a`. + ## Machine objects {#ssec-machine-objects} The following methods are available on machine objects: diff --git a/nixos/lib/test-driver/test_driver/driver.py b/nixos/lib/test-driver/test_driver/driver.py index ad52f365737..ea6ba4b65b5 100644 --- a/nixos/lib/test-driver/test_driver/driver.py +++ b/nixos/lib/test-driver/test_driver/driver.py @@ -2,6 +2,7 @@ from contextlib import contextmanager from pathlib import Path from typing import Any, Dict, Iterator, List, Union, Optional, Callable, ContextManager import os +import re import tempfile from test_driver.logger import rootlog @@ -28,6 +29,10 @@ def get_tmp_dir() -> Path: return tmp_dir +def pythonize_name(name: str) -> str: + return re.sub(r"^[^A-z_]|[^A-z0-9_]", "_", name) + + class Driver: """A handle to the driver that sets up the environment and runs the tests""" @@ -113,7 +118,7 @@ class Driver: polling_condition=self.polling_condition, Machine=Machine, # for typing ) - machine_symbols = {m.name: m for m in self.machines} + machine_symbols = {pythonize_name(m.name): m for m in self.machines} # If there's exactly one machine, make it available under the name # "machine", even if it's not called that. if len(self.machines) == 1: diff --git a/nixos/lib/testing/driver.nix b/nixos/lib/testing/driver.nix index fb181c1d7e9..25759a91dda 100644 --- a/nixos/lib/testing/driver.nix +++ b/nixos/lib/testing/driver.nix @@ -21,29 +21,20 @@ let in nodesList ++ lib.optional (lib.length nodesList == 1 && !lib.elem "machine" nodesList) "machine"; - # TODO: This is an implementation error and needs fixing - # the testing famework cannot legitimately restrict hostnames further - # beyond RFC1035 - invalidNodeNames = lib.filter - (node: builtins.match "^[A-z_]([A-z0-9_]+)?$" node == null) - nodeHostNames; + pythonizeName = name: + let + head = lib.substring 0 1 name; + tail = lib.substring 1 (-1) name; + in + (if builtins.match "[A-z_]" head == null then "_" else head) + + lib.stringAsChars (c: if builtins.match "[A-z0-9_]" c == null then "_" else c) tail; uniqueVlans = lib.unique (builtins.concatLists vlans); vlanNames = map (i: "vlan${toString i}: VLan;") uniqueVlans; - machineNames = map (name: "${name}: Machine;") nodeHostNames; + pythonizedNames = map pythonizeName nodeHostNames; + machineNames = map (name: "${name}: Machine;") pythonizedNames; - withChecks = - if lib.length invalidNodeNames > 0 then - throw '' - Cannot create machines out of (${lib.concatStringsSep ", " invalidNodeNames})! - All machines are referenced as python variables in the testing framework which will break the - script when special characters are used. - - This is an IMPLEMENTATION ERROR and needs to be fixed. Meanwhile, - please stick to alphanumeric chars and underscores as separation. - '' - else - lib.warnIf config.skipLint "Linting is disabled"; + withChecks = lib.warnIf config.skipLint "Linting is disabled"; driver = hostPkgs.runCommand "nixos-test-driver-${config.name}" @@ -87,7 +78,7 @@ let ${testDriver}/bin/generate-driver-symbols ${lib.optionalString (!config.skipLint) '' PYFLAKES_BUILTINS="$( - echo -n ${lib.escapeShellArg (lib.concatStringsSep "," nodeHostNames)}, + echo -n ${lib.escapeShellArg (lib.concatStringsSep "," pythonizedNames)}, < ${lib.escapeShellArg "driver-symbols"} )" ${hostPkgs.python3Packages.pyflakes}/bin/pyflakes $out/test-script ''} diff --git a/nixos/tests/all-tests.nix b/nixos/tests/all-tests.nix index 79058ae41f4..461678cdefe 100644 --- a/nixos/tests/all-tests.nix +++ b/nixos/tests/all-tests.nix @@ -66,6 +66,15 @@ let ; in { + + # Testing the test driver + nixos-test-driver = { + extra-python-packages = handleTest ./nixos-test-driver/extra-python-packages.nix {}; + node-name = runTest ./nixos-test-driver/node-name.nix; + }; + + # NixOS vm tests and non-vm unit tests + _3proxy = runTest ./3proxy.nix; aaaaxy = runTest ./aaaaxy.nix; acme = runTest ./acme.nix; @@ -220,7 +229,6 @@ in { etcd-cluster = handleTestOn ["x86_64-linux"] ./etcd-cluster.nix {}; etebase-server = handleTest ./etebase-server.nix {}; etesync-dav = handleTest ./etesync-dav.nix {}; - extra-python-packages = handleTest ./extra-python-packages.nix {}; evcc = handleTest ./evcc.nix {}; fancontrol = handleTest ./fancontrol.nix {}; fcitx5 = handleTest ./fcitx5 {}; diff --git a/nixos/tests/extra-python-packages.nix b/nixos/tests/nixos-test-driver/extra-python-packages.nix similarity index 84% rename from nixos/tests/extra-python-packages.nix rename to nixos/tests/nixos-test-driver/extra-python-packages.nix index 7a48077cf98..1146bedd996 100644 --- a/nixos/tests/extra-python-packages.nix +++ b/nixos/tests/nixos-test-driver/extra-python-packages.nix @@ -1,4 +1,4 @@ -import ./make-test-python.nix ({ ... }: +import ../make-test-python.nix ({ ... }: { name = "extra-python-packages"; diff --git a/nixos/tests/nixos-test-driver/node-name.nix b/nixos/tests/nixos-test-driver/node-name.nix new file mode 100644 index 00000000000..31386813a51 --- /dev/null +++ b/nixos/tests/nixos-test-driver/node-name.nix @@ -0,0 +1,33 @@ +{ + name = "nixos-test-driver.node-name"; + nodes = { + "ok" = { }; + + # Valid node name, but not a great host name. + "one_two" = { }; + + # Valid node name, good host name + "a-b" = { }; + + # TODO: would be nice to test these eval failures + # Not allowed by lib/testing/network.nix (yet?) + # "foo.bar" = { }; + # Not allowed. + # "not ok" = { }; # not ok + }; + + testScript = '' + start_all() + + with subtest("python vars exist and machines are reachable through test backdoor"): + ok.succeed("true") + one_two.succeed("true") + a_b.succeed("true") + + with subtest("hostname is derived from the node name"): + ok.succeed("hostname | tee /dev/stderr | grep '^ok$'") + one_two.succeed("hostname | tee /dev/stderr | grep '^onetwo$'") + a_b.succeed("hostname | tee /dev/stderr | grep '^a-b$'") + + ''; +}