tests: isolate flake scenarios, check eval errors against controls

Several weaknesses let checks pass although the feature under test is
broken:

- Nix restores fetched inputs from its fetcher cache, so evaluating a
  delivered flake offline after collecting garbage succeeds even if
  populate rooted none of its inputs.  nix_eval uses an empty cache
  directory.
- Scenarios share target directories and stores, so store paths and roots
  of earlier scenarios stand in for missing ones, and uploads of flakes
  never run in a passing scenario.  Scenarios of the flake, flake-input,
  and features tests are isolated now: they start without target
  directories and with a collected store on both machines.
- Phases expecting evaluation to fail pass on any evaluation failure,
  e.g. a misspelt option.  Each such phase has a control now: sources
  differing only in what makes evaluation fail, which must evaluate.

Further, populate runs with a time limit, so a hang fails its scenario
instead of the whole test, phases can run steps on the deploying machine
before and after, and evaluating the scenarios fails if the remote
target counts as local for whoever evaluates them.

3 files changed, 98 insertions, 22 deletions

diff --git a/tests/lib.sh b/tests/lib.sh
index 13b1ded..33a0c6e 100644
--- a/tests/lib.sh
+++ b/tests/lib.sh
@@ -19,14 +19,21 @@ nix() {
 }
 
 # Evaluates a flake output without network access to the fixtures, so all
-# inputs must be in the store.
+# inputs must be in the store.  An empty cache directory keeps Nix from
+# restoring inputs from its fetcher cache.
 nix_eval() {
+  nix_eval_cache=$(mktemp -d)
+  nix_eval_status=0
   env \
+      XDG_CACHE_HOME="$nix_eval_cache" \
       http_proxy=http://127.0.0.1:9 https_proxy=http://127.0.0.1:9 \
       HTTP_PROXY=http://127.0.0.1:9 HTTPS_PROXY=http://127.0.0.1:9 \
       all_proxy=http://127.0.0.1:9 ALL_PROXY=http://127.0.0.1:9 \
       no_proxy= NO_PROXY= \
-    nix --extra-experimental-features 'nix-command flakes' eval --raw "$@"
+    nix --extra-experimental-features 'nix-command flakes' eval --raw "$@" ||
+    nix_eval_status=$?
+  rm -fR "$nix_eval_cache"
+  return "$nix_eval_status"
 }
 
 # Evaluates a flake output, fetching missing inputs.
diff --git a/tests/scenarios.nix b/tests/scenarios.nix
index ce356b0..d8081d3 100644
--- a/tests/scenarios.nix
+++ b/tests/scenarios.nix
@@ -62,37 +62,57 @@ let
   alt = { alt.file = "/fixtures/alt"; };
 
   # Builds a scenario from a name, a target kind, and phases with sources.
-  scenario = { name, targetKind, node ? null, prepare ? "", phases }: let
+  # Isolated scenarios start with no target directories and a collected store
+  # on both machines, so no other scenario's store paths or roots stand in for
+  # missing ones.
+  scenario = {
+    name, targetKind, node ? null, isolate ? false,
+    prepare ? "", prepareDeployer ? null, phases,
+  }: let
     path = "/var/krops/${name}";
     target = targets.${targetKind} path;
     vars = "T=${path}\n";
+    # A phase expecting evaluation to fail has a control: sources that
+    # differ only in what makes evaluation fail, or a function returning them
+    # for the phase's sources, and that must evaluate.  So the phase doesn't
+    # pass for failing for an unrelated reason, e.g. a misspelt option.
     phase = {
       sources, prune ? false, deploy ? null,
-      expect ? "success", check ? "", mutate ? null,
+      expect ? "success", control ? null,
+      check ? "", mutate ? null, checkDeployer ? null, mutateDeployer ? null,
     }: let
-      args = {
-        source = krops.evalSource [ sources ];
-        inherit prune target;
-        force = true;
-      };
-      command =
+      command = sources: let
+        args = {
+          source = krops.evalSource [ sources ];
+          inherit prune target;
+          force = true;
+        };
+      in
         if deploy == null then populate args
         else writeDeploy "deploy" (args // { useNixOutputMonitor = false; } // deploy);
-      evaluated = builtins.tryEval (builtins.seq (toString command) true);
+      evaluates = sources:
+        (builtins.tryEval (builtins.seq (toString (command sources)) true)).success;
+      script = text: if text == null then null else prelude + vars + text;
     in
       if expect == "eval-error" then
-        if evaluated.success then
+        if control == null then
+          throw "scenario ${name}: expecting evaluation to fail needs a control"
+        else if evaluates sources then
           throw "scenario ${name}: populate evaluates, but is expected to fail"
+        else if !evaluates (if lib.isFunction control then control sources else control) then
+          throw "scenario ${name}: the control doesn't evaluate either"
         else
           null
       else {
-        populate = toString command;
+        populate = toString (command sources);
         expect = if expect == "success" then null else expect.fail;
-        check = prelude + vars + check;
-        mutate = if mutate == null then null else prelude + vars + mutate;
+        check = script check;
+        mutate = script mutate;
+        checkDeployer = script checkDeployer;
+        mutateDeployer = script mutateDeployer;
       };
   in {
-    inherit name;
+    inherit name isolate;
     node =
       if node != null then node
       else if targetKind == "local" then "deployer"
@@ -100,6 +120,8 @@ let
     prepare = prelude + vars + ''
       rm -fR "$T"
     '' + prepare;
+    prepareDeployer =
+      if prepareDeployer == null then null else prelude + vars + prepareDeployer;
     phases = lib.filter (x: x != null) (map phase phases);
   };
 
@@ -145,7 +167,19 @@ let
   copyingWithSudo = flags: ctx:
     ctx.targetKind == "sudo" && !(flags.fetchOnTarget or false);
 
-in {
+  # The sources with fetchOnTarget set for all flake and flake-input sources,
+  # the control for copying flakes to targets using sudo.
+  withFetchOnTarget = lib.mapAttrs (_: lib.mapAttrs (type: options:
+    if lib.elem type [ "flake" "flake-input" ]
+      then options // { fetchOnTarget = true; }
+      else options));
+
+in
+
+# The remote target must not count as local wherever this gets evaluated.
+assert !(krops.isLocalTarget (targets.remote "/"));
+
+{
 
   derivation = matrix "derivation" {} (_: _: {
     phases = twice {
@@ -255,6 +289,7 @@ in {
   in
     # The flake in a subdirectory has no inputs to draw from sources.
     if flags.kind == "subdir" && flags.inputs then null else {
+    isolate = true;
     phases = twice ({
       sources = {
         x.flake = {
@@ -265,6 +300,7 @@ in {
       } // optionalAttrs flags.inputs alt;
     } // (if copyingWithSudo flags ctx then {
       expect = "eval-error";
+      control = withFetchOnTarget;
     } else {
       check = if flags.kind == "subdir" then checkFlake {
         attr = "who"; expected = "sub"; subdir = true;
@@ -282,6 +318,7 @@ in {
     }) (flags: ctx: let
       inputs = flags.inputs or false;
     in {
+      isolate = true;
       phases = twice ({
         sources = {
           x.flake-input = {
@@ -293,6 +330,7 @@ in {
         } // optionalAttrs inputs alt;
       } // (if copyingWithSudo flags ctx then {
         expect = "eval-error";
+        control = withFetchOnTarget;
       } else {
         check = {
           dep = checkFlake {
@@ -472,7 +510,11 @@ in {
 
   # Features spanning several sources.
   features = concatMap (targetKind: let
-    s = name: args: scenario ({ name = "${name}-${targetKind}"; inherit targetKind; } // args);
+    s = name: args: scenario ({
+      name = "${name}-${targetKind}";
+      inherit targetKind;
+      isolate = true;
+    } // args);
     copying = targetKind != "sudo";
     fetchOnTarget = !copying;
   in [
@@ -602,18 +644,29 @@ in {
           b.file = { path = "/fixtures/alt"; inputs.foo.follows = "a"; };
         };
         expect = "eval-error";
+        control = {
+          a.file = { path = "/fixtures/alt"; inputs.foo.follows = "b"; };
+          b.file = "/fixtures/alt";
+        };
       }];
     })
     (s "self" {
       phases = [{
         sources.a.file = { path = "/fixtures/alt"; inputs.foo.follows = "a"; };
         expect = "eval-error";
+        control = alt // {
+          a.file = { path = "/fixtures/alt"; inputs.foo.follows = "alt"; };
+        };
       }];
     })
     (s "missing" {
       phases = [{
         sources.a.file = { path = "/fixtures/alt"; inputs.foo.follows = "nope"; };
         expect = "eval-error";
+        control = {
+          a.file = { path = "/fixtures/alt"; inputs.foo.follows = "nope"; };
+          nope.file = "/fixtures/alt";
+        };
       }];
     })
     (s "noflake" {
diff --git a/tests/vm.nix b/tests/vm.nix
index b46b015..78b861b 100644
--- a/tests/vm.nix
+++ b/tests/vm.nix
@@ -8,16 +8,19 @@ let
 
   sshKeys = import (pkgs.path + "/nixos/tests/ssh-keys.nix") pkgs;
 
+  script = name: text: if text == null then null else pkgs.writeText name text;
+
   # The scenarios as JSON, with scripts as files.
   plan = pkgs.writeText "krops-test-${name}.json" (builtins.toJSON (map
     (scenario: scenario // {
       prepare = pkgs.writeText "prepare" scenario.prepare;
+      prepareDeployer = script "prepare-deployer" scenario.prepareDeployer;
       phases = map
         (phase: phase // {
           check = pkgs.writeText "check" phase.check;
-          mutate =
-            if phase.mutate == null then null
-            else pkgs.writeText "mutate" phase.mutate;
+          mutate = script "mutate" phase.mutate;
+          checkDeployer = script "check-deployer" phase.checkDeployer;
+          mutateDeployer = script "mutate-deployer" phase.mutateDeployer;
         })
         scenario.phases;
     })
@@ -125,9 +128,18 @@ in pkgs.testers.runNixOSTest {
         node = nodes[scenario["node"]]
         try:
             with subtest(scenario["name"]):
+                if scenario["isolate"]:
+                    for machine in nodes.values():
+                        run(machine, "rm -fR /var/krops && nix-store --gc >/dev/null")
+                if scenario["prepareDeployer"] is not None:
+                    run(deployer, f"sh {scenario['prepareDeployer']}")
                 run(node, f"sh {scenario['prepare']}")
                 for i, phase in enumerate(scenario["phases"]):
-                    status, out = deployer.execute(f"HOME=/root {phase['populate']} 2>&1")
+                    # A hang fails the scenario, not the whole test.
+                    status, out = deployer.execute(
+                        f"HOME=/root timeout 600 {phase['populate']} 2>&1",
+                        timeout=900,
+                    )
                     if phase["expect"] is None:
                         if status != 0:
                             raise Exception(f"populate {i} fails:\n{out}")
@@ -136,8 +148,12 @@ in pkgs.testers.runNixOSTest {
                             f"populate {i} doesn't fail with {phase['expect']!r}:\n{out}"
                         )
                     run(node, f"KROPS_TEST_GC=1 sh {phase['check']}")
+                    if phase["checkDeployer"] is not None:
+                        run(deployer, f"sh {phase['checkDeployer']}")
                     if phase["mutate"] is not None:
                         run(node, f"sh {phase['mutate']}")
+                    if phase["mutateDeployer"] is not None:
+                        run(deployer, f"sh {phase['mutateDeployer']}")
         except Exception as e:
             print(f"scenario {scenario['name']} fails: {e}")
             failed.append(scenario["name"])