Skip to content

Add Test Expert Reviewer workflow#7872

Merged
Evangelink merged 3 commits intomainfrom
dev/amauryleve/add-test-expert-reviewer
Apr 27, 2026
Merged

Add Test Expert Reviewer workflow#7872
Evangelink merged 3 commits intomainfrom
dev/amauryleve/add-test-expert-reviewer

Conversation

@Evangelink
Copy link
Copy Markdown
Member

Summary

Add a Test Expert Reviewer that focuses exclusively on test code quality — isolation, assertions, flakiness prevention, coverage, and structure.

Review categories (7 analysis lenses)

# Category What it catches
4.1 Test Isolation Static mutable fields, shared state, env variable pollution, missing cleanup
4.2 Assertion Quality Assert.IsTrue(a == b)AreEqual, wrong assertion methods, over-assertion on implementation details
4.3 Flakiness Patterns Thread.Sleep, hard-coded ports, wall-clock assertions, order-dependent collection checks
4.4 Test Completeness New public methods without tests, missing boundary values, untested error paths
4.5 Data-Driven Coverage Missing null/empty/boundary [DataRow] values, redundant test cases, missing negative cases
4.6 Test Structure Missing Arrange/Act/Assert, multiple acts, copy-paste tests that should be parameterized
4.7 Test Performance Real I/O in unit tests, heavy DI graph construction, unnecessarily async tests

Trigger model

  • Auto: Runs on pull_request: opened
  • On-demand: /test-review slash command

Separation of concerns

Reviewer Scope Files reviewed
Nitpick Reviewer Style, naming, formatting All files
Expert Reviewer Correctness, perf, threading, API compat src/ files
Test Expert Reviewer (this PR) Test quality, isolation, flakiness test/ files

The test reviewer explicitly scopes to test files and defers production code review to the Expert Reviewer.

Portability

This reviewer is designed to be portable to other MSTest repositories — the analysis categories are framework-aware but not repo-specific. The only testfx-specific line is the context note about TestFramework.ForTestingMSTest.

Max comments: 7 (vs 5 for expert, 10 for nitpick)

Test PRs often touch more files than production PRs, and test quality issues are typically easier to fix, so a slightly higher budget is justified.

Before merging

gh aw compile must be run to generate pr-test-expert-reviewer.lock.yml.

New agentic workflow that reviews test code quality on all opened PRs,
focusing on:
- Test isolation (shared state, static field pollution, cleanup)
- Assertion quality (weak assertions, wrong assertion methods)
- Flakiness patterns (timing, ports, file system races)
- Test completeness (missing edge cases, untested error paths)
- Data-driven coverage ([DataRow] edge cases, boundary values)
- Test structure (Arrange/Act/Assert, copy-paste tests)
- Test performance (unnecessary I/O, heavy DI in unit tests)

Also available on-demand via /test-review slash command.
Explicitly scoped to test files only — production code review is
handled by the Expert Code Reviewer.

Designed to be portable to other MSTest repositories.

Requires `gh aw compile` to generate the .lock.yml file.
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new agentic workflow to provide a dedicated, test-focused PR reviewer for MSTest repositories, emphasizing test isolation, assertion quality, flakiness prevention, and coverage/structure.

Changes:

  • Introduces pr-test-expert-reviewer.md workflow definition with /test-review on-demand triggering plus pull_request: opened auto-triggering.
  • Defines a 7-lens review rubric (isolation, assertions, flakiness, completeness, data-driven coverage, structure, performance) and a max budget of 7 inline comments.
  • Adds cache-memory based context loading and a deduplication step to avoid duplicate reviews.
Show a summary per file
File Description
.github/workflows/pr-test-expert-reviewer.md New agentic workflow prompt/config for a test-quality-focused PR reviewer.

Copilot's findings

  • Files reviewed: 1/1 changed files
  • Comments generated: 2

Comment thread .github/workflows/pr-test-expert-reviewer.md Outdated
Comment thread .github/workflows/pr-test-expert-reviewer.md Outdated
Evangelink and others added 2 commits April 27, 2026 18:11
Add blank lines between bold cache-memory file headings and their
associated list items in Step 6.
Copilot AI review requested due to automatic review settings April 27, 2026 16:17
@Evangelink Evangelink enabled auto-merge (squash) April 27, 2026 16:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 1

Comment thread .github/workflows/pr-test-expert-reviewer.lock.yml
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-04-27
Repository: microsoft/testfx

Key Findings

This PR adds a new GitHub Actions agentic workflow (pr-test-expert-reviewer.md + compiled pr-test-expert-reviewer.lock.yml). No .cs or other production/test .NET code is changed, so the standard Expert Code Reviewer categories (algorithmic correctness, threading, performance, .NET API compatibility, cross-TFM, IDisposable management) do not apply.

Security & Workflow Configuration — No issues found:

  • ✅ Top-level permissions: {} (deny-all default) with minimal per-job grants (pull-requests: write only on the two jobs that actually post review comments)
  • ✅ No bash tool access declared — the reviewer is read-only, which is the correct security posture
  • ✅ Fork PR protection: the activation job's if: condition checks github.event.pull_request.head.repo.id == github.repository_id, preventing fork PRs from triggering with elevated permissions
  • ✅ All third-party action refs are pinned to SHA digests, not mutable tags
  • ✅ Concurrency group (cancel-in-progress: true) prevents parallel duplicate runs

Recommendations

None — no actionable issues found in the Expert Code Reviewer scope for this PR.


Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

@Evangelink Evangelink merged commit 6e4ba08 into main Apr 27, 2026
21 checks passed
@Evangelink Evangelink deleted the dev/amauryleve/add-test-expert-reviewer branch April 27, 2026 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants