From 850e50cd3f9ce83e4e2d73e48a84f496c27b31b3 Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Tue, 23 Jan 2024 16:19:01 +0200
Subject: [PATCH] feat(tvix/eval/tests): compare .xml outputs

In case a .exp.xml file is provided alongside the test, compare its
output with the desired state.

Also, add some function .exp.xml that were presumably moved out of the
way back to src/tests/nix_tests, as they now produce the correct XML
output.

Change-Id: Ibd8123f3e6ed7bae3a44407d2284a2b2c8ce9a28
Reviewed-on: https://cl.tvl.fyi/c/depot/+/10687
Reviewed-by: sterni <sternenseemann@systemli.org>
Autosubmit: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Reviewed-by: tazjin <tazjin@tvl.su>
---
 tvix/eval/src/tests/mod.rs                    | 49 +++++++++++--------
 .../eval-okay-closure.exp.xml                 |  0
 .../eval-okay-functionargs.exp.xml            |  0
 .../eval-okay-functionargs.nix                |  0
 4 files changed, 29 insertions(+), 20 deletions(-)
 rename tvix/eval/src/tests/nix_tests/{notyetpassing => }/eval-okay-closure.exp.xml (100%)
 rename tvix/eval/src/tests/nix_tests/{notyetpassing => }/eval-okay-functionargs.exp.xml (100%)
 rename tvix/eval/src/tests/nix_tests/{notyetpassing => }/eval-okay-functionargs.nix (100%)

diff --git a/tvix/eval/src/tests/mod.rs b/tvix/eval/src/tests/mod.rs
index 856707119..5a7708e29 100644
--- a/tvix/eval/src/tests/mod.rs
+++ b/tvix/eval/src/tests/mod.rs
@@ -48,17 +48,9 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
         "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 = crate::Evaluation::new_impure();
     eval.strict = true;
     eval.builtins.extend(mock_builtins::builtins());
@@ -79,37 +71,54 @@ fn eval_test(code_path: PathBuf, expect_success: bool) {
     if !expect_success && failed {
         return;
     }
+    // !expect_success can also mean the output differs, so don't panic if the
+    // evaluation didn't fail.
 
     let value = result.value.unwrap();
     let result_str = value.to_string();
 
-    if let Ok(exp) = std::fs::read_to_string(exp_path) {
+    let exp_path = code_path.with_extension("exp");
+    if exp_path.exists() {
+        // If there's an .exp file provided alongside, compare it with the
+        // output of the NixValue .to_string() method.
+        let exp_str = std::fs::read_to_string(&exp_path).expect("unable to read .exp file");
+
         if expect_success {
             assert_eq!(
                 result_str,
-                exp.trim(),
+                exp_str.trim(),
                 "{}: result value representation (left) must match expectation (right)",
                 code_path.display()
             );
         } else {
             assert_ne!(
                 result_str,
-                exp.trim(),
+                exp_str.trim(),
                 "{}: test passed unexpectedly!  consider moving it out of notyetpassing",
                 code_path.display()
             );
+
+            // Early return here, we don't compare .xml outputs if this is a !
+            // expect_success test.
+            return;
         }
-    } else if expect_success {
-        panic!(
-            "{}: should be able to read test expectation",
+    }
+
+    let exp_xml_path = code_path.with_extension("exp.xml");
+    if exp_xml_path.exists() {
+        // If there's an XML file provided alongside, compare it with the
+        // output produced when serializing the Value as XML.
+        let exp_xml_str = std::fs::read_to_string(exp_xml_path).expect("unable to read .xml file");
+
+        let mut xml_actual_buf = Vec::new();
+        crate::builtins::value_to_xml(&mut xml_actual_buf, &value).expect("value_to_xml failed");
+
+        assert_eq!(
+            String::from_utf8(xml_actual_buf).expect("to_xml produced invalid utf-8"),
+            exp_xml_str,
+            "{}: result value representation (left) must match expectation (right)",
             code_path.display()
         );
-    } else {
-        panic!(
-            "{}: test should have failed, but succeeded with output {}",
-            code_path.display(),
-            result_str
-        );
     }
 }
 
diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-closure.exp.xml b/tvix/eval/src/tests/nix_tests/eval-okay-closure.exp.xml
similarity index 100%
rename from tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-closure.exp.xml
rename to tvix/eval/src/tests/nix_tests/eval-okay-closure.exp.xml
diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-functionargs.exp.xml b/tvix/eval/src/tests/nix_tests/eval-okay-functionargs.exp.xml
similarity index 100%
rename from tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-functionargs.exp.xml
rename to tvix/eval/src/tests/nix_tests/eval-okay-functionargs.exp.xml
diff --git a/tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-functionargs.nix b/tvix/eval/src/tests/nix_tests/eval-okay-functionargs.nix
similarity index 100%
rename from tvix/eval/src/tests/nix_tests/notyetpassing/eval-okay-functionargs.nix
rename to tvix/eval/src/tests/nix_tests/eval-okay-functionargs.nix