Merge pull request #122201 (black -> pyflakes)

This switches the linting of the NixOS test driver script from Black
(which is a code *formatter*) to PyFlakes. Contrary to Black, which only
does formatting and a basic syntax check, PyFlakes actually performs
useful checks early on before spinning up VMs and evaluating the actual
test script and thus becomes actually useful in development rather than
an annoyance.

One of the reasons why Black has been an annoyance[1] was because it
assumed that the files that it's formatting aren't inlined inside
another programming language.

With NixOS VM tests however, we inline these Python scripts in the
testScript attribute. With some of them using string antiquotations,
things are getting rather ugly because Black now no longer formats
static code but generated code from Nix being used as a macro language.

This becomes especially annoying when an antiquotation contains an
option definition from the NixOS module system, since an unrelated
change might change its length or contents (eg. suddenly containing a
double quote) for which Black will report an error.

While issue #72964 has been sitting around for a while (and probably
annoyed everyone involved), nobody has actually proposed an
implementation until @roberth did a first pull request[2] yesterday
which added a "skipFormatter" attribute, which contrary to skipLint
silently disabled Black.

This has led to very legitimate opposition[3] from @flokli:

> As of now, this only adds an option that does exactly the same as the
> already existing one.
>
> black does more than linting, yes. Last September it was proposed to
> switch from black to to a more permissive (only-)linter.
>
> I don't think adding another option (skipFormatter) that currently
> does exactly the same as skipLint will help out of this confusion.
>
> IMHO, we should keep skipLint, but use a linter that doesn't format,
> at least not enforce the line length (due to the nix interpolation we
> do).

This was written while I was doing an alternative implementation and
pretty much sums up the work I'm merging here, which switches to
PyFlakes, which only checks for various errors in the code (eg.
undefined variables, shadowing, wrong comparisons and more) but does not
do *any* formatting.

Since Black didn't do any of the checks performed by PyFlakes (except a
basic syntax check), the existing test scripts needed to be fixed.

Thanks to @blaggacao, @Ma27 and @roberth for helping with testing and
fixing those scripts.

[1]: #72964
[2]: #122197
[3]: #122197 (review)

nixos/lib/testing-python.nix

@@ -32,6 +32,14 @@ rec {
 
       preferLocalBuild = true;
 
+      buildPhase = ''
+        python <<EOF
+        from pydoc import importfile
+        with open('driver-exports', 'w') as fp:
+          fp.write(','.join(dir(importfile('${testDriverScript}'))))
+        EOF
+      '';
+
       doCheck = true;
       checkPhase = ''
         mypy --disallow-untyped-defs \
@@ -50,6 +58,8 @@ rec {
 
           wrapProgram $out/bin/nixos-test-driver \
             --prefix PATH : "${lib.makeBinPath [ qemu_pkg vde2 netpbm coreutils ]}" \
+
+          install -m 0644 -vD driver-exports $out/nix-support/driver-exports
         '';
     };
 
@@ -73,7 +83,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;
     };
@@ -159,7 +171,10 @@ rec {
 
             echo -n "$testScript" > $out/test-script
             ${lib.optionalString (!skipLint) ''
-              ${python3Packages.black}/bin/black --check --diff $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/
@@ -193,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 ''

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
 

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

nixos/tests/containers-custom-pkgs.nix

@@ -30,5 +30,5 @@ in {
   };
 
   # This test only consists of evaluating the test machine
-  testScript = "";
+  testScript = "pass";
 })

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")
     '';
 })

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.

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(

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)
       '';
     };

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")
 

nixos/tests/printing.nix

@@ -50,7 +50,6 @@ in {
   testScript = ''
     import os
     import re
-    import sys
 
     start_all()
 

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")

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")]

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)

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}")