Repository: tweag/nickel
Pull request #969 (2022-12-05 16:31:40)

[fix] Re-enable integration tests
I made a mistake when implementing #954 and gave the `integration` crate a `lib.rs` instead of  a `main.rs` which means none of the tests were actually running.

This PR fixes that, and then fixes the failures in the test case. Almost all of them were due to failures when loading Nickel files, due to the paths now being wrong. One additional failure was due to a test using an illegal metadata annotation, which I fixed by inserting the bindings into a parent record. 

Apologies for this. :pray: 

matthew-healy: I'm going to merge this as soon as the tests pass even without review, because I think it's more important that these run again. If there's any feedback after that I'll submit follow-up PRs.

Patch:
diff --git a/utilities/src/lib.rs b/utilities/src/lib.rs
index 4d26d783..05e90e44 100644
--- a/utilities/src/lib.rs
+++ b/utilities/src/lib.rs
@@ -37,7 +37,7 @@ pub fn typecheck_fixture(f: &str) -> Result<(), Error> {
 }
 
 fn program_from_test_fixture(f: &str) -> Program {
-    let path = format!("{}/../tests/{}", env!("CARGO_MANIFEST_DIR"), f);
+    let path = format!("{}/../tests/integration/{}", env!("CARGO_MANIFEST_DIR"), f);
     Program::new_from_file(&path)
         .unwrap_or_else(|e| panic!("Could not create program from `{}`\n {}", path, e))
 }


Test patch:
diff --git a/tests/integration/contracts_fail.rs b/tests/integration/contracts_fail.rs
index e73885de..f1483908 100644
--- a/tests/integration/contracts_fail.rs
+++ b/tests/integration/contracts_fail.rs
@@ -224,7 +224,7 @@ fn records_contracts_poly() {
         remove_x { x = 1 }
     "#;
     assert_matches!(
-        eval(&format!("{}", remove_sealed_field)),
+        eval(remove_sealed_field),
         Err(Error::EvalError(EvalError::FieldMissing(..)))
     );
 }
diff --git a/tests/integration/free_vars.rs b/tests/integration/free_vars.rs
index e9e347c9..a80e9f6e 100644
--- a/tests/integration/free_vars.rs
+++ b/tests/integration/free_vars.rs
@@ -1,7 +1,9 @@
+use nickel_lang::term::FieldDeps;
 use nickel_lang::{identifier::Ident, term::Term, transform::free_vars};
 
 use std::collections::{HashMap, HashSet};
 use std::iter::IntoIterator;
+use std::rc::Rc;
 
 use nickel_lang_utilities::parse;
 
@@ -9,13 +11,13 @@ use nickel_lang_utilities::parse;
 //    free_vars.into_iter().all(|id| rec_fields.contains(&id))
 // }
 
-fn free_vars_eq(free_vars: &HashSet<Ident>, expected: Vec<&str>) -> bool {
+fn free_vars_eq(free_vars: &FieldDeps, expected: Vec<&str>) -> bool {
     let expected_set: HashSet<Ident> = expected.into_iter().map(Ident::from).collect();
-    *free_vars == expected_set
+    *free_vars == FieldDeps::Known(Rc::new(expected_set))
 }
 
 fn stat_free_vars_incl(
-    stat_fields: &HashMap<Ident, HashSet<Ident>>,
+    stat_fields: &HashMap<Ident, FieldDeps>,
     mut expected: HashMap<&str, Vec<&str>>,
 ) -> bool {
     stat_fields
@@ -23,7 +25,7 @@ fn stat_free_vars_incl(
         .all(|(id, set)| free_vars_eq(set, expected.remove(id.as_ref()).unwrap()))
 }
 
-fn dyn_free_vars_incl(dyn_fields: &Vec<HashSet<Ident>>, mut expected: Vec<Vec<&str>>) -> bool {
+fn dyn_free_vars_incl(dyn_fields: &[FieldDeps], mut expected: Vec<Vec<&str>>) -> bool {
     dyn_fields
         .iter()
         .enumerate()
diff --git a/tests/integration/imports.rs b/tests/integration/imports.rs
index 6d5d7618..a489ff68 100644
--- a/tests/integration/imports.rs
+++ b/tests/integration/imports.rs
@@ -7,7 +7,7 @@ use std::path::PathBuf;
 
 fn mk_import(file: &str) -> String {
     let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    path.push(format!("tests/imports/{}", file));
+    path.push(format!("tests/integration/imports/{}", file));
     format!(
         "import \"{}\"",
         path.into_os_string().into_string().unwrap()
diff --git a/tests/integration/lib.rs b/tests/integration/main.rs
similarity index 82%
rename from tests/integration/lib.rs
rename to tests/integration/main.rs
index fb055e5a..6d4a5e66 100644
--- a/tests/integration/lib.rs
+++ b/tests/integration/main.rs
@@ -11,7 +11,7 @@ mod pass;
 mod pretty;
 mod query;
 mod records_fail;
-mod stdlib_arrays_fails;
+mod stdlib_arrays_fail;
 mod stdlib_typecheck;
 mod typecheck_fail;
-mod unbound_type_variables;
\ No newline at end of file
+mod unbound_type_variables;
diff --git a/tests/integration/pass.rs b/tests/integration/pass.rs
index a132211a..66bc59a8 100644
--- a/tests/integration/pass.rs
+++ b/tests/integration/pass.rs
@@ -21,7 +21,7 @@ fn run(path: impl Into<OsString>) {
 
 fn check_file(file: &str) {
     let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    path.push(format!("tests/pass/{}", file));
+    path.push(format!("tests/integration/pass/{}", file));
 
     thread::Builder::new()
         .name(String::from(file))
diff --git a/tests/integration/pass/multiple-overrides.ncl b/tests/integration/pass/multiple-overrides.ncl
index c26de6e8..7504abbb 100644
--- a/tests/integration/pass/multiple-overrides.ncl
+++ b/tests/integration/pass/multiple-overrides.ncl
@@ -76,31 +76,33 @@ let {check, ..} = import "testlib.ncl" in
   },
 
   # merging recursive expressions with different dependencies
-  let fst_data = {
-    common.fst = snd_data ++ "_data",
-    snd_data | Str
-             | default = "",
-    fst_data = "fst",
-  } in
-  let snd_data = {
-    common.snd = fst_data ++ "_data",
-    fst_data | Str
-             | default = "",
-    snd_data = "snd",
-  } in
-  let final_override | _push_force = {
-    fst_data = "override",
-    snd_data = "override",
-    common.final = fst_data ++ "_" ++ snd_data,
+  let parent = {
+    fst_data = {
+      common.fst = snd_data ++ "_data",
+      snd_data | Str
+               | default = "",
+      fst_data = "fst",
+    },
+    snd_data = {
+      common.snd = fst_data ++ "_data",
+      fst_data | Str
+               | default = "",
+      snd_data = "snd",
+    },
+    final_override | _push_force = {
+      fst_data = "override",
+      snd_data = "override",
+      common.final = fst_data ++ "_" ++ snd_data,
+    },
   } in
   [
-    fst_data & snd_data == {
+    parent.fst_data & parent.snd_data == {
       common.fst = "snd_data",
       common.snd = "fst_data",
       fst_data = "fst",
       snd_data = "snd",
     },
-    (fst_data & snd_data) & final_override == {
+    (parent.fst_data & parent.snd_data) & parent.final_override == {
       common.fst = "override_data",
       common.snd = "override_data",
       common.final = "override_override",
diff --git a/tests/integration/pretty.rs b/tests/integration/pretty.rs
index 2289e0c5..236e5011 100644
--- a/tests/integration/pretty.rs
+++ b/tests/integration/pretty.rs
@@ -13,14 +13,8 @@ fn diff(s1: &str, s2: &str) {
 
     for change in diff.iter_all_changes() {
         match change.tag() {
-            ChangeTag::Delete => {
-                nb_diff += 1;
-                println!("- {}", change);
-            }
-            ChangeTag::Insert => {
-                nb_diff += 1;
-                println!("+ {}", change);
-            }
+            ChangeTag::Delete => nb_diff += 1,
+            ChangeTag::Insert => nb_diff += 1,
             ChangeTag::Equal => (),
         };
     }
@@ -40,7 +34,7 @@ fn pretty(rt: &RichTerm) -> String {
 fn check_file(file: &str) {
     let mut buffer = String::new();
     let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
-    path.push(format!("tests/pass/{}", file));
+    path.push(format!("tests/integration/pass/{}", file));
     std::fs::File::open(&path)
         .and_then(|mut file| file.read_to_string(&mut buffer))
         .unwrap();
diff --git a/tests/integration/typecheck_fail.rs b/tests/integration/typecheck_fail.rs
index eaf57aa3..1a4c0e34 100644
--- a/tests/integration/typecheck_fail.rs
+++ b/tests/integration/typecheck_fail.rs
@@ -9,7 +9,7 @@ use nickel_lang::{typecheck, typecheck::Context};
 use nickel_lang_utilities::typecheck_fixture;
 
 fn type_check(rt: &RichTerm) -> Result<(), TypecheckError> {
-    typecheck::type_check(rt, Context::new(), &mut DummyResolver {}).map(|_| ())
+    typecheck::type_check(rt, Context::new(), &DummyResolver {}).map(|_| ())
 }
 
 fn type_check_expr(s: impl std::string::ToString) -> Result<(), TypecheckError> {
@@ -175,11 +175,10 @@ fn polymorphic_row_constraints() {
         assert!(match res.unwrap_err() {
             TypecheckError::RowConflict(_, _, _, _, _) => true,
             TypecheckError::ArrowTypeMismatch(_, _, _, err_boxed, _) => {
-                if let TypecheckError::RowConflict(_, _, _, _, _) = err_boxed.as_ref() {
-                    true
-                } else {
-                    false
-                }
+                matches!(
+                    err_boxed.as_ref(),
+                    TypecheckError::RowConflict(_, _, _, _, _)
+                )
             }
             _ => true,
         })