Skip to content

Commit bb42bb5

Browse files
Copilotpelikhan
Copilot
and
pelikhan
authored
Optimize test-quality-sentinel with cached diffs and inline analyzers (#30882)
* Initial plan * feat: optimize test-quality-sentinel workflow Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ea5a23f-1f60-4c59-80bc-ec21f8078260 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> * fix: clarify test-quality-sentinel agent instructions Agent-Logs-Url: https://github.com/github/gh-aw/sessions/3ea5a23f-1f60-4c59-80bc-ec21f8078260 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
1 parent b50f7cc commit bb42bb5

2 files changed

Lines changed: 127 additions & 50 deletions

File tree

‎.github/workflows/test-quality-sentinel.lock.yml‎

Lines changed: 42 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎.github/workflows/test-quality-sentinel.md‎

Lines changed: 85 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,9 @@ steps:
5353
touch /tmp/gh-aw/agent/test-diff.txt
5454
fi
5555
56+
git diff "${{ github.event.pull_request.base.sha }}...HEAD" --numstat \
57+
> /tmp/gh-aw/agent/diff-numstat.txt 2>/dev/null || true
58+
5659
echo "Pre-fetched $(grep -c . /tmp/gh-aw/agent/test-files.txt || echo 0) test files"
5760
safe-outputs:
5861
add-comment:
@@ -71,6 +74,7 @@ imports:
7174
- shared/reporting.md
7275
features:
7376
copilot-requests: true
77+
inline-agents: true
7478
---
7579

7680
# Test Quality Sentinel 🧪
@@ -103,6 +107,9 @@ cat /tmp/gh-aw/agent/test-files.txt
103107

104108
# Diff for test files only
105109
cat /tmp/gh-aw/agent/test-diff.txt
110+
111+
# Numstat for all changed files
112+
cat /tmp/gh-aw/agent/diff-numstat.txt
106113
```
107114

108115
Then identify all **new and modified test files** in the diff:
@@ -155,24 +162,10 @@ For each changed test file, run structural checks using available tools.
155162

156163
### 3a. Go — `Test*` functions
157164

158-
Analyze Go test functions using grep and awk on the diff. This codebase uses **both** stdlib assertions (`t.Errorf`, `t.Fatalf`, `t.Error`) **and** testify (`assert.*`, `require.*`). The project guideline is **no mock libraries** — tests must interact with real components; any use of `gomock`, `testify/mock`, or `EXPECT()` in Go is itself a red flag.
159-
160-
```bash
161-
# Count assertions, error checks, table-driven subtests, and any forbidden mock calls per Test* function
162-
git diff ${{ github.event.pull_request.base.sha }}...HEAD -- '*_test.go' | awk '
163-
/^\+func Test/ {
164-
if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks
165-
match($0, /func (Test[^(]+)/, arr); test_name=arr[1]; assertions=0; errors=0; table_driven=0; forbidden_mocks=0
166-
}
167-
test_name && /^\+.*(assert\.|require\.)/ { assertions++ }
168-
test_name && /^\+.*t\.(Error|Errorf|Fatal|Fatalf)\(/ { assertions++; errors++ }
169-
test_name && /^\+.*(assert\.Error|require\.Error|assert\.NoError|require\.NoError)/ { errors++ }
170-
test_name && /^\+.*t\.Run\(/ { table_driven++ }
171-
test_name && /^\+.*(gomock\.|testify\/mock|\.EXPECT\(\)|\.On\(|\.Return\()/ { forbidden_mocks++ }
172-
test_name && /^\+\}$/ { print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks; test_name="" }
173-
END { if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks }
174-
'
175-
```
165+
Use the `go-test-analyzer` agent to extract per-function assertion counts, error coverage,
166+
table-driven usage, and forbidden mock calls from the pre-fetched test diff. It also checks
167+
for missing `//go:build` tags in newly added Go test files. Use its table and build-tag findings
168+
as input to Step 4.
176169

177170
Key signals for Go tests in this codebase:
178171
- **Assertions (accepted forms)**:
@@ -185,22 +178,9 @@ Key signals for Go tests in this codebase:
185178

186179
### 3b. JavaScript — vitest `test()` / `it()` blocks
187180

188-
This codebase uses **vitest** (not jest). Mock helpers come from vitest: `vi.fn()`, `vi.spyOn()`, `vi.mock()`. Primary test file extension is `.test.cjs`; scripts tests use `.test.js`.
189-
190-
```bash
191-
# Count expect() assertions, error matchers, and vi.* mock calls per test block
192-
git diff ${{ github.event.pull_request.base.sha }}...HEAD -- '*.test.cjs' '*.test.js' | awk '
193-
/^\+(it|test)\(/ {
194-
if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks
195-
match($0, /(it|test)\(["'"'"']([^"'"'"']+)/, arr); test_name=arr[2]; assertions=0; errors=0; mocks=0
196-
}
197-
test_name && /^\+.*expect\(/ { assertions++ }
198-
test_name && /^\+.*(\.toThrow|\.rejects|\.toThrowError)/ { errors++ }
199-
test_name && /^\+.*(vi\.mock|vi\.spyOn|vi\.fn)/ { mocks++ }
200-
test_name && /^\+\}\)/ { print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks; test_name="" }
201-
END { if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks }
202-
'
203-
```
181+
Use the `js-test-analyzer` agent to extract per-test assertion counts, error matcher usage,
182+
and `vi.*` mock calls from the pre-fetched test diff. Covers both `.test.cjs` (primary) and
183+
`.test.js` (scripts) formats. Use its table as input to Step 4.
204184

205185
Key signals for JavaScript tests in this codebase:
206186
- **Assertions**: `expect(...)` calls with vitest matchers (`.toBe`, `.toEqual`, `.toMatchObject`, `.toContain`, `.toBeNull`, etc.)
@@ -254,8 +234,7 @@ Calculate the test inflation ratio for each changed test file:
254234

255235
```bash
256236
# Count lines added to test files vs. production files
257-
git diff ${{ github.event.pull_request.base.sha }}...HEAD --stat | grep -E "test|spec" || echo "no test stat"
258-
git diff ${{ github.event.pull_request.base.sha }}...HEAD --numstat
237+
cat /tmp/gh-aw/agent/diff-numstat.txt
259238
```
260239

261240
For each **Go and JavaScript** test file, find the corresponding production file and compare the ratio of lines added:
@@ -475,3 +454,73 @@ After posting the comment, submit a pull request review based on the verdict:
475454
2. In the PR comment (Step 7), add a note such as: "⚠️ Sampling applied — analyzed the first 50 of N test functions. Prioritized newly added tests."
476455
- Keep individual test analysis concise — 2–3 sentences per test in the flagged section.
477456
- Use `<details>` tags for per-test tables with more than 10 rows.
457+
458+
## agent: `go-test-analyzer`
459+
---
460+
description: Run awk analysis on Go test diff and return per-function stats plus missing build tags
461+
model: small
462+
---
463+
Read the pre-fetched test diff and extract per-function Go test stats:
464+
465+
```bash
466+
cat /tmp/gh-aw/agent/test-diff.txt | awk '
467+
/^\+func Test/ {
468+
if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks
469+
match($0, /func (Test[^(]+)/, arr); test_name=arr[1]; assertions=0; errors=0; table_driven=0; forbidden_mocks=0
470+
}
471+
test_name && /^\+.*(assert\.|require\.)/ { assertions++ }
472+
test_name && /^\+.*t\.(Error|Errorf|Fatal|Fatalf)\(/ { assertions++; errors++ }
473+
test_name && /^\+.*(assert\.Error|require\.Error|assert\.NoError|require\.NoError)/ { errors++ }
474+
test_name && /^\+.*t\.Run\(/ { table_driven++ }
475+
test_name && /^\+.*(gomock\.|testify\/mock|\.EXPECT\(\)|\.On\(|\.Return\()/ { forbidden_mocks++ }
476+
test_name && /^\+\}$/ { print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks; test_name="" }
477+
END { if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "table_driven=" table_driven, "forbidden_mocks=" forbidden_mocks }
478+
'
479+
```
480+
481+
Also check for newly added Go test files missing the mandatory build tag:
482+
483+
```bash
484+
git diff ${{ github.event.pull_request.base.sha }}...HEAD --diff-filter=A --name-only | grep '_test\.go$' | while read f; do
485+
if ! head -1 "$f" | grep -qE '^//go:build'; then
486+
echo "MISSING BUILD TAG: $f"
487+
fi
488+
done
489+
```
490+
491+
Return:
492+
1. A markdown table with this exact header:
493+
`| Test Function | Assertions | Error Checks | Table-Driven Subtests | Forbidden Mock Calls |`
494+
Example row:
495+
`| TestCompile | 4 | 2 | 1 | 0 |`
496+
2. A `Missing Build Tags` section listing any `MISSING BUILD TAG: <file>` lines, or `None.`
497+
3. If no Go test functions are in the diff, return: `No Go test functions found in diff.`
498+
499+
## agent: `js-test-analyzer`
500+
---
501+
description: Run awk analysis on JavaScript vitest diff and return per-test stats
502+
model: small
503+
---
504+
Read the pre-fetched test diff and extract per-test JavaScript vitest stats:
505+
506+
```bash
507+
cat /tmp/gh-aw/agent/test-diff.txt | awk '
508+
/^\+(it|test)\(/ {
509+
if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks
510+
match($0, /(it|test)\(["'"'"']([^"'"'"']+)/, arr); test_name=arr[2]; assertions=0; errors=0; mocks=0
511+
}
512+
test_name && /^\+.*expect\(/ { assertions++ }
513+
test_name && /^\+.*(\.toThrow|\.rejects|\.toThrowError)/ { errors++ }
514+
test_name && /^\+.*(vi\.mock|vi\.spyOn|vi\.fn)/ { mocks++ }
515+
test_name && /^\+\}\)/ { print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks; test_name="" }
516+
END { if (test_name) print test_name, "assertions=" assertions, "errors=" errors, "mocks=" mocks }
517+
'
518+
```
519+
520+
Return a markdown table with this exact header:
521+
`| Test Name | Assertions | Error Matchers | vi.* Mock Calls |`
522+
523+
Example row:
524+
`| should_validate_input | 3 | 1 | 0 |`
525+
526+
If no JavaScript test blocks are in the diff, return: `No JavaScript test blocks found in diff.`

0 commit comments

Comments
 (0)