Skip to content

Commit b5db6db

Browse files
committed
ci(triage): match test as a name segment, not just a whole component
#202 landed the namespaced-attribute fix, so main already detects #[tokio::test] and #[sqlx::test]. What it still misses is a third-party harness whose final path component carries `test` as an underscore segment. #[test_case(1)] and #[wasm_bindgen_test] both fall through and wrongly earn the PR a needs-tests label. Widen the final component to (?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*, keeping the leading ::, the r# form, and the whitespace tolerance #202 added. Differential over 972 real file-patches from 400 commits: no verdict changes either way, so this is a forward-looking net for harnesses the tree does not use yet, not a fix for a current mislabel. Segment rather than substring, on purpose. addsInlineTest suppresses the label when it matches, so a false positive is the quiet failure: #[contest] or #[latest] would clear needs-tests on a PR that added no tests and nobody would notice. The cost is the harness names with no underscore, such as #[rstest], which stay unmatched and produce the loud, author-correctable failure instead. The segment classes are [A-Za-z0-9]+ rather than \w+ so that `_` is only ever the literal delimiter. \w+ contains `_`, which makes the repetition ambiguous and backtracks exponentially. That matters here specifically because the workflow triggers on pull_request_target, so f.patch is fork-controlled text on a privileged runner: with \w+, `#[test` followed by a long `_a` run took 71ms at 22 characters and roughly quadrupled every two more, which stalls the triage job for any contributor who can open a PR. The character-class form measures linear across every adversarial input tried.
1 parent bfc44f9 commit b5db6db

1 file changed

Lines changed: 16 additions & 1 deletion

File tree

‎.github/workflows/pr-triage.yml‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,24 @@ jobs:
7676
// line after horizontal whitespace, covering bare, cfg(test), namespaced,
7777
// optional leading ::, and raw-identifier (r#test) forms. No head-file fetch
7878
// or lexer: needs-tests is advisory, not a merge gate.
79+
//
80+
// The final path component may also carry `test` as an underscore-delimited
81+
// segment, so third-party harnesses land without enumerating them one by one
82+
// (#[test_case(1)], #[wasm_bindgen_test]). It is deliberately a segment and
83+
// not a substring: a substring would also match #[contest] / #[latest], and a
84+
// false positive here SUPPRESSES needs-tests rather than adding it. This
85+
// heuristic has to fail loud, because a wrongly labeled PR gets corrected by
86+
// the author while a wrongly cleared one is invisible. That trade costs the
87+
// no-underscore harness names (#[rstest]), which stay unmatched on purpose.
88+
//
89+
// The segment classes are [A-Za-z0-9]+ rather than \w+ on purpose, so that `_`
90+
// is only ever the literal delimiter. \w+ contains `_`, which makes the repeat
91+
// ambiguous and backtracks exponentially: on this workflow's pull_request_target
92+
// trigger f.patch is fork-controlled, so `#[test` followed by a long `_a` run is
93+
// a permissionless stall of the triage job.
7994
const addsInlineTest = files.some(f =>
8095
f.filename.endsWith(".rs") && f.patch &&
81-
/^\+[ \t]*#\[[ \t]*(?:cfg[ \t]*\([ \t]*test[ \t]*\)|(?:::[ \t]*)?(?:[\w-]+[ \t]*::[ \t]*)*(?:r#)?test\b)/m.test(f.patch));
96+
/^\+[ \t]*#\[[ \t]*(?:cfg[ \t]*\([ \t]*test[ \t]*\)|(?:::[ \t]*)?(?:[\w-]+[ \t]*::[ \t]*)*(?:r#)?(?:[A-Za-z0-9]+_)*test(?:_[A-Za-z0-9]+)*[ \t]*[\]\(])/m.test(f.patch));
8297
const touchedTests =
8398
names.some(n => n.includes("/tests/") || n.endsWith("_test.rs")) || addsInlineTest;
8499
if (changedRust && !touchedTests) want.add("needs-tests");

0 commit comments

Comments
 (0)