fix(tester): remove duplicate solutions when checking against Conjure

Remove duplicate solutions found by Minion before comparing them against
Conjures solutions.

Duplicates are introduced when we remove auxiliary variables from our
solutions. Conjure. For example, `{__0=1,x=1,y=2}` and `{__0=0,x=1,y=2}`
become identical to each other. As Conjure only has this solution once,
the check would fail. This was causing problems in #425.

To fix this, this patch changes the type of solutions from `HashMap` to
`BTreeMap`. This implements `Eq` and `Hash`, allowing us to use
`.unique()` on our list of solutions.

Note that this patch only changes this in the tester; future work should
make all solution fetching code use `BTreeMap`. Once this is done, the
solution checking logic could likely be simplified as it was written the
way it was because `HashMaps` could not be directly compared.

.gitignore

@@ -2,6 +2,7 @@
 coverage
 info.json
 
+crates/tree-sitter-essence/src
 conjure_oxide_log.json
 conjure_oxide/tests/integration/**/*generated*
 conjure_oxide/tests/*.png

conjure_oxide/src/utils/conjure.rs

@@ -1,4 +1,4 @@
-use std::collections::HashMap;
+use std::collections::BTreeMap;
 use std::io::Read;
 use std::path::Path;
 use std::string::ToString;
@@ -81,14 +81,14 @@ pub fn parse_essence_file(
 pub fn get_minion_solutions(
     model: Model,
     num_sols: i32,
-) -> Result<Vec<HashMap<Name, Literal>>, anyhow::Error> {
+) -> Result<Vec<BTreeMap<Name, Literal>>, anyhow::Error> {
     let solver = Solver::new(Minion::new());
     println!("Building Minion model...");
     let solver = solver.load_model(model)?;
 
     println!("Running Minion...");
 
-    let all_solutions_ref = Arc::new(Mutex::<Vec<HashMap<Name, Literal>>>::new(vec![]));
+    let all_solutions_ref = Arc::new(Mutex::<Vec<BTreeMap<Name, Literal>>>::new(vec![]));
     let all_solutions_ref_2 = all_solutions_ref.clone();
     let solver = if num_sols > 0 {
         let sols_left = Mutex::new(num_sols);
@@ -97,7 +97,7 @@ pub fn get_minion_solutions(
         solver
             .solve(Box::new(move |sols| {
                 let mut all_solutions = (*all_solutions_ref_2).lock().unwrap();
-                (*all_solutions).push(sols);
+                (*all_solutions).push(sols.into_iter().collect());
                 let mut sols_left = sols_left.lock().unwrap();
                 *sols_left -= 1;
 
@@ -109,7 +109,7 @@ pub fn get_minion_solutions(
         solver
             .solve(Box::new(move |sols| {
                 let mut all_solutions = (*all_solutions_ref_2).lock().unwrap();
-                (*all_solutions).push(sols);
+                (*all_solutions).push(sols.into_iter().collect());
                 true
             }))
             .unwrap()
@@ -126,7 +126,7 @@ pub fn get_minion_solutions(
 #[allow(clippy::unwrap_used)]
 pub fn get_solutions_from_conjure(
     essence_file: &str,
-) -> Result<Vec<HashMap<Name, Literal>>, EssenceParseError> {
+) -> Result<Vec<BTreeMap<Name, Literal>>, EssenceParseError> {
     // this is ran in parallel, and we have no guarantee by rust that invocations to this function
     // don't share the same tmp dir.
     let mut rng = rand::thread_rng();
@@ -180,10 +180,10 @@ pub fn get_solutions_from_conjure(
             "expected solutions to be an array".to_owned(),
         ))?;
 
-    let mut solutions_set: Vec<HashMap<Name, Literal>> = Vec::new();
+    let mut solutions_set: Vec<BTreeMap<Name, Literal>> = Vec::new();
 
     for solution in solutions {
-        let mut solution_map = HashMap::new();
+        let mut solution_map = BTreeMap::new();
         let solution = solution
             .as_object()
             .ok_or(EssenceParseError::ConjureSolutionsError(
@@ -209,7 +209,7 @@ pub fn get_solutions_from_conjure(
     Ok(solutions_set)
 }
 
-pub fn minion_solutions_to_json(solutions: &Vec<HashMap<Name, Literal>>) -> JsonValue {
+pub fn minion_solutions_to_json(solutions: &Vec<BTreeMap<Name, Literal>>) -> JsonValue {
     let mut json_solutions = Vec::new();
     for solution in solutions {
         let mut json_solution = Map::new();

conjure_oxide/src/utils/testing.rs

@@ -1,4 +1,4 @@
-use std::collections::{HashMap, HashSet};
+use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt::Debug;
 
 use std::fs::File;
@@ -148,7 +148,7 @@ pub fn minion_solutions_from_json(
 }
 
 pub fn save_minion_solutions_json(
-    solutions: &Vec<HashMap<Name, Literal>>,
+    solutions: &Vec<BTreeMap<Name, Literal>>,
     path: &str,
     test_name: &str,
     accept: bool,

conjure_oxide/tests/generated_tests.rs

@@ -1,7 +1,8 @@
 use conjure_oxide::utils::essence_parser::parse_essence_file_native;
 use conjure_oxide::utils::testing::read_human_rule_trace;
 use glob::glob;
-use std::collections::HashMap;
+use itertools::Itertools;
+use std::collections::BTreeMap;
 use std::env;
 use std::error::Error;
 use std::fs;
@@ -224,7 +225,7 @@ fn integration_test_inner(
 
     // test solutions against conjure before writing
     if accept {
-        let mut conjure_solutions: Vec<HashMap<Name, Literal>> =
+        let mut conjure_solutions: Vec<BTreeMap<Name, Literal>> =
             get_solutions_from_conjure(&format!("{}/{}.{}", path, essence_base, extension))?;
 
         // Change bools to nums in both outputs, as we currently don't convert 0,1 back to
@@ -251,6 +252,9 @@ fn integration_test_inner(
             }
         }
 
+        // remove duplicate entries (created when we removed machine names above)
+        username_solutions = username_solutions.into_iter().unique().collect();
+
         for solset in &mut conjure_solutions {
             for (k, v) in solset.clone().into_iter() {
                 match v {
@@ -265,6 +269,8 @@ fn integration_test_inner(
             }
         }
 
+        conjure_solutions = conjure_solutions.into_iter().unique().collect();
+
         // I can't make these sets of hashmaps due to hashmaps not implementing hash; so, to
         // compare these, I make them both json and compare that.
         let mut conjure_solutions_json: serde_json::Value =