From 282177fde2b5bf5c0c29cb6fafa790e6e4c64162 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Mon, 22 Jun 2026 14:40:40 +0000
Subject: [PATCH] test: fix weak and incorrect assertions in pr_filters tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Five fixes in src/compile/pr_filters.rs:

- test_gate_step_commit_message: pattern.contains("skip-agent") was too
  weak and would pass for many wrong patterns; replace with

- test_gate_step_change_count_includes_changed_files_fact: test name claims
  to verify that ChangedFileCount transitively depends on ChangedFiles, but
  the body also added an explicit changed_files filter — masking the
  dependency entirely. Remove that filter so only min_changes is set and
  changed_files can only appear via the declared dependency.

- test_gate_step_build_reason_include: match arm used .. to skip the fact
  field, so a regression that wired the wrong fact would go undetected.

- test_gate_step_build_reason_exclude: same issue as above for

- test_gate_step_labels_none_of: was missing both check.name and fact
  assertions that the symmetric test_gate_step_labels_any_of already

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 src/compile/pr_filters.rs | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/src/compile/pr_filters.rs b/src/compile/pr_filters.rs
index af89d50f..90614714 100644
--- a/src/compile/pr_filters.rs
+++ b/src/compile/pr_filters.rs
@@ -111,8 +111,11 @@ mod tests {
         };
         let checks = lower_pr_filters(&filters);
         let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap();
-        match &spec.checks[0].predicate {
-            PredicateSpec::LabelSetMatch { none_of, .. } => {
+        let check = &spec.checks[0];
+        assert_eq!(check.name, "labels");
+        match &check.predicate {
+            PredicateSpec::LabelSetMatch { fact, none_of, .. } => {
+                assert_eq!(fact, "pr_labels");
                 assert!(none_of.contains(&"do-not-run".to_string()));
             }
             other => panic!("expected LabelSetMatch, got {:?}", other),
@@ -285,7 +288,8 @@ mod tests {
         let checks = lower_pr_filters(&filters);
         let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap();
         match &spec.checks[0].predicate {
-            PredicateSpec::ValueInSet { values, .. } => {
+            PredicateSpec::ValueInSet { fact, values, .. } => {
+                assert_eq!(fact, "build_reason");
                 assert!(values.contains(&"PullRequest".to_string()));
                 assert!(values.contains(&"Manual".to_string()));
             }
@@ -309,7 +313,8 @@ mod tests {
         let checks = lower_pr_filters(&filters);
         let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap();
         match &spec.checks[0].predicate {
-            PredicateSpec::ValueNotInSet { values, .. } => {
+            PredicateSpec::ValueNotInSet { fact, values, .. } => {
+                assert_eq!(fact, "build_reason");
                 assert!(values.contains(&"Schedule".to_string()));
             }
             other => panic!("expected ValueNotInSet, got {:?}", other),
@@ -319,18 +324,18 @@ mod tests {
 
     #[test]
     fn test_gate_step_change_count_includes_changed_files_fact() {
+        // Uses only min_changes (no changed_files filter) to verify that the
+        // ChangedFileCount fact transitively requires ChangedFiles via its
+        // declared dependency — without an explicit changed_files filter the
+        // only source of the changed_files fact is the dependency graph.
         use crate::compile::filter_ir::{GateContext, build_gate_spec, lower_pr_filters};
         let filters = PrFilters {
-            changed_files: Some(IncludeExcludeFilter {
-                include: vec!["src/**".into()],
-                ..Default::default()
-            }),
             min_changes: Some(3),
             ..Default::default()
         };
         let checks = lower_pr_filters(&filters);
         let spec = build_gate_spec(GateContext::PullRequest, &checks).unwrap();
-        // Both changed_files and changed_file_count facts should be present
+        // changed_files must appear because changed_file_count depends on it
         assert!(spec.facts.iter().any(|f| f.kind == "changed_files"));
         assert!(spec.facts.iter().any(|f| f.kind == "changed_file_count"));
     }
@@ -387,7 +392,7 @@ triggers:
         match &spec.checks[0].predicate {
             PredicateSpec::GlobMatch { fact, pattern } => {
                 assert_eq!(fact, "commit_message");
-                assert!(pattern.contains("skip-agent"));
+                assert_eq!(pattern, "*[skip-agent]*");
             }
             other => panic!("expected GlobMatch, got {:?}", other),
         }