feat: add strict mode for package content tests to enforce file matching (#1677)

Author: zelosleone

docs/reference/recipe_file.md

@@ -890,6 +890,10 @@ tests:
       bin:
         - mamba
 
+      # enable strict mode: error if any file in the package is not matched by one of the globs
+      # (default: false)
+      strict: true
+
       # searches for `$PREFIX/lib/libmamba.so` or `$PREFIX/lib/libmamba.dylib` on Linux or macOS,
       # on Windows for %PREFIX%\Library\lib\mamba.dll & %PREFIX%\Library\bin\mamba.bin
       lib:

docs/testing.md

@@ -63,6 +63,15 @@ tests:
   - package_contents:
       files:
         - share/package/*.txt
+
+  # test with strict mode: fails if there are any files not matched by the globs
+  - package_contents:
+      strict: true
+      files:
+        - share/package/*.txt
+        - bin/myapp
+      lib:
+        - mylib
 ```
 
 ### Testing package contents
@@ -78,6 +87,7 @@ It has multiple sub-keys that help when building cross-platform packages:
 - **`include`**: matches files under the `include` directory in the package. You can specify the file name like `foo.h`.
 - **`bin`**: matches files under the `bin` directory in the package. You can specify executable names like `foo` which will match `foo.exe` on Windows and `foo` on Linux and macOS.
 - **`site_packages`**: matches files under the `site-packages` directory in the package. You can specify the import path like `foobar.api` which will match `foobar/api.py` and `foobar/api/__init__.py`.
+- **`strict`**: when set to `true`, enables strict mode. In strict mode, the test will fail if there are any files in the package that don't match any of the specified globs. (default: `false`).
 
 ## Testing existing packages
 

src/package_test/content_test.rs

@@ -1,3 +1,4 @@
+use std::collections::HashSet;
 use std::path::PathBuf;
 
 use crate::recipe::parser::PackageContentsTest;
@@ -229,106 +230,90 @@ impl PackageContentsTest {
         let span = tracing::info_span!("Package content test");
         let _enter = span.enter();
         let target_platform = output.target_platform();
-        let paths = paths
-            .paths
-            .iter()
-            .map(|p| &p.relative_path)
-            .collect::<Vec<_>>();
-
-        let include_globs = self.include_as_globs(target_platform)?;
-        let bin_globs = self.bin_as_globs(target_platform)?;
-        let lib_globs = self.lib_as_globs(target_platform)?;
-        let site_package_globs = self.site_packages_as_globs(
-            target_platform,
-            output.recipe.build().is_python_version_independent(),
-        )?;
-        let file_globs = self.files_as_globs()?;
-
-        fn match_glob<'a>(glob: &GlobSet, paths: &'a Vec<&PathBuf>) -> Vec<&'a PathBuf> {
-            let mut matches: Vec<&'a PathBuf> = Vec::new();
-            for path in paths {
-                if glob.is_match(path) {
-                    matches.push(path);
-                }
-            }
-            matches
-        }
-
-        let mut collected_issues = Vec::new();
-
-        for glob in include_globs {
-            let matches = match_glob(&glob.1, &paths);
-
-            if !matches.is_empty() {
-                display_success(&matches, &glob.0, "include");
-            }
-
-            if matches.is_empty() {
-                collected_issues.push(format!("No match for include glob: {}", glob.0));
-            }
-        }
-
-        for glob in bin_globs {
-            let matches = match_glob(&glob.1, &paths);
-
-            if !matches.is_empty() {
-                display_success(&matches, &glob.0, "bin");
-            }
-
-            if matches.is_empty() {
-                collected_issues.push(format!("No match for bin glob: {}", glob.0));
-            }
-        }
-
-        for glob in lib_globs {
-            let matches = match_glob(&glob.1, &paths);
-
-            if !matches.is_empty() {
-                display_success(&matches, &glob.0, "lib");
-            }
-
-            if matches.is_empty() {
-                collected_issues.push(format!("No match for lib glob: {}", glob.0));
-            }
-        }
+        let paths: Vec<&PathBuf> = paths.paths.iter().map(|p| &p.relative_path).collect();
+
+        // Collect all glob patterns
+        let all_globs = [
+            ("include", self.include_as_globs(target_platform)?),
+            ("bin", self.bin_as_globs(target_platform)?),
+            ("lib", self.lib_as_globs(target_platform)?),
+            (
+                "site_packages",
+                self.site_packages_as_globs(
+                    target_platform,
+                    output.recipe.build().is_python_version_independent(),
+                )?,
+            ),
+            ("files", self.files_as_globs()?),
+        ];
 
-        for glob in site_package_globs {
-            let matches = match_glob(&glob.1, &paths);
+        let mut matched_paths = HashSet::<&PathBuf>::new();
+        let mut issues = Vec::new();
 
-            if !matches.is_empty() {
-                display_success(&matches, &glob.0, "site_packages");
-            }
+        // Check all globs
+        for (section, globs) in &all_globs {
+            for (glob_str, globset) in globs {
+                let matches: Vec<&PathBuf> = paths
+                    .iter()
+                    .filter(|path| globset.is_match(path))
+                    .copied()
+                    .collect();
 
-            if matches.is_empty() {
-                collected_issues.push(format!("No match for site_package glob: {}", glob.0));
+                if matches.is_empty() {
+                    issues.push(format!("No match for {} glob: {}", section, glob_str));
+                } else {
+                    display_success(&matches, glob_str, section);
+                    matched_paths.extend(&matches);
+                }
             }
         }
 
-        for glob in file_globs {
-            let matches = match_glob(&glob.1, &paths);
-
-            if !matches.is_empty() {
-                display_success(&matches, &glob.0, "file");
-            }
-
-            if matches.is_empty() {
-                collected_issues.push(format!("No match for file glob: {}", glob.0));
+        // Check strict mode
+        let strict_mode_issue = if self.strict {
+            let unmatched: Vec<&PathBuf> = paths
+                .iter()
+                .filter(|p| !matched_paths.contains(*p))
+                .copied()
+                .collect();
+
+            if !unmatched.is_empty() {
+                Some((
+                    format!("Strict mode: {} unmatched files found", unmatched.len()),
+                    unmatched,
+                ))
+            } else {
+                None
             }
-        }
+        } else {
+            None
+        };
 
-        if !collected_issues.is_empty() {
+        if !issues.is_empty() || strict_mode_issue.is_some() {
             tracing::error!("Package content test failed:");
-            for issue in &collected_issues {
+
+            // Print regular issues first
+            for issue in &issues {
                 tracing::error!(
                     "- {} {}",
                     console::style(console::Emoji("❌", " ")).red(),
                     issue
                 );
             }
 
-            return Err(TestError::PackageContentTestFailed(
-                collected_issues.join("\n"),
-            ));
+            // Print strict mode issues if any
+            if let Some((message, unmatched)) = &strict_mode_issue {
+                tracing::error!("\nStrict mode violations:");
+                for file in unmatched {
+                    tracing::error!(
+                        "- {} {}",
+                        console::style(console::Emoji("❌", " ")).red(),
+                        file.display()
+                    );
+                }
+                issues.push(message.clone());
+            }
+
+            return Err(TestError::PackageContentTestFailed(issues.join("\n")));
         }
 
         Ok(())
@@ -524,4 +509,20 @@ mod tests {
         let test_case = load_test_case(Path::new("test_files.yaml"));
         evaluate_test_case(test_case).unwrap();
     }
+
+    #[test]
+    fn test_strict_mode() {
+        let strict_contents = PackageContentsTest {
+            files: GlobVec::from_vec(vec!["matched.txt"], None),
+            strict: true,
+            ..Default::default()
+        };
+        assert!(strict_contents.strict);
+
+        let non_strict_contents = PackageContentsTest {
+            files: GlobVec::from_vec(vec!["*.txt"], None),
+            ..Default::default()
+        };
+        assert!(!non_strict_contents.strict);
+    }
 }

src/recipe/parser/snapshots/rattler_build__recipe__parser__test__test__script_parsing.snap

@@ -32,9 +32,11 @@ expression: yaml_serde
     files:
     - foo
     - bar
+    strict: false
 - package_contents:
     lib:
     - libfoo.so
     - libbar.so
     include:
     - xtensor/xarray.hpp
+    strict: false

src/recipe/parser/test.rs

@@ -194,6 +194,9 @@ pub struct PackageContentsTest {
     /// check if include path contains the file, direct or glob?
     #[serde(default, skip_serializing_if = "GlobVec::is_empty")]
     pub include: GlobVec,
+    /// whether to enable strict mode (error on non-matched files or missing files)
+    #[serde(default)]
+    pub strict: bool,
 }
 
 impl TryConvertNode<Vec<TestType>> for RenderedNode {
@@ -474,7 +477,8 @@ impl TryConvertNode<PackageContentsTest> for RenderedMappingNode {
             site_packages,
             lib,
             bin,
-            include
+            include,
+            strict
         );
         Ok(package_contents)
     }
@@ -630,4 +634,43 @@ mod test {
             _ => panic!("expected python test"),
         }
     }
+
+    #[test]
+    fn test_package_contents_strict_mode() {
+        let test_section = r#"
+        tests:
+          - package_contents:
+              strict: true
+              files:
+                - "**/*.txt"
+              bin:
+                - rust
+          - package_contents:
+              files:
+                - "**/*.txt"
+        "#;
+
+        let yaml_root = RenderedNode::parse_yaml(0, test_section)
+            .map_err(|err| vec![err])
+            .unwrap();
+        let tests_node = yaml_root.as_mapping().unwrap().get("tests").unwrap();
+        let tests: Vec<TestType> = tests_node.try_convert("tests").unwrap();
+
+        match &tests[0] {
+            TestType::PackageContents { package_contents } => {
+                assert!(package_contents.strict);
+                assert_eq!(package_contents.files.include_globs().len(), 1);
+                assert_eq!(package_contents.bin.include_globs().len(), 1);
+            }
+            _ => panic!("expected package contents test"),
+        }
+
+        match &tests[1] {
+            TestType::PackageContents { package_contents } => {
+                assert!(!package_contents.strict);
+                assert_eq!(package_contents.files.include_globs().len(), 1);
+            }
+            _ => panic!("expected package contents test"),
+        }
+    }
 }

src/recipe/snapshots/rattler_build__recipe__parser__tests__complete_recipe.snap

@@ -169,6 +169,7 @@ tests:
     include:
     - test.h
     - test.h*
+    strict: false
 - python:
     imports:
     - numpy

src/recipe/snapshots/rattler_build__recipe__parser__tests__recipe_windows.snap

@@ -366,6 +366,7 @@ Recipe {
                 include: [
                     "xtensor/xarray.hpp{,/**}",
                 ],
+                strict: false,
             },
         },
         Command(

src/recipe/snapshots/rattler_build__recipe__parser__tests__unix_recipe.snap

@@ -388,6 +388,7 @@ Recipe {
                 include: [
                     "xtensor/xarray.hpp{,/**}",
                 ],
+                strict: false,
             },
         },
         Command(

test-data/recipes/strict-mode/recipe-fail.yaml

@@ -0,0 +1,19 @@
+package:
+  name: test-strict-fail
+  version: 0.1.0
+
+build:
+  script:
+    - if: unix
+      then:
+        - echo "test" > $PREFIX/test.txt
+        - echo "extra" > $PREFIX/extra.txt
+      else:
+        - echo "test" > %PREFIX%\test.txt
+        - echo "extra" > %PREFIX%\extra.txt
+
+tests:
+  - package_contents:
+      strict: true
+      files:
+        - test.txt

test-data/recipes/strict-mode/recipe-many-files.yaml

@@ -0,0 +1,31 @@
+package:
+  name: test-strict-many
+  version: 0.1.0
+
+build:
+  script:
+    - if: unix
+      then:
+        - echo "matched" > $PREFIX/matched.txt
+        - echo "unmatched1" > $PREFIX/unmatched1.txt
+        - echo "unmatched2" > $PREFIX/unmatched2.txt
+        - echo "unmatched3" > $PREFIX/unmatched3.txt
+        - echo "unmatched4" > $PREFIX/unmatched4.txt
+        - echo "unmatched5" > $PREFIX/unmatched5.txt
+        - echo "unmatched6" > $PREFIX/unmatched6.txt
+        - echo "unmatched7" > $PREFIX/unmatched7.txt
+      else:
+        - echo "matched" > %PREFIX%\matched.txt
+        - echo "unmatched1" > %PREFIX%\unmatched1.txt
+        - echo "unmatched2" > %PREFIX%\unmatched2.txt
+        - echo "unmatched3" > %PREFIX%\unmatched3.txt
+        - echo "unmatched4" > %PREFIX%\unmatched4.txt
+        - echo "unmatched5" > %PREFIX%\unmatched5.txt
+        - echo "unmatched6" > %PREFIX%\unmatched6.txt
+        - echo "unmatched7" > %PREFIX%\unmatched7.txt
+
+tests:
+  - package_contents:
+      strict: true
+      files:
+        - matched.txt