Merge pull request #206 from github/test-fixes

hendrikvanantwerpen · web-flow · commit 06d5d9e43ef7 · 2023-02-27T14:20:42.000+01:00
diff --git a/stack-graphs/CHANGELOG.md b/stack-graphs/CHANGELOG.md
@@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - The `IncorrectDefinitions` error is renamed to `IncorrectlyDefined`, and `IncorrectDefinitions` is the error used for the `Defines` assertion.
+- The `PartialPaths::find_all_partial_paths_in_file` method has been replaced by `PartialPaths::find_minimal_partial_path_set_in_file`, which computes a smaller set. The `ForwardPartialPathStitcher::find_locally_maximal_partial_path_set` function can be used to compute the set previously returned by `find_all_partial_paths_in_file` from the minimal partial path set.
 
 ## v0.10.2 -- 2023-01-10
 
diff --git a/stack-graphs/src/c.rs b/stack-graphs/src/c.rs
@@ -1577,7 +1577,7 @@ pub extern "C" fn sg_partial_path_arena_find_partial_paths_in_file(
     let cancellation_flag: Option<&AtomicUsize> =
         unsafe { std::mem::transmute(cancellation_flag.as_ref()) };
     partials
-        .find_minimal_partial_paths_set_in_file(
+        .find_minimal_partial_path_set_in_file(
             graph,
             file,
             &AtomicUsizeCancellationFlag(cancellation_flag),
diff --git a/stack-graphs/src/partial.rs b/stack-graphs/src/partial.rs
@@ -2485,7 +2485,7 @@ impl PartialPaths {
     /// paths will not cover paths going through those edges.
     ///
     /// [`PartialPath::extend`]: struct.PartialPath.html#method.extend
-    pub fn find_minimal_partial_paths_set_in_file<F>(
+    pub fn find_minimal_partial_path_set_in_file<F>(
         &mut self,
         graph: &StackGraph,
         file: Handle<File>,
diff --git a/stack-graphs/tests/it/can_find_local_nodes.rs b/stack-graphs/tests/it/can_find_local_nodes.rs
@@ -20,7 +20,7 @@ fn check_local_nodes(graph: &StackGraph, file: &str, expected_local_nodes: &[&st
     let mut partials = PartialPaths::new();
     let mut database = Database::new();
     partials
-        .find_minimal_partial_paths_set_in_file(
+        .find_minimal_partial_path_set_in_file(
             graph,
             file,
             &NoCancellation,
diff --git a/stack-graphs/tests/it/can_find_node_partial_paths_in_database.rs b/stack-graphs/tests/it/can_find_node_partial_paths_in_database.rs
@@ -29,7 +29,7 @@ fn check_node_partial_paths(
     let mut partials = PartialPaths::new();
     let mut db = Database::new();
     partials
-        .find_minimal_partial_paths_set_in_file(
+        .find_minimal_partial_path_set_in_file(
             graph,
             file,
             &NoCancellation,
diff --git a/stack-graphs/tests/it/can_find_partial_paths_in_file.rs b/stack-graphs/tests/it/can_find_partial_paths_in_file.rs
@@ -19,7 +19,7 @@ fn check_partial_paths_in_file(graph: &StackGraph, file: &str, expected_paths: &
     let mut partials = PartialPaths::new();
     let mut results = BTreeSet::new();
     partials
-        .find_minimal_partial_paths_set_in_file(
+        .find_minimal_partial_path_set_in_file(
             graph,
             file,
             &NoCancellation,
diff --git a/stack-graphs/tests/it/can_find_root_partial_paths_in_database.rs b/stack-graphs/tests/it/can_find_root_partial_paths_in_database.rs
@@ -33,7 +33,7 @@ fn check_root_partial_paths(
     let mut partials = PartialPaths::new();
     let mut db = Database::new();
     partials
-        .find_minimal_partial_paths_set_in_file(
+        .find_minimal_partial_path_set_in_file(
             graph,
             file,
             &NoCancellation,
diff --git a/stack-graphs/tests/it/can_jump_to_definition_with_forward_partial_path_stitching.rs b/stack-graphs/tests/it/can_jump_to_definition_with_forward_partial_path_stitching.rs
@@ -23,7 +23,7 @@ fn check_jump_to_definition(graph: &StackGraph, expected_partial_paths: &[&str])
     // Generate partial paths for everything in the database.
     for file in graph.iter_files() {
         partials
-            .find_minimal_partial_paths_set_in_file(
+            .find_minimal_partial_path_set_in_file(
                 graph,
                 file,
                 &NoCancellation,
diff --git a/stack-graphs/tests/it/can_jump_to_definition_with_forward_path_stitching.rs b/stack-graphs/tests/it/can_jump_to_definition_with_forward_path_stitching.rs
@@ -25,7 +25,7 @@ fn check_jump_to_definition(graph: &StackGraph, expected_paths: &[&str]) {
     // Generate partial paths for everything in the database.
     for file in graph.iter_files() {
         partials
-            .find_minimal_partial_paths_set_in_file(
+            .find_minimal_partial_path_set_in_file(
                 graph,
                 file,
                 &NoCancellation,
diff --git a/stack-graphs/tests/it/json.rs b/stack-graphs/tests/it/json.rs
@@ -1818,7 +1818,7 @@ fn can_serialize_partial_paths() {
     let mut db = Database::new();
     for file in graph.iter_files() {
         partials
-            .find_minimal_partial_paths_set_in_file(&graph, file, &NoCancellation, |g, ps, p| {
+            .find_minimal_partial_path_set_in_file(&graph, file, &NoCancellation, |g, ps, p| {
                 db.add_partial_path(g, ps, p);
             })
             .expect("Expect path finding to work");
diff --git a/tree-sitter-stack-graphs/src/cli/analyze.rs b/tree-sitter-stack-graphs/src/cli/analyze.rs
@@ -224,7 +224,7 @@ impl AnalyzeArgs {
 
         let mut partials = PartialPaths::new();
         let mut db = Database::new();
-        match partials.find_minimal_partial_paths_set_in_file(
+        match partials.find_minimal_partial_path_set_in_file(
             &graph,
             file,
             &cancellation_flag.as_ref(),
diff --git a/tree-sitter-stack-graphs/src/cli/test.rs b/tree-sitter-stack-graphs/src/cli/test.rs
@@ -37,22 +37,6 @@ use crate::StackGraphLanguage;
 
 use super::util::FileStatusLogger;
 
-/// Flag to control output
-#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ArgEnum)]
-pub enum OutputMode {
-    Always,
-    OnFailure,
-}
-
-impl OutputMode {
-    fn test(&self, failure: bool) -> bool {
-        match self {
-            Self::Always => true,
-            Self::OnFailure => failure,
-        }
-    }
-}
-
 /// Run tests
 #[derive(Args)]
 #[clap(after_help = r#"PATH SPECIFICATIONS:
@@ -143,9 +127,28 @@ pub struct TestArgs {
         long,
         arg_enum,
         default_value_t = OutputMode::OnFailure,
-        require_equals = true,
     )]
     pub output_mode: OutputMode,
+
+    /// Do not load builtins for tests.
+    #[clap(long)]
+    pub no_builtins: bool,
+}
+
+/// Flag to control output
+#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, ArgEnum)]
+pub enum OutputMode {
+    Always,
+    OnFailure,
+}
+
+impl OutputMode {
+    fn test(&self, failure: bool) -> bool {
+        match self {
+            Self::Always => true,
+            Self::OnFailure => failure,
+        }
+    }
 }
 
 impl TestArgs {
@@ -159,6 +162,7 @@ impl TestArgs {
             save_paths: None,
             save_visualization: None,
             output_mode: OutputMode::OnFailure,
+            no_builtins: false,
         }
     }
 
@@ -230,8 +234,10 @@ impl TestArgs {
 
         let default_fragment_path = test_path.strip_prefix(test_root).unwrap();
         let mut test = Test::from_source(&test_path, &source, default_fragment_path)?;
-        self.load_builtins_into(&lc, &mut test.graph)
-            .with_context(|| format!("Loading builtins into {}", test_path.display()))?;
+        if !self.no_builtins {
+            self.load_builtins_into(&lc, &mut test.graph)
+                .with_context(|| format!("Loading builtins into {}", test_path.display()))?;
+        }
         let mut globals = Variables::new();
         for test_fragment in &test.fragments {
             if let Some(fa) = test_fragment
@@ -274,6 +280,16 @@ impl TestArgs {
         }
         let mut partials = PartialPaths::new();
         let mut db = Database::new();
+        for file in test.graph.iter_files() {
+            partials.find_minimal_partial_path_set_in_file(
+                &test.graph,
+                file,
+                &(cancellation_flag as &dyn CancellationFlag),
+                |g, ps, p| {
+                    db.add_partial_path(g, ps, p);
+                },
+            )?;
+        }
         let result = test.run(&mut partials, &mut db, cancellation_flag)?;
         let success = self.handle_result(&result, &mut file_status)?;
         if self.output_mode.test(!success) {
diff --git a/tree-sitter-stack-graphs/src/test.rs b/tree-sitter-stack-graphs/src/test.rs
@@ -617,27 +617,14 @@ impl std::fmt::Display for TestFailure {
 
 impl Test {
     /// Run the test. It is the responsibility of the caller to ensure that
-    /// the stack graph has been constructed for the test fragments before running
-    /// the test.
+    /// the stack graph for the test fragments has been constructed, and the
+    /// database has been filled with partial paths before running the test.
     pub fn run(
         &mut self,
         partials: &mut PartialPaths,
         db: &mut Database,
         cancellation_flag: &dyn CancellationFlag,
     ) -> Result<TestResult, stack_graphs::CancellationError> {
-        // build partial paths
-        for file in self.graph.iter_files() {
-            partials.find_minimal_partial_paths_set_in_file(
-                &self.graph,
-                file,
-                &cancellation_flag,
-                |g, ps, p| {
-                    db.add_partial_path(g, ps, p);
-                },
-            )?;
-        }
-
-        // test assertions
         let mut result = TestResult::new();
         for fragment in &self.fragments {
             for assertion in &fragment.assertions {
diff --git a/tree-sitter-stack-graphs/tests/it/test.rs b/tree-sitter-stack-graphs/tests/it/test.rs
@@ -107,6 +107,19 @@ fn check_test(
     }
     let mut partials = PartialPaths::new();
     let mut db = Database::new();
+    for fragment in &test.fragments {
+        partials
+            .find_minimal_partial_path_set_in_file(
+                &test.graph,
+                fragment.file,
+                &stack_graphs::NoCancellation,
+                |graph, partials, path| {
+                    db.add_partial_path(graph, partials, path);
+                },
+            )
+            .expect("should nopt be cancelled");
+    }
+
     let results = test
         .run(&mut partials, &mut db, &NoCancellation)
         .expect("should never be cancelled");
