feat(tvix/glue): introduce test suite for context strings r/7404

This is an additional test suite on the top of the Nix ones
for context strings matters.

It already smoked out multiple mistakes and potential bugs and non-deterministic result from the evaluator.

It uses a similar technology as the one in the tvix-eval albeit we instantiate a fully fledged evaluator
with in-memory store.

We copy the files instead of symlinking them because crates are built in
isolation, so symlinks cannot work.

Change-Id: I63ae225ce4f83c6e2c8ccd60d779c2f8eb9d08fb
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10619
Autosubmit: raitobezarius <tvl@lahfa.xyz>
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>

14 files changed, 377 insertions, 9 deletions

diff --git a/tvix/Cargo.lock b/tvix/Cargo.lock
index 12200b4fda36..13470fb1850f 100644
--- a/tvix/Cargo.lock
+++ b/tvix/Cargo.lock
@@ -3367,6 +3367,8 @@ dependencies = [
  "data-encoding",
  "lazy_static",
  "nix-compat",
+ "pretty_assertions",
+ "rstest",
  "serde",
  "serde_json",
  "sha2",
diff --git a/tvix/Cargo.nix b/tvix/Cargo.nix
index 432c83192b3b..170e53dbd22b 100644
--- a/tvix/Cargo.nix
+++ b/tvix/Cargo.nix
@@ -10648,6 +10648,14 @@ rec {
             packageId = "lazy_static";
           }
           {
+            name = "pretty_assertions";
+            packageId = "pretty_assertions";
+          }
+          {
+            name = "rstest";
+            packageId = "rstest";
+          }
+          {
             name = "tempfile";
             packageId = "tempfile";
           }
@@ -10656,7 +10664,10 @@ rec {
             packageId = "test-case";
           }
         ];
-
+        features = {
+          "default" = [ "nix_tests" ];
+        };
+        resolvedDefaultFeatures = [ "default" "nix_tests" ];
       };
       "tvix-serde" = rec {
         crateName = "tvix-serde";
diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs
index 2dceeadc7ae9..7509379fdab6 100644
--- a/tvix/eval/src/tests/mod.rs
+++ b/tvix/eval/src/tests/mod.rs
@@ -166,14 +166,6 @@ fn nix_eval_okay(#[files("src/tests/nix_tests/eval-okay-*.nix")] code_path: Path
 // notyetpassing; this makes the test suite much more useful for
 // regression testing, since there should always be zero non-ignored
 // failing tests.
-//
-// Unfortunately test_generator is unmaintained, so the PRs to make
-// it understand #[ignored] has been sitting for two years, so we
-// can't use `cargo test --include-ignored`, which is the normal way
-// of handling this situation.
-//
-//   https://github.com/frehberg/test-generator/pull/10
-//   https://github.com/frehberg/test-generator/pull/8
 #[rstest]
 fn nix_eval_okay_currently_failing(
     #[files("src/tests/nix_tests/notyetpassing/eval-okay-*.nix")] code_path: PathBuf,
diff --git a/tvix/glue/Cargo.toml b/tvix/glue/Cargo.toml
index c58c66dd60a1..451b28f1f0a0 100644
--- a/tvix/glue/Cargo.toml
+++ b/tvix/glue/Cargo.toml
@@ -25,9 +25,17 @@ git = "https://github.com/tvlfyi/wu-manber.git"
 [dev-dependencies]
 criterion = { version = "0.5", features = ["html_reports"] }
 lazy_static = "1.4.0"
+pretty_assertions = "1.4.0"
+rstest = "0.18.2"
 tempfile = "3.8.1"
 test-case = "3.3.1"
 
+[features]
+default = ["nix_tests"]
+# Enables running the Nix language test suite from the original C++
+# Nix implementation (at version 2.3) against Tvix.
+nix_tests = []
+
 [[bench]]
 name = "eval"
 harness = false
diff --git a/tvix/glue/src/lib.rs b/tvix/glue/src/lib.rs
index 2b36a0b890bb..31bcbede37d4 100644
--- a/tvix/glue/src/lib.rs
+++ b/tvix/glue/src/lib.rs
@@ -5,6 +5,9 @@ pub mod tvix_build;
 pub mod tvix_io;
 pub mod tvix_store_io;
 
+#[cfg(test)]
+mod tests;
+
 /// Tell the Evaluator to resolve `<nix>` to the path `/__corepkgs__`,
 /// which has special handling in [tvix_io::TvixIO].
 /// This is used in nixpkgs to import `fetchurl.nix` from `<nix>`.
diff --git a/tvix/glue/src/tests/mod.rs b/tvix/glue/src/tests/mod.rs
new file mode 100644
index 000000000000..9ae17c219ac9
--- /dev/null
+++ b/tvix/glue/src/tests/mod.rs
@@ -0,0 +1,140 @@
+use std::{cell::RefCell, rc::Rc, sync::Arc};
+
+use pretty_assertions::assert_eq;
+use std::path::PathBuf;
+use tvix_castore::{
+    blobservice::{BlobService, MemoryBlobService},
+    directoryservice::{DirectoryService, MemoryDirectoryService},
+};
+use tvix_eval::Value;
+use tvix_store::pathinfoservice::{MemoryPathInfoService, PathInfoService};
+
+use rstest::rstest;
+
+use crate::{
+    builtins::add_derivation_builtins, known_paths::KnownPaths, tvix_store_io::TvixStoreIO,
+};
+
+fn eval_test(code_path: PathBuf, expect_success: bool) {
+    assert_eq!(
+        code_path.extension().unwrap(),
+        "nix",
+        "test files always end in .nix"
+    );
+    let exp_path = code_path.with_extension("exp");
+    let exp_xml_path = code_path.with_extension("exp.xml");
+
+    let code = std::fs::read_to_string(&code_path).expect("should be able to read test code");
+
+    if exp_xml_path.exists() {
+        // We can't test them at the moment because we don't have XML output yet.
+        // Checking for success / failure only is a bit disingenious.
+        return;
+    }
+
+    let mut eval = tvix_eval::Evaluation::new_impure();
+
+    let blob_service = Arc::new(MemoryBlobService::default()) as Arc<dyn BlobService>;
+    let directory_service =
+        Arc::new(MemoryDirectoryService::default()) as Arc<dyn DirectoryService>;
+    let path_info_service = Box::new(MemoryPathInfoService::new(
+        blob_service.clone(),
+        directory_service.clone(),
+    )) as Box<dyn PathInfoService>;
+    let runtime = tokio::runtime::Runtime::new().unwrap();
+
+    eval.io_handle = Box::new(TvixStoreIO::new(
+        blob_service,
+        directory_service,
+        path_info_service,
+        runtime.handle().clone(),
+    ));
+
+    let known_paths: Rc<RefCell<KnownPaths>> = Default::default();
+
+    eval.strict = true;
+    add_derivation_builtins(&mut eval, known_paths.clone());
+
+    let result = eval.evaluate(code, Some(code_path.clone()));
+    let failed = match result.value {
+        Some(Value::Catchable(_)) => true,
+        _ => !result.errors.is_empty(),
+    };
+    if expect_success && failed {
+        panic!(
+            "{}: evaluation of eval-okay test should succeed, but failed with {:?}",
+            code_path.display(),
+            result.errors,
+        );
+    }
+
+    if !expect_success && failed {
+        return;
+    }
+
+    let value = result.value.unwrap();
+    let result_str = value.to_string();
+
+    if let Ok(exp) = std::fs::read_to_string(exp_path) {
+        if expect_success {
+            assert_eq!(
+                result_str,
+                exp.trim(),
+                "{}: result value representation (left) must match expectation (right)",
+                code_path.display()
+            );
+        } else {
+            assert_ne!(
+                result_str,
+                exp.trim(),
+                "{}: test passed unexpectedly!  consider moving it out of notyetpassing",
+                code_path.display()
+            );
+        }
+    } else if expect_success {
+        panic!(
+            "{}: should be able to read test expectation",
+            code_path.display()
+        );
+    } else {
+        panic!(
+            "{}: test should have failed, but succeeded with output {}",
+            code_path.display(),
+            result_str
+        );
+    }
+}
+
+// eval-okay-* tests contain a snippet of Nix code, and an expectation
+// of the produced string output of the evaluator.
+//
+// These evaluations are always supposed to succeed, i.e. all snippets
+// are guaranteed to be valid Nix code.
+#[rstest]
+fn eval_okay(#[files("src/tests/tvix_tests/eval-okay-*.nix")] code_path: PathBuf) {
+    eval_test(code_path, true)
+}
+
+// eval-okay-* tests from the original Nix test suite.
+#[cfg(feature = "nix_tests")]
+#[rstest]
+fn nix_eval_okay(#[files("src/tests/nix_tests/eval-okay-*.nix")] code_path: PathBuf) {
+    eval_test(code_path, true)
+}
+
+// eval-okay-* tests from the original Nix test suite which do not yet pass for tvix
+//
+// Eventually there will be none of these left, and this function
+// will disappear :) Until then, to run these tests, use `cargo test
+// --features expected_failures`.
+//
+// Please don't submit failing tests unless they're in
+// notyetpassing; this makes the test suite much more useful for
+// regression testing, since there should always be zero non-ignored
+// failing tests.
+#[rstest]
+fn nix_eval_okay_currently_failing(
+    #[files("src/tests/nix_tests/notyetpassing/eval-okay-*.nix")] code_path: PathBuf,
+) {
+    eval_test(code_path, false)
+}
diff --git a/tvix/glue/src/tests/nix_tests/eval-okay-context.exp b/tvix/glue/src/tests/nix_tests/eval-okay-context.exp
new file mode 100644
index 000000000000..2f535bdbc454
--- /dev/null
+++ b/tvix/glue/src/tests/nix_tests/eval-okay-context.exp
@@ -0,0 +1 @@
+"foo eval-okay-context.nix bar"
diff --git a/tvix/glue/src/tests/nix_tests/eval-okay-context.nix b/tvix/glue/src/tests/nix_tests/eval-okay-context.nix
new file mode 100644
index 000000000000..c8732118628a
--- /dev/null
+++ b/tvix/glue/src/tests/nix_tests/eval-okay-context.nix
@@ -0,0 +1,6 @@
+let s = "foo ${builtins.substring 33 100 (baseNameOf "${./eval-okay-context.nix}")} bar";
+in
+if s != "foo eval-okay-context.nix bar"
+then abort "context not discarded"
+else builtins.unsafeDiscardStringContext s
+
diff --git a/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.exp b/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.exp
new file mode 100644
index 000000000000..03b400cc8862
--- /dev/null
+++ b/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.exp
@@ -0,0 +1 @@
+[ true true true true true true ]
diff --git a/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.nix b/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.nix
new file mode 100644
index 000000000000..354376b89535
--- /dev/null
+++ b/tvix/glue/src/tests/nix_tests/notyetpassing/eval-okay-context-introspection.nix
@@ -0,0 +1,42 @@
+let
+  drv = derivation {
+    name = "fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+
+  path = "${./eval-okay-context-introspection.nix}";
+
+  desired-context = {
+    "${builtins.unsafeDiscardStringContext path}" = {
+      path = true;
+    };
+    "${builtins.unsafeDiscardStringContext drv.drvPath}" = {
+      outputs = [ "foo" "out" ];
+      allOutputs = true;
+    };
+  };
+
+  combo-path = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}";
+  legit-context = builtins.getContext combo-path;
+
+  reconstructed-path = builtins.appendContext
+    (builtins.unsafeDiscardStringContext combo-path)
+    desired-context;
+
+  # Eta rule for strings with context.
+  etaRule = str:
+    str == builtins.appendContext
+      (builtins.unsafeDiscardStringContext str)
+      (builtins.getContext str);
+
+in
+[
+  (legit-context == desired-context)
+  (reconstructed-path == combo-path)
+  (etaRule "foo")
+  (etaRule drv.drvPath)
+  (etaRule drv.foo.outPath)
+  (etaRule (builtins.unsafeDiscardOutputDependency drv.drvPath))
+]
diff --git a/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.exp b/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.exp
new file mode 100644
index 000000000000..3f8905e9bd0d
--- /dev/null
+++ b/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.exp
@@ -0,0 +1 @@
+[ true true true true true true true true true true true ]
diff --git a/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.nix b/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.nix
new file mode 100644
index 000000000000..ed78112ab7eb
--- /dev/null
+++ b/tvix/glue/src/tests/tvix_tests/eval-okay-context-introspection.nix
@@ -0,0 +1,72 @@
+# Contrary to the Nix tests, this one does not make any use of `builtins.appendContext`
+# It's a weaker yet interesting test by abusing knowledge on how does our builtins
+# performs propagation.
+let
+  drv = derivation {
+    name = "fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+
+  # `substr` propagates context, we truncate to an empty string and concatenate to the target
+  # to infect it with the context of `copied`.
+  appendContextFrom = copied: target: (builtins.substring 0 0 "${copied}") + "${target}";
+
+  # `split` discards (!!) contexts, we first split by `/` (there's at least one such `/` by
+  # virtue of `target` being a store path, i.e. starting with `$store_root/$derivation_name`)
+  # then, we reassemble the list into a proper string.
+  discardContext = target: builtins.concatStringsSep "" (builtins.split "(.*)" "${target}");
+
+  hasContextInAttrKeys = attrs: builtins.all builtins.hasContext (builtins.attrNames attrs);
+
+  path = "${./eval-okay-context-introspection.nix}";
+
+  # This is a context-less attribute set, which should be exactly the same
+  # as `builtins.getContext combo-path`.
+  desired-context = {
+    "${builtins.unsafeDiscardStringContext path}" = {
+      path = true;
+    };
+    "${builtins.unsafeDiscardStringContext drv.drvPath}" = {
+      outputs = [ "foo" "out" ];
+      allOutputs = true;
+    };
+  };
+
+  combo-path = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}";
+  legit-context = builtins.getContext combo-path;
+
+  reconstructed-path = appendContextFrom combo-path
+    (builtins.unsafeDiscardStringContext combo-path);
+
+  # Eta rule for strings with context.
+  etaRule = str:
+    str == appendContextFrom
+      str
+      (builtins.unsafeDiscardStringContext str);
+
+  etaRule' = str:
+    str == appendContextFrom
+      str
+      (discardContext str);
+
+in
+[
+  (!hasContextInAttrKeys desired-context)
+  (legit-context."${builtins.unsafeDiscardStringContext path}".path)
+  (legit-context."${builtins.unsafeDiscardStringContext drv.drvPath}".outputs == [ "foo" "out" ])
+  # `allOutputs` is present only on DrvClosure-style context string, i.e. the
+  # context string of a drvPath itself, not an outPath.
+  (!builtins.hasAttr "allOutputs" (builtins.getContext drv.outPath)."${builtins.unsafeDiscardStringContext drv.drvPath}")
+  (builtins.hasAttr "allOutputs" legit-context."${builtins.unsafeDiscardStringContext drv.drvPath}")
+  (builtins.hasAttr "allOutputs" (builtins.getContext drv.drvPath)."${builtins.unsafeDiscardStringContext drv.drvPath}")
+  (legit-context == desired-context) # FIXME(raitobezarius): this should not use `builtins.seq`, this is a consequence of excessive laziness of Tvix, I believe.
+  (reconstructed-path == combo-path)
+  # Those are too slow?
+  # (etaRule' "foo")
+  # (etaRule' combo-path)
+  (etaRule "foo")
+  (etaRule drv.drvPath)
+  (etaRule drv.foo.outPath)
+]
diff --git a/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.exp b/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.exp
new file mode 100644
index 000000000000..71c6ded40612
--- /dev/null
+++ b/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.exp
@@ -0,0 +1 @@
+[ true true true true true true true true true true true true true true true true true true true true true true true true true true ]
diff --git a/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.nix b/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.nix
new file mode 100644
index 000000000000..67f0ac46729b
--- /dev/null
+++ b/tvix/glue/src/tests/tvix_tests/eval-okay-context-propagation.nix
@@ -0,0 +1,88 @@
+# We test various propagation of contexts under other builtins here.
+let
+  drv = derivation {
+    name = "fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "foo" ];
+  };
+  other-drv = derivation {
+    name = "other-fail";
+    builder = "/bin/false";
+    system = "x86_64-linux";
+    outputs = [ "out" "bar" ];
+  };
+
+  # `substr` propagates context, we truncate to an empty string and concatenate to the target
+  # to infect it with the context of `copied`.
+  appendContextFrom = copied: target: (builtins.substring 0 0 copied) + target;
+
+  path = "${./eval-okay-context-introspection.nix}";
+
+  combo-path = "${path}${drv.outPath}${drv.foo.outPath}${drv.drvPath}";
+
+  mergeContext = a: b:
+    builtins.getContext a // builtins.getContext b;
+
+  preserveContext = origin: result:
+    builtins.getContext "${result}" == builtins.getContext "${origin}";
+
+  preserveContexts = origins: result:
+    let union = builtins.foldl' (x: y: x // y) { } (builtins.map (d: builtins.getContext "${d}") origins);
+    in
+    union == builtins.getContext "${result}";
+in
+[
+  # `toFile` should produce context.
+  (builtins.hasContext "${(builtins.toFile "myself" "${./eval-okay-context-introspection.nix}")}")
+  # `derivation` should produce context.
+  (builtins.hasContext "${drv}")
+  # Low-level test to ensure that interpolation is working as expected.
+  (builtins.length (builtins.attrNames (builtins.getContext "${drv}${other-drv}")) == 2)
+  (builtins.getContext "${drv}${other-drv}" == mergeContext drv other-drv)
+  # Those three next tests are extremely related.
+  # To test interpolation, we need concatenation to be working and vice versa.
+  # In addition, we need `builtins.substring` empty string propagation to attach context
+  # in absence of `builtins.appendContext`.
+  # The previous test should ensure that we don't test vacuous truths.
+  # Substring preserves contexts.
+  (preserveContext combo-path (builtins.substring 0 0 combo-path)) # <- FIXME: broken
+  # Interpolation preserves contexts.
+  (preserveContext "${drv}${other-drv}" (appendContextFrom drv other-drv))
+  # Concatenation preserves contexts.
+  (preserveContext "${drv}${other-drv}" (drv + other-drv))
+  # Special case when Nix does not assert that the length argument is non-negative
+  # when the starting index is ≥ than the string's length.
+  # FIXME: those three are broken too, NON DETERMINISTIC!!!
+  (preserveContext combo-path (builtins.substring 5 (-5) (builtins.substring 0 0 combo-path)))
+  (preserveContext combo-path (toString combo-path))
+  # No replacement should yield at least the same context.
+  (preserveContext combo-path (builtins.replaceStrings [ ] [ ] combo-path))
+  # This is an idempotent replacement, it should yield therefore to full preservation of the context.
+  (preserveContext "${drv}${drv}" (builtins.replaceStrings [ "${drv}" ] [ "${drv}" ] "${drv}"))
+  # There's no context here, so no context should appear from `drv`.
+  (preserveContext "abc" (builtins.replaceStrings [ "${drv}" ] [ "${drv}" ] "abc"))
+  # Context should appear by a successful replacement.
+  (preserveContext "${drv}" (builtins.replaceStrings [ "a" ] [ "${drv}" ] "a"))
+  # We test multiple successful replacements.
+  (preserveContexts [ drv other-drv ] (builtins.replaceStrings [ "a" "b" ] [ "${drv}" "${other-drv}" ] "ab"))
+  # We test *empty* string replacements.
+  (preserveContext "${drv}" (builtins.replaceStrings [ "" ] [ "${drv}" ] "abc"))
+  (preserveContext "${drv}" (builtins.replaceStrings [ "" ] [ "${drv}" ] ""))
+  # There should be no context in a parsed derivation name.
+  (!builtins.any builtins.hasContext (builtins.attrValues (builtins.parseDrvName "${drv.name}")))
+  # Nix does not propagate contexts for `match`.
+  (!builtins.any builtins.hasContext (builtins.match "(.*)" "${drv}"))
+  # `dirOf` preserves contexts of non-paths.
+  (preserveContext "${drv}" (builtins.dirOf "${drv}"))
+  (preserveContext "abc" (builtins.dirOf "abc"))
+  # `baseNameOf propagates context of argument
+  (preserveContext "${drv}" (builtins.baseNameOf drv))
+  (preserveContext "abc" (builtins.baseNameOf "abc"))
+  # `concatStringsSep` preserves contexts of both arguments.
+  (preserveContexts [ drv other-drv ] (builtins.concatStringsSep "${other-drv}" (map toString [ drv drv drv drv drv ])))
+  (preserveContext drv (builtins.concatStringsSep "|" (map toString [ drv drv drv drv drv ])))
+  (preserveContext other-drv (builtins.concatStringsSep "${other-drv}" [ "abc" "def" ]))
+  # `attrNames` will never ever produce context.
+  (preserveContext "abc" (toString (builtins.attrNames { a = { }; b = { }; c = { }; })))
+]