Skip to content

Commit dd6cff5

Browse files
committed
ci(triage): detect harness test attributes, not just #[test]
The needs-tests detector matched an added `#[test]` or `#[cfg(test)]` line and nothing else, so it missed every test written with a harness attribute. That is most of this repo: 345 `#[tokio::test]` and 189 `#[sqlx::test]` against 540 bare `#[test]`. Any PR whose tests are all DB or async tests got labeled needs-tests, which is what happened on #275 (three added `#[sqlx::test]` functions) and #262 (eight). Match an added attribute whose last path component contains `test`, followed by `]` or `(`. That covers the four forms in the tree today and the harnesses a contributor might reach for later (`#[rstest]`, `#[test_case]`, `#[wasm_bindgen_test]`) without enumerating them. Deliberately not keying on an added `assert!`: a production assertion is ordinary Rust, so that signal would let a testless PR clear the label silently. The label is a nudge, so it should fail loud. Labeling a PR that did add tests gets corrected by the author; clearing one that did not is invisible. Anchoring to the start of the added line also drops three false positives the old `.*` prefix accepted: a comment, a string literal, and a doc example that merely mention `#[test]`. Verified against the real patches of 14 open PRs: #275 and #262 stop being labeled, both of which do add tests, and no other verdict changes.
1 parent c926e1e commit dd6cff5

1 file changed

Lines changed: 15 additions & 1 deletion

File tree

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,23 @@ jobs:
7070
const changedRust = names.some(n => n.startsWith("crates/") && n.endsWith(".rs"));
7171
// A test can live in a tests/ dir, a *_test.rs file, or (the common Rust
7272
// pattern) an inline #[test] / #[cfg(test)] block added inside the source file.
73+
// Match an added attribute whose last path component contains `test`,
74+
// followed by `]` or `(`. That covers the four forms in the tree today
75+
// (#[test], #[tokio::test], #[tokio::test(flavor = ...)], #[sqlx::test])
76+
// and the harnesses a future contributor might reach for without naming
77+
// them one by one (#[rstest], #[test_case], #[wasm_bindgen_test]).
78+
//
79+
// Deliberately NOT keying on an added assert!/assert_eq!: a production
80+
// assertion is ordinary Rust, so that signal lets a testless PR clear the
81+
// label silently. This heuristic is only a nudge, so it should fail in the
82+
// loud direction -- labeling a PR that did add tests gets corrected by the
83+
// author, while clearing one that did not is invisible.
84+
//
85+
// Anchored to the start of the added line, so a comment or a string that
86+
// merely mentions #[test] does not count.
7387
const addsInlineTest = files.some(f =>
7488
f.filename.endsWith(".rs") && f.patch &&
75-
/^\+.*#\[(test\]|cfg\(test\))/m.test(f.patch));
89+
/^\+\s*#\[(cfg\(test\)|(?:\w+::)*\w*test\w*[\](])/m.test(f.patch));
7690
const touchedTests =
7791
names.some(n => n.includes("/tests/") || n.endsWith("_test.rs")) || addsInlineTest;
7892
if (changedRust && !touchedTests) want.add("needs-tests");

0 commit comments

Comments
 (0)