Test Anti-Pattern Detection
Analyze .NET test code for anti-patterns, code smells, and quality issues that undermine test reliability, maintainability, and diagnostic value.
When to Use
- User asks to review test quality or find test smells
- User wants to know why tests are flaky or unreliable
- User asks "are my tests good?" or "what's wrong with my tests?"
- User requests a test audit or test code review
- User wants to improve existing test code
When Not to Use
- User wants to write new tests from scratch (use
writing-mstest-tests)
- User wants to run or execute tests (use
run-tests)
- User wants to migrate between test frameworks or versions (use migration skills)
- User wants to measure code coverage (out of scope)
Inputs
| Input |
Required |
Description |
| Test code |
Yes |
One or more test files or classes to analyze |
| Production code |
No |
The code under test, for context on what tests should verify |
| Specific concern |
No |
A focused area like "flakiness" or "naming" to narrow the review |
Workflow
Step 1: Gather the test code
Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files (files containing [TestClass], [TestMethod], [Fact], [Test], or [Theory] attributes).
If production code is available, read it too — this is critical for detecting tests that are coupled to implementation details rather than behavior.
Step 2: Scan for anti-patterns
Check each test file against the anti-pattern catalog below. Report findings grouped by severity.
Critical — Tests that give false confidence
| Anti-Pattern |
What to Look For |
| No assertions |
Test methods that execute code but never assert anything. A passing test without assertions proves nothing. |
| Swallowed exceptions |
try { ... } catch { } or catch (Exception) without rethrowing or asserting. Failures are silently hidden. |
| Assert in catch block only |
try { Act(); } catch (Exception ex) { Assert.Fail(ex.Message); } — use Assert.ThrowsException or equivalent instead. The test passes when no exception is thrown even if the result is wrong. |
| Always-true assertions |
Assert.IsTrue(true), Assert.AreEqual(x, x), or conditions that can never fail. |
| Commented-out assertions |
Assertions that were disabled but the test still runs, giving the illusion of coverage. |
High — Tests likely to cause pain
| Anti-Pattern |
What to Look For |
| Flakiness indicators |
Thread.Sleep(...), Task.Delay(...) for synchronization, DateTime.Now/DateTime.UtcNow without abstraction, Random without a seed, environment-dependent paths. |
| Test ordering dependency |
Static mutable fields modified across tests, [TestInitialize] that doesn't fully reset state, tests that fail when run individually but pass in suite (or vice versa). |
| Over-mocking |
More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. |
| Implementation coupling |
Testing private methods via reflection, asserting on internal state, verifying exact method call counts on collaborators instead of observable behavior. |
| Broad exception assertions |
Assert.ThrowsException<Exception>(...) instead of the specific exception type. Also: [ExpectedException(typeof(Exception))]. |
Medium — Maintainability and clarity issues
| Anti-Pattern |
What to Look For |
| Poor naming |
Test names like Test1, TestMethod, names that don't describe the scenario or expected outcome. Good: Add_NegativeNumber_ThrowsArgumentException. |
| Magic values |
Unexplained numbers or strings in arrange/assert: Assert.AreEqual(42, result) — what does 42 mean? |
| Duplicate tests |
Three or more test methods with near-identical bodies that differ only in a single input value. Should be data-driven ([DataRow], [Theory], [TestCase]). Note: Two tests covering distinct boundary conditions (e.g., zero vs. negative) are NOT duplicates — separate tests for different edge cases provide clearer failure diagnostics and are a valid practice. |
| Giant tests |
Test methods exceeding ~30 lines or testing multiple behaviors at once. Hard to diagnose when they fail. |
| Assertion messages that repeat the assertion |
Assert.AreEqual(expected, actual, "Expected and actual are not equal") adds no information. Messages should describe the business meaning. |
| Missing AAA separation |
Arrange, Act, Assert phases are interleaved or indistinguishable. |
Low — Style and hygiene
| Anti-Pattern |
What to Look For |
| Unused test infrastructure |
[TestInitialize]/[SetUp] that does nothing, test helper methods that are never called. |
| IDisposable not disposed |
Test creates HttpClient, Stream, or other disposable objects without using or cleanup. |
| Console.WriteLine debugging |
Leftover Console.WriteLine or Debug.WriteLine statements used during test development. |
| Inconsistent naming convention |
Mix of naming styles in the same test class (e.g., some use Method_Scenario_Expected, others use ShouldDoSomething). |
Step 3: Calibrate severity honestly
Before reporting, re-check each finding against these severity rules:
- Critical/High: Only for issues that cause tests to give false confidence or be unreliable. A test that always passes regardless of correctness is Critical. Flaky shared state is High.
- Medium: Only for issues that actively harm maintainability — 5+ nearly-identical tests, truly meaningless names like
Test1.
- Low: Cosmetic naming mismatches, minor style preferences, assertion messages that could be better. When in doubt, rate Low.
- Not an issue: Separate tests for distinct boundary conditions (zero vs. negative vs. null). Explicit per-test setup instead of
[TestInitialize] (this improves isolation). Tests that are short and clear but could theoretically be consolidated.
IMPORTANT: If the tests are well-written, say so clearly up front. Do not inflate severity to justify the review. A review that finds zero Critical/High issues and only minor Low suggestions is a valid and valuable outcome. Lead with what the tests do well.
Step 4: Report findings
Present findings in this structure:
- Summary — Total issues found, broken down by severity (Critical / High / Medium / Low). If tests are well-written, lead with that assessment.
- Critical and High findings — List each with:
- The anti-pattern name
- The specific location (file, method name, line)
- A brief explanation of why it's a problem
- A concrete fix (show before/after code when helpful)
- Medium and Low findings — Summarize in a table unless the user wants full detail
- Positive observations — Call out things the tests do well (sealed class, specific exception types, data-driven tests, clear AAA structure, proper use of fakes, good naming). Don't only report negatives.
Step 4: Prioritize recommendations
If there are many findings, recommend which to fix first:
- Critical — Fix immediately, these tests may be giving false confidence
- High — Fix soon, these cause flakiness or maintenance burden
- Medium/Low — Fix opportunistically during related edits
Validation
Common Pitfalls
| Pitfall |
Solution |
| Reporting style issues as critical |
Naming and formatting are Medium/Low, never Critical |
| Suggesting rewrites instead of targeted fixes |
Show minimal diffs — change the assertion, not the whole test |
| Flagging intentional design choices |
If Thread.Sleep is in an integration test testing actual timing, that's not an anti-pattern. Consider context. |
| Inventing false positives on clean code |
If tests follow best practices, say so. A review finding "0 Critical, 0 High, 1 Low" is perfectly valid. Don't inflate findings to justify the review. |
| Flagging separate boundary tests as duplicates |
Two tests for zero and negative inputs test different edge cases. Only flag as duplicates when 3+ tests have truly identical bodies differing by a single value. |
| Rating cosmetic issues as Medium |
Naming mismatches (e.g., method name says ArgumentException but asserts ArgumentOutOfRangeException) are Low, not Medium — the test still works correctly. |
| Ignoring the test framework |
xUnit uses [Fact]/[Theory], NUnit uses [Test]/[TestCase], MSTest uses [TestMethod]/[DataRow] — use correct terminology |
| Missing the forest for the trees |
If 80% of tests have no assertions, lead with that systemic issue rather than listing every instance |
1---2name: exp-test-anti-patterns3description: Detects anti-patterns and code smells in .NET test suites. Use when the user asks to review test quality, find test smells, identify flaky test indicators, or audit tests for common mistakes. Covers assertion quality, test isolation, naming, flakiness indicators, over-mocking, and structural problems. Works with MSTest, xUnit, NUnit, and TUnit.4---5
6# Test Anti-Pattern Detection
7
8Analyze .NET test code for anti-patterns, code smells, and quality issues that undermine test reliability, maintainability, and diagnostic value.
9
10## When to Use
11
12- User asks to review test quality or find test smells
13- User wants to know why tests are flaky or unreliable
14- User asks "are my tests good?" or "what's wrong with my tests?"
15- User requests a test audit or test code review
16- User wants to improve existing test code
17
18## When Not to Use
19
20- User wants to write new tests from scratch (use `writing-mstest-tests`)
21- User wants to run or execute tests (use `run-tests`)
22- User wants to migrate between test frameworks or versions (use migration skills)
23- User wants to measure code coverage (out of scope)
24
25## Inputs
26
27| Input | Required | Description |
28|-------|----------|-------------|
29| Test code | Yes | One or more test files or classes to analyze |
30| Production code | No | The code under test, for context on what tests should verify |
31| Specific concern | No | A focused area like "flakiness" or "naming" to narrow the review |
32
33## Workflow
34
35### Step 1: Gather the test code
36
37Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files (files containing `[TestClass]`, `[TestMethod]`, `[Fact]`, `[Test]`, or `[Theory]` attributes).
38
39If production code is available, read it too — this is critical for detecting tests that are coupled to implementation details rather than behavior.
40
41### Step 2: Scan for anti-patterns
42
43Check each test file against the anti-pattern catalog below. Report findings grouped by severity.
44
45#### Critical — Tests that give false confidence
46
47| Anti-Pattern | What to Look For |
48|---|---|
49| **No assertions** | Test methods that execute code but never assert anything. A passing test without assertions proves nothing. |
50| **Swallowed exceptions** | `try { ... } catch { }` or `catch (Exception)` without rethrowing or asserting. Failures are silently hidden. |
51| **Assert in catch block only** | `try { Act(); } catch (Exception ex) { Assert.Fail(ex.Message); }` — use `Assert.ThrowsException` or equivalent instead. The test passes when no exception is thrown even if the result is wrong. |
52| **Always-true assertions** | `Assert.IsTrue(true)`, `Assert.AreEqual(x, x)`, or conditions that can never fail. |
53| **Commented-out assertions** | Assertions that were disabled but the test still runs, giving the illusion of coverage. |
54
55#### High — Tests likely to cause pain
56
57| Anti-Pattern | What to Look For |
58|---|---|
59| **Flakiness indicators** | `Thread.Sleep(...)`, `Task.Delay(...)` for synchronization, `DateTime.Now`/`DateTime.UtcNow` without abstraction, `Random` without a seed, environment-dependent paths. |
60| **Test ordering dependency** | Static mutable fields modified across tests, `[TestInitialize]` that doesn't fully reset state, tests that fail when run individually but pass in suite (or vice versa). |
61| **Over-mocking** | More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. |
62| **Implementation coupling** | Testing private methods via reflection, asserting on internal state, verifying exact method call counts on collaborators instead of observable behavior. |
63| **Broad exception assertions** | `Assert.ThrowsException<Exception>(...)` instead of the specific exception type. Also: `[ExpectedException(typeof(Exception))]`. |
64
65#### Medium — Maintainability and clarity issues
66
67| Anti-Pattern | What to Look For |
68|---|---|
69| **Poor naming** | Test names like `Test1`, `TestMethod`, names that don't describe the scenario or expected outcome. Good: `Add_NegativeNumber_ThrowsArgumentException`. |
70| **Magic values** | Unexplained numbers or strings in arrange/assert: `Assert.AreEqual(42, result)` — what does 42 mean? |
71| **Duplicate tests** | Three or more test methods with near-identical bodies that differ only in a single input value. Should be data-driven (`[DataRow]`, `[Theory]`, `[TestCase]`). Note: Two tests covering distinct boundary conditions (e.g., zero vs. negative) are NOT duplicates — separate tests for different edge cases provide clearer failure diagnostics and are a valid practice. |
72| **Giant tests** | Test methods exceeding ~30 lines or testing multiple behaviors at once. Hard to diagnose when they fail. |
73| **Assertion messages that repeat the assertion** | `Assert.AreEqual(expected, actual, "Expected and actual are not equal")` adds no information. Messages should describe the business meaning. |
74| **Missing AAA separation** | Arrange, Act, Assert phases are interleaved or indistinguishable. |
75
76#### Low — Style and hygiene
77
78| Anti-Pattern | What to Look For |
79|---|---|
80| **Unused test infrastructure** | `[TestInitialize]`/`[SetUp]` that does nothing, test helper methods that are never called. |
81| **IDisposable not disposed** | Test creates `HttpClient`, `Stream`, or other disposable objects without `using` or cleanup. |
82| **Console.WriteLine debugging** | Leftover `Console.WriteLine` or `Debug.WriteLine` statements used during test development. |
83| **Inconsistent naming convention** | Mix of naming styles in the same test class (e.g., some use `Method_Scenario_Expected`, others use `ShouldDoSomething`). |
84
85### Step 3: Calibrate severity honestly
86
87Before reporting, re-check each finding against these severity rules:
88
89- **Critical/High**: Only for issues that cause tests to give false confidence or be unreliable. A test that always passes regardless of correctness is Critical. Flaky shared state is High.
90- **Medium**: Only for issues that actively harm maintainability — 5+ nearly-identical tests, truly meaningless names like `Test1`.
91- **Low**: Cosmetic naming mismatches, minor style preferences, assertion messages that could be better. When in doubt, rate Low.
92- **Not an issue**: Separate tests for distinct boundary conditions (zero vs. negative vs. null). Explicit per-test setup instead of `[TestInitialize]` (this *improves* isolation). Tests that are short and clear but could theoretically be consolidated.
93
94IMPORTANT: If the tests are well-written, say so clearly up front. Do not inflate severity to justify the review. A review that finds zero Critical/High issues and only minor Low suggestions is a valid and valuable outcome. Lead with what the tests do well.
95
96### Step 4: Report findings
97
98Present findings in this structure:
99
1001. **Summary** — Total issues found, broken down by severity (Critical / High / Medium / Low). If tests are well-written, lead with that assessment.
1012. **Critical and High findings** — List each with:
102 - The anti-pattern name
103 - The specific location (file, method name, line)
104 - A brief explanation of why it's a problem
105 - A concrete fix (show before/after code when helpful)
1063. **Medium and Low findings** — Summarize in a table unless the user wants full detail
1074. **Positive observations** — Call out things the tests do well (sealed class, specific exception types, data-driven tests, clear AAA structure, proper use of fakes, good naming). Don't only report negatives.
108
109### Step 4: Prioritize recommendations
110
111If there are many findings, recommend which to fix first:
112
1131. **Critical** — Fix immediately, these tests may be giving false confidence
1142. **High** — Fix soon, these cause flakiness or maintenance burden
1153. **Medium/Low** — Fix opportunistically during related edits
116
117## Validation
118
119- [ ] Every finding includes a specific location (not just a general warning)
120- [ ] Every Critical/High finding includes a concrete fix
121- [ ] Report covers all categories (assertions, isolation, naming, structure)
122- [ ] Positive observations are included alongside problems
123- [ ] Recommendations are prioritized by severity
124
125## Common Pitfalls
126
127| Pitfall | Solution |
128|---------|----------|
129| Reporting style issues as critical | Naming and formatting are Medium/Low, never Critical |
130| Suggesting rewrites instead of targeted fixes | Show minimal diffs — change the assertion, not the whole test |
131| Flagging intentional design choices | If `Thread.Sleep` is in an integration test testing actual timing, that's not an anti-pattern. Consider context. |
132| Inventing false positives on clean code | If tests follow best practices, say so. A review finding "0 Critical, 0 High, 1 Low" is perfectly valid. Don't inflate findings to justify the review. |
133| Flagging separate boundary tests as duplicates | Two tests for zero and negative inputs test different edge cases. Only flag as duplicates when 3+ tests have truly identical bodies differing by a single value. |
134| Rating cosmetic issues as Medium | Naming mismatches (e.g., method name says `ArgumentException` but asserts `ArgumentOutOfRangeException`) are Low, not Medium — the test still works correctly. |
135| Ignoring the test framework | xUnit uses `[Fact]`/`[Theory]`, NUnit uses `[Test]`/`[TestCase]`, MSTest uses `[TestMethod]`/`[DataRow]` — use correct terminology |
136| Missing the forest for the trees | If 80% of tests have no assertions, lead with that systemic issue rather than listing every instance |
majiayu000