From c362a28fcf2621fd3b6d6a96c821e9af9d123e21 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 16:57:02 +0200 Subject: [PATCH 01/16] nixos/testing: Switch from black to pyflakes 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 Closes: https://github.com/NixOS/nixpkgs/issues/72964 --- nixos/lib/testing-python.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/lib/testing-python.nix b/nixos/lib/testing-python.nix index 6497b897eafd3a..6327f7a10a3a43 100644 --- a/nixos/lib/testing-python.nix +++ b/nixos/lib/testing-python.nix @@ -159,7 +159,7 @@ rec { echo -n "$testScript" > $out/test-script ${lib.optionalString (!skipLint) '' - ${python3Packages.black}/bin/black --check --diff $out/test-script + ${python3Packages.pyflakes}/bin/pyflakes $out/test-script ''} ln -s ${testDriver}/bin/nixos-test-driver $out/bin/ From 71087b2bc4e3d78a44c8d562be63fc91c0acbefa Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 8 May 2021 17:24:47 +0200 Subject: [PATCH 02/16] nixos/testing-python.nix: Expose driver (cherry picked from commit a2c9220568648b4528154ebd8e657add243ed0b4) --- nixos/lib/testing-python.nix | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/nixos/lib/testing-python.nix b/nixos/lib/testing-python.nix index 6327f7a10a3a43..fde52ba4fc1fe9 100644 --- a/nixos/lib/testing-python.nix +++ b/nixos/lib/testing-python.nix @@ -73,7 +73,9 @@ rec { LOGFILE=/dev/null tests='exec(os.environ["testScript"])' ${driver}/bin/nixos-test-driver ''; - passthru = driver.passthru; + passthru = driver.passthru // { + inherit driver; + }; inherit pos; }; From 56d9637119ef4f05be20887ed09b2a2b760e6dae Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 8 May 2021 17:52:22 +0200 Subject: [PATCH 03/16] nixos/testing: Set up scope for testScript linter 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 Co-Authored-By: aszlig --- nixos/lib/testing-python.nix | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/nixos/lib/testing-python.nix b/nixos/lib/testing-python.nix index fde52ba4fc1fe9..679c31f3e35461 100644 --- a/nixos/lib/testing-python.nix +++ b/nixos/lib/testing-python.nix @@ -32,6 +32,14 @@ rec { preferLocalBuild = true; + buildPhase = '' + python < $out/test-script ${lib.optionalString (!skipLint) '' - ${python3Packages.pyflakes}/bin/pyflakes $out/test-script + PYFLAKES_BUILTINS="$( + echo -n ${lib.escapeShellArg (lib.concatStringsSep "," nodeHostNames)}, + < ${lib.escapeShellArg "${testDriver}/nix-support/driver-exports"} + )" ${python3Packages.pyflakes}/bin/pyflakes $out/test-script ''} ln -s ${testDriver}/bin/nixos-test-driver $out/bin/ @@ -195,6 +208,8 @@ rec { (node: builtins.match "^[A-z_]([A-z0-9_]+)?$" node == null) nodeNames; + nodeHostNames = map (c: c.config.system.name) (lib.attrValues driver.nodes); + in if lib.length invalidNodeNames > 0 then throw '' From 06b070ffe7f4f24a2a92c22b0696517b247cd862 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 8 May 2021 18:38:00 +0200 Subject: [PATCH 04/16] nixosTests.acme: lint --- nixos/tests/acme.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/tests/acme.nix b/nixos/tests/acme.nix index 99dd8ec6fd3c1c..6f98b0da37805d 100644 --- a/nixos/tests/acme.nix +++ b/nixos/tests/acme.nix @@ -245,7 +245,7 @@ in import ./make-test-python.nix ({ lib, ... }: { ) for line in subject_data.lower().split("\n"): if "subject" in line: - print(f"First subject in fullchain.pem: ", line) + print(f"First subject in fullchain.pem: {line}") assert cert_name.lower() in line return From b9e7fb14e2ea9a7047e719f65aa1b465ec472829 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 8 May 2021 22:57:44 +0200 Subject: [PATCH 05/16] nixos/tests/nfs: lint --- nixos/tests/nfs/kerberos.nix | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/nixos/tests/nfs/kerberos.nix b/nixos/tests/nfs/kerberos.nix index 75d1210496b003..5684131f671b13 100644 --- a/nixos/tests/nfs/kerberos.nix +++ b/nixos/tests/nfs/kerberos.nix @@ -88,8 +88,8 @@ in "kdb5_util create -s -r NFS.TEST -P master_key", "systemctl restart kadmind.service kdc.service", ) - server.wait_for_unit(f"kadmind.service") - server.wait_for_unit(f"kdc.service") + server.wait_for_unit("kadmind.service") + server.wait_for_unit("kdc.service") # create principals server.succeed( @@ -102,8 +102,8 @@ in # add principals to server keytab server.succeed("kadmin.local ktadd nfs/server.nfs.test") server.succeed("systemctl start rpc-gssd.service rpc-svcgssd.service") - server.wait_for_unit(f"rpc-gssd.service") - server.wait_for_unit(f"rpc-svcgssd.service") + server.wait_for_unit("rpc-gssd.service") + server.wait_for_unit("rpc-svcgssd.service") client.wait_for_unit("network-online.target") From 774aba102a842c60c635fdc583ba871cf1dea8cf Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 8 May 2021 22:58:41 +0200 Subject: [PATCH 06/16] nixosTests.chromium: lint Note: I didn't execute it entirely because I'd have to build chromium for this, but the diff appears fine. --- nixos/tests/chromium.nix | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nixos/tests/chromium.nix b/nixos/tests/chromium.nix index 8429d932ae694c..60ecf986d6eed4 100644 --- a/nixos/tests/chromium.nix +++ b/nixos/tests/chromium.nix @@ -54,7 +54,8 @@ mapAttrs (channel: chromiumPkg: makeTest rec { in "${pkgs.xdotool}/bin/xdotool ${xdoScript}"; in '' import shlex - from contextlib import contextmanager, _GeneratorContextManager + import re + from contextlib import contextmanager # Run as user alice From fc76a44d0f9cc43a496b0bb4afb5b628b8fb250e Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 8 May 2021 22:59:12 +0200 Subject: [PATCH 07/16] nixosTests.containers-custom-pkgs: lint The new linter basically does def testScript # ... before calling `pyflakes`. As this test-script is empty, it would lead to a syntax-error unless `pass` is added. --- nixos/tests/containers-custom-pkgs.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/tests/containers-custom-pkgs.nix b/nixos/tests/containers-custom-pkgs.nix index c050e49bc29d2d..1627a2c70c3ca9 100644 --- a/nixos/tests/containers-custom-pkgs.nix +++ b/nixos/tests/containers-custom-pkgs.nix @@ -30,5 +30,5 @@ in { }; # This test only consists of evaluating the test machine - testScript = ""; + testScript = "pass"; }) From b4b5dcb66947cf195fd05978cae0eb9d3a708199 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 8 May 2021 23:01:59 +0200 Subject: [PATCH 08/16] nixosTests.containers-imperative: lint --- nixos/tests/containers-imperative.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/tests/containers-imperative.nix b/nixos/tests/containers-imperative.nix index bb207165a02afc..1dcccfc306a353 100644 --- a/nixos/tests/containers-imperative.nix +++ b/nixos/tests/containers-imperative.nix @@ -162,6 +162,6 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: { machine.fail( "nixos-container create b0rk --config-file ${brokenCfg}" ) - machine.succeed(f"test ! -e /var/lib/containers/b0rk") + machine.succeed("test ! -e /var/lib/containers/b0rk") ''; }) From b782440a62879f1ea9cfe3b9be4475de924c9a51 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 8 May 2021 23:02:08 +0200 Subject: [PATCH 09/16] nixosTests.custom-ca: lint --- nixos/tests/custom-ca.nix | 1 + 1 file changed, 1 insertion(+) diff --git a/nixos/tests/custom-ca.nix b/nixos/tests/custom-ca.nix index 31909188d3a789..7ce1101911db0b 100644 --- a/nixos/tests/custom-ca.nix +++ b/nixos/tests/custom-ca.nix @@ -112,6 +112,7 @@ in }; testScript = '' + from typing import Tuple def execute_as(user: str, cmd: str) -> Tuple[int, str]: """ Run a shell command as a specific user. From 62a518b904a235709b30293a08caffa202b7cbb2 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 23:25:20 +0200 Subject: [PATCH 10/16] nixos/tests/yggdrasil: Fix linting error Linter error was: f-string is missing placeholders Signed-off-by: aszlig --- nixos/tests/yggdrasil.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nixos/tests/yggdrasil.nix b/nixos/tests/yggdrasil.nix index 0b58ad29aa2b12..0e75ed54db2812 100644 --- a/nixos/tests/yggdrasil.nix +++ b/nixos/tests/yggdrasil.nix @@ -147,7 +147,7 @@ in import ./make-test-python.nix ({ pkgs, ...} : { # If Alice can talk to Carol, then Bob's outbound peering and Carol's # local peering have succeeded and everybody is connected. alice.wait_until_succeeds(f"ping -c 1 {carol_ip6}") - alice.succeed(f"ping -c 1 ${bobIp6}") + alice.succeed("ping -c 1 ${bobIp6}") bob.succeed("ping -c 1 ${aliceIp6}") bob.succeed(f"ping -c 1 {carol_ip6}") From c066cc3c0b1f7f3c4b7e11ef330ce0980440273a Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 23:36:39 +0200 Subject: [PATCH 11/16] nixos/tests/networking: Fix str literal comparison Linter error: use ==/!= to compare constant literals (str, bytes, int, float, tuple) Signed-off-by: aszlig --- nixos/tests/networking.nix | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nixos/tests/networking.nix b/nixos/tests/networking.nix index 1ea61f99a95139..c8756207f27b6f 100644 --- a/nixos/tests/networking.nix +++ b/nixos/tests/networking.nix @@ -511,7 +511,7 @@ let machine.sleep(10) residue = machine.succeed("ip tuntap list") assert ( - residue is "" + residue == "" ), "Some virtual interface has not been properly cleaned:\n{}".format(residue) ''; }; @@ -665,10 +665,10 @@ let ipv4Residue = machine.succeed("ip -4 route list dev eth0 | head -n-3").strip() ipv6Residue = machine.succeed("ip -6 route list dev eth0 | head -n-3").strip() assert ( - ipv4Residue is "" + ipv4Residue == "" ), "The IPv4 routing table has not been properly cleaned:\n{}".format(ipv4Residue) assert ( - ipv6Residue is "" + ipv6Residue == "" ), "The IPv6 routing table has not been properly cleaned:\n{}".format(ipv6Residue) ''; }; From e157ad41cb2df25b21a2d648f519caa0dc6dc712 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 23:41:39 +0200 Subject: [PATCH 12/16] nixos/tests/printing: Remove unused 'sys' import Signed-off-by: aszlig --- nixos/tests/printing.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/nixos/tests/printing.nix b/nixos/tests/printing.nix index 6a1801fb288408..b2f2540fb7a138 100644 --- a/nixos/tests/printing.nix +++ b/nixos/tests/printing.nix @@ -50,7 +50,6 @@ in { testScript = '' import os import re - import sys start_all() From 6c0ec527b98de827e4dcb868498409b8e2cee1f3 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 23:46:07 +0200 Subject: [PATCH 13/16] nixos/tests/shadow: Fix linting errors Linter errors reported: 6:32 f-string is missing placeholders 7:26 f-string is missing placeholders 8:32 f-string is missing placeholders 30:32 f-string is missing placeholders 31:26 f-string is missing placeholders 32:32 f-string is missing placeholders 48:32 f-string is missing placeholders 49:26 f-string is missing placeholders 50:32 f-string is missing placeholders 76:32 f-string is missing placeholders 77:26 f-string is missing placeholders 78:32 f-string is missing placeholders Signed-off-by: aszlig --- nixos/tests/shadow.nix | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/nixos/tests/shadow.nix b/nixos/tests/shadow.nix index c51961e1fc68b4..dd2a575b1935a8 100644 --- a/nixos/tests/shadow.nix +++ b/nixos/tests/shadow.nix @@ -36,9 +36,9 @@ in import ./make-test-python.nix ({ pkgs, ... }: { with subtest("Normal login"): shadow.send_key("alt-f2") - shadow.wait_until_succeeds(f"[ $(fgconsole) = 2 ]") - shadow.wait_for_unit(f"getty@tty2.service") - shadow.wait_until_succeeds(f"pgrep -f 'agetty.*tty2'") + shadow.wait_until_succeeds("[ $(fgconsole) = 2 ]") + shadow.wait_for_unit("getty@tty2.service") + shadow.wait_until_succeeds("pgrep -f 'agetty.*tty2'") shadow.wait_until_tty_matches(2, "login: ") shadow.send_chars("emma\n") shadow.wait_until_tty_matches(2, "login: emma") @@ -60,9 +60,9 @@ in import ./make-test-python.nix ({ pkgs, ... }: { with subtest("Change password"): shadow.send_key("alt-f3") - shadow.wait_until_succeeds(f"[ $(fgconsole) = 3 ]") - shadow.wait_for_unit(f"getty@tty3.service") - shadow.wait_until_succeeds(f"pgrep -f 'agetty.*tty3'") + shadow.wait_until_succeeds("[ $(fgconsole) = 3 ]") + shadow.wait_for_unit("getty@tty3.service") + shadow.wait_until_succeeds("pgrep -f 'agetty.*tty3'") shadow.wait_until_tty_matches(3, "login: ") shadow.send_chars("emma\n") shadow.wait_until_tty_matches(3, "login: emma") @@ -78,9 +78,9 @@ in import ./make-test-python.nix ({ pkgs, ... }: { shadow.send_chars("${password3}\n") shadow.sleep(2) shadow.send_key("alt-f4") - shadow.wait_until_succeeds(f"[ $(fgconsole) = 4 ]") - shadow.wait_for_unit(f"getty@tty4.service") - shadow.wait_until_succeeds(f"pgrep -f 'agetty.*tty4'") + shadow.wait_until_succeeds("[ $(fgconsole) = 4 ]") + shadow.wait_for_unit("getty@tty4.service") + shadow.wait_until_succeeds("pgrep -f 'agetty.*tty4'") shadow.wait_until_tty_matches(4, "login: ") shadow.send_chars("emma\n") shadow.wait_until_tty_matches(4, "login: emma") @@ -106,9 +106,9 @@ in import ./make-test-python.nix ({ pkgs, ... }: { with subtest("nologin shell"): shadow.send_key("alt-f5") - shadow.wait_until_succeeds(f"[ $(fgconsole) = 5 ]") - shadow.wait_for_unit(f"getty@tty5.service") - shadow.wait_until_succeeds(f"pgrep -f 'agetty.*tty5'") + shadow.wait_until_succeeds("[ $(fgconsole) = 5 ]") + shadow.wait_for_unit("getty@tty5.service") + shadow.wait_until_succeeds("pgrep -f 'agetty.*tty5'") shadow.wait_until_tty_matches(5, "login: ") shadow.send_chars("layla\n") shadow.wait_until_tty_matches(5, "login: layla") From 6ad2e41269db0d68aaf92440ad44d354e9608ff0 Mon Sep 17 00:00:00 2001 From: David Arnold Date: Sat, 8 May 2021 16:53:52 -0400 Subject: [PATCH 14/16] nixos/testing: lint jellyfin test --- nixos/tests/jellyfin.nix | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/nixos/tests/jellyfin.nix b/nixos/tests/jellyfin.nix index f8c2429a7b8d80..cae31a7192582d 100644 --- a/nixos/tests/jellyfin.nix +++ b/nixos/tests/jellyfin.nix @@ -24,7 +24,6 @@ import ./make-test-python.nix ({ lib, pkgs, ... }: in '' import json - import time from urllib.parse import urlencode machine.wait_for_unit("jellyfin.service") @@ -101,7 +100,7 @@ import ./make-test-python.nix ({ lib, pkgs, ... }: def is_refreshed(_): - folders = machine.succeed(api_get(f"/Library/VirtualFolders")) + folders = machine.succeed(api_get("/Library/VirtualFolders")) folders = json.loads(folders) print(folders) return all(folder["RefreshStatus"] == "Idle" for folder in folders) @@ -141,7 +140,7 @@ import ./make-test-python.nix ({ lib, pkgs, ... }: "ffmpeg" + f" -headers 'X-Emby-Authorization:{auth_header}'" + f" -i http://localhost:8096/Videos/{video}/master.m3u8?mediaSourceId={media_source_id}" - + f" /tmp/test.mkv" + + " /tmp/test.mkv" ) duration = machine.succeed( From 74bff4e6678a90934a37ab7d912c480fd313a588 Mon Sep 17 00:00:00 2001 From: aszlig Date: Sat, 8 May 2021 23:50:44 +0200 Subject: [PATCH 15/16] nixos/tests/unbound: Remove unused 'json' import Signed-off-by: aszlig --- nixos/tests/unbound.nix | 1 - 1 file changed, 1 deletion(-) diff --git a/nixos/tests/unbound.nix b/nixos/tests/unbound.nix index e24c3ef6c99d28..fcfa222299c886 100644 --- a/nixos/tests/unbound.nix +++ b/nixos/tests/unbound.nix @@ -192,7 +192,6 @@ import ./make-test-python.nix ({ pkgs, lib, ... }: testScript = { nodes, ... }: '' import typing - import json zone = "example.local." records = [("AAAA", "abcd::eeff"), ("A", "1.2.3.4")] From 54bc69637bf2a891977dc87418e4c8cd1f71ce6f Mon Sep 17 00:00:00 2001 From: aszlig Date: Sun, 9 May 2021 00:43:41 +0200 Subject: [PATCH 16/16] nixos/test/virtualbox: Fix linting errors There were a bunch of unnecessary f-strings in there and I also removed the "# fmt: on/off" comments, because we no longer use Black and thus won't need those comments anymore. Signed-off-by: aszlig --- nixos/tests/virtualbox.nix | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/nixos/tests/virtualbox.nix b/nixos/tests/virtualbox.nix index 0a7369b0fa2aae..09314d93b7d04e 100644 --- a/nixos/tests/virtualbox.nix +++ b/nixos/tests/virtualbox.nix @@ -226,18 +226,16 @@ let def create_vm_${name}(): - # fmt: off - vbm(f"createvm --name ${name} ${createFlags}") - vbm(f"modifyvm ${name} ${vmFlags}") - vbm(f"setextradata ${name} VBoxInternal/PDM/HaltOnReset 1") - vbm(f"storagectl ${name} ${controllerFlags}") - vbm(f"storageattach ${name} ${diskFlags}") - vbm(f"sharedfolder add ${name} ${sharedFlags}") - vbm(f"sharedfolder add ${name} ${nixstoreFlags}") + vbm("createvm --name ${name} ${createFlags}") + vbm("modifyvm ${name} ${vmFlags}") + vbm("setextradata ${name} VBoxInternal/PDM/HaltOnReset 1") + vbm("storagectl ${name} ${controllerFlags}") + vbm("storageattach ${name} ${diskFlags}") + vbm("sharedfolder add ${name} ${sharedFlags}") + vbm("sharedfolder add ${name} ${nixstoreFlags}") cleanup_${name}() ${mkLog "$HOME/VirtualBox VMs/${name}/Logs/VBox.log" "HOST-${name}"} - # fmt: on def destroy_vm_${name}(): @@ -259,9 +257,7 @@ let def wait_for_ip_${name}(interface): property = f"/VirtualBox/GuestInfo/Net/{interface}/V4/IP" - # fmt: off getip = f"VBoxManage guestproperty get ${name} {property} | sed -n -e 's/^Value: //p'" - # fmt: on ip = machine.succeed( ru( @@ -394,9 +390,7 @@ let machine.wait_for_x() - # fmt: off ${mkLog "$HOME/.config/VirtualBox/VBoxSVC.log" "HOST-SVC"} - # fmt: on ${testScript} # (keep black happy)