diff options
author | Ryan Lahfa <tvl@lahfa.xyz> | 2024-01-14T00·41+0100 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-01-17T17·31+0000 |
commit | f71bb351d29f58935e6002615cd94d6e6259bf26 (patch) | |
tree | 168311dd206ec4524db0105d8109e0ba4d698ec7 /tvix/glue/src | |
parent | 75cc52ddb136e66b1a79117425fb35f80dcecc07 (diff) |
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>
Diffstat (limited to 'tvix/glue/src')
10 files changed, 355 insertions, 0 deletions
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 = { }; }))) +] |