Test Anti-Pattern Detection

Workflow

Step 1: Detect language and load extension

Identify the target codebase's language and test framework. Call the test-analysis-extensions skill and read the matching extension file. The extension file documents framework-specific anti-pattern markers — what counts as a sleep/wait, a test marker, a skip, a setup/teardown, a shared-state hot spot, and an integration boundary — so this skill stays language-neutral.

Step 2: 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 using the discovery markers in the loaded language extension file (e.g., [TestClass]/[Fact]/[Test] for .NET, test_*.py / def test_* for pytest, *.test.ts / it() for Jest, *Test.java / @Test for JUnit, *_test.go / func TestXxx for Go, *_spec.rb for RSpec, #[test] for Rust, *.Tests.ps1 / Describe for Pester, TEST(...) for GoogleTest, TEST_CASE(...) for Catch2/doctest).

If production code is available, read it too -- this is critical for detecting tests that are coupled to implementation details rather than behavior.

Step 3: Scan for anti-patterns

Check each test file against the anti-pattern catalog below. Report findings grouped by severity. The examples are .NET-centric but the patterns generalize — use the loaded language extension file to map each pattern to the framework you are auditing.

#### 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. In .NET look for missing Assert.*; in pytest a function with no assert and no pytest.raises; in Jest no expect(...); in JUnit no assert*/assertThat; in Go a test that never calls t.Error*, t.Fatal*, or testify; in RSpec a block with no expect; in Pester no Should. Mock-call verifications (verify(mock), expect(mock).toHaveBeenCalled, Should -Invoke) are real assertions. | | Missing await on async assertions (JS/TS, .NET, Python, Kotlin, Swift) | expect(promise).resolves.toBe(x) without await/return, pytest-asyncio test with un-awaited coroutine, async Task xUnit test calling Assert.ThrowsAsync without await, Kotest suspending test without runTest, Swift Testing async test without await. These tests silently pass even when the underlying assertion would have failed. | | Coverage touching | Test class that methodically calls every public member on a type — often in alphabetical or declaration order — without asserting meaningful outcomes. Each test typically does var result = sut.MethodName(...) (or result = sut.method_name(...), sut.methodName(), sut.MethodName(t)) with no assertion, or only a trivial null/None/nil check. The intent is to inflate code-coverage metrics rather than verify behavior. Distinct from a single assertion-free test: the pattern is *systematic* coverage of the surface area with no real verification. | | Self-referential assertion | The expected value is computed from the same actual value, such as Assert.AreEqual(dto.Name, dto.Name), Assert.AreEqual(result, result), or equivalents. Do not apply this label merely because a valid identity, clone, serialization, or round-trip contract compares output with input: those assertions can fail. Instead check whether the input exercises a transformation and whether independently known representation, field, reference-identity, or invalid-input assertions are missing. | | Swallowed exceptions | try { ... } catch { }, catch (Exception) without rethrowing or asserting (.NET); bare except: or except Exception: with pass (Python); try { ... } catch (e) {} (JS/TS/Java); defer recover() without re-panic and no assertion (Go); rescue StandardError with no assertion (Ruby); Result::unwrap_or(...) swallowing errors in a test (Rust); empty catch block (Kotlin/Swift). | | Assert in catch block only | try { Act(); } catch (Exception ex) { Assert.Fail(ex.Message); } (and equivalents in other languages) -- use Assert.ThrowsException / pytest.raises / expect(fn).toThrow / assertThrows / assert.Error(t, err) / #[should_panic] / Should -Throw / EXPECT_THROW 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), assert True, expect(true).toBe(true), assert.True(t, true), assert!(true), 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 | Wall-clock sleeps/waits used for synchronization: Thread.Sleep / Task.Delay (.NET), time.sleep (Python), setTimeout / await new Promise(r => setTimeout(...)) (JS/TS), Thread.sleep (Java/Kotlin), time.Sleep (Go), sleep (Ruby/Bash), std::thread::sleep (Rust), Start-Sleep (Pester), std::this_thread::sleep_for (C++). Wall-clock reads without abstraction: DateTime.Now/UtcNow, datetime.now()/datetime.utcnow(), Date.now() / new Date(), System.currentTimeMillis(), time.Now(), Time.now, Instant::now(), Date()/Date.now, Get-Date, std::chrono::system_clock::now. Unseeded randomness: new Random(), random.random()/random.randint(), Math.random(), new Random() (Java/Kotlin), rand.Int() without seed, rand (Ruby), rand::random() (Rust). Environment-dependent paths (hard-coded C:\..., /tmp/..., network hosts). | | Test ordering dependency | Static/global mutable state modified across tests; setup that doesn't fully reset state ([TestInitialize], setUp, beforeEach, before(:each), BeforeEach, t.Cleanup); tests that fail when run individually but pass in suite (or vice versa). Examples per language: static fields (.NET/Java), module-level globals (Python), top-level let/const in test file (JS/TS), var package globals (Go), class variables (Ruby), static mut/lazy_static!/OnceCell (Rust), $script: variables (PowerShell). | | Over-mocking | More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. Per language: Moq/NSubstitute/FakeItEasy (.NET), unittest.mock / pytest-mock (Python), Jest auto-mocks / Sinon (JS/TS), Mockito/PowerMock (Java), gomock/testify mock (Go), RSpec mocks/mocha (Ruby), mockall (Rust), MockK (Kotlin), Mock cmdlet (Pester), gmock (C++). For a deep mock audit in .NET, use exp-mock-usage-analysis. | | Implementation coupling | Testing private methods via reflection (MethodInfo.Invoke, getattr in Python, (thing as any) in TS, Field.setAccessible(true) in Java, Object#send in Ruby, internal pub(crate) access in Rust). Asserting on internal state instead of observable behavior. Verifying exact method call counts on collaborators instead of business outcomes. | | Broad exception assertions | Assert.ThrowsException<Exception>(...) (.NET) / pytest.raises(Exception) / expect(fn).toThrow(Error) without a message matcher / assertThrows(Exception.class, ...) (Java) / assert.Error(t, err) without checking the kind / expect { ... }.to raise_error without class (RSpec) / #[should_panic] without expected = "..." / Should -Throw without -ExpectedMessage / EXPECT_ANY_THROW instead of EXPECT_THROW(stmt, SpecificType). |

#### Medium -- Maintainability and clarity issues

| Anti-Pattern | What to Look For | |---|---| | Poor naming | Test names like Test1, TestMethod, test, names that don't describe the scenario or expected outcome. Good naming differs by language convention — see the loaded language extension file (e.g., Add_NegativeNumber_ThrowsArgumentException for .NET, test_add_negative_number_raises_value_error for pytest, addNegativeNumber_throwsArgumentException for Java, 'adds negative number throws' for Jest descriptions, TestAdd_NegativeNumber_ReturnsError for Go). | | Magic values | Unexplained numbers or strings in arrange/assert: Assert.AreEqual(42, result) / assert result == 42 / expect(result).toBe(42) -- 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 parametrized: [DataRow]/[Theory]/[TestCase] (.NET), @pytest.mark.parametrize (pytest), test.each / it.each (Jest/Vitest), @ParameterizedTest + @ValueSource (JUnit 5), @DataProvider (TestNG), Go table-driven tests, where / shared examples (RSpec), #[rstest] (Rust), @ParameterizedTest + @MethodSource (Kotlin), -ForEach / -TestCases (Pester), INSTANTIATE_TEST_SUITE_P (GoogleTest), SECTION / GENERATE (Catch2), TEST_CASE_TEMPLATE (doctest). For a detailed duplication analysis in .NET, use exp-test-maintainability. 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") / assert x == y, "x is not equal to y" / assertEquals(x, y, "values not equal") add no information. Messages should describe the business meaning. | | Missing AAA / Given-When-Then separation | Arrange/Act/Assert (or Given/When/Then for BDD frameworks like RSpec, Kotest behavior specs, Pester) phases are interleaved or indistinguishable. |

#### Low -- Style and hygiene

| Anti-Pattern | What to Look For | |---|---| | Unused test infrastructure | Setup/teardown hooks that do nothing — [TestInitialize]/[SetUp]/[BeforeEach], setUp/@BeforeEach/@BeforeAll, beforeEach/beforeAll, before(:each)/before(:all), BeforeEach/BeforeAll (Pester), setUpWithError (XCTest) — and test helper methods that are never called. | | Unmanaged resources | Test creates disposable/closeable resources without cleanup: HttpClient/Stream without using (.NET), file/connection without with block or try/finally (Python), FileInputStream without try-with-resources (Java), defer file.Close() missing (Go), connection without ensure (Ruby), Drop not relied on / forgotten close (Rust), missing teardown for temp files / DBs in any language. | | Print debugging | Leftover Console.WriteLine / Debug.WriteLine / print() / console.log / System.out.println / fmt.Println / puts / dbg! / Write-Host / std::cout statements used during test development. | | Inconsistent naming convention | Mix of naming styles in the same test class/module/file (e.g., some use Method_Scenario_Expected, others use ShouldDoSomething). |

Step 4: 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. Missing-await on async assertions is Critical (silent pass).
• Medium: Only for issues that actively harm maintainability -- 5+ nearly-identical tests, truly meaningless names like Test1 / test / it1.
• Low: Cosmetic naming mismatches, minor style preferences, assertion messages that could be better. When in doubt, rate Low.
• Use the caller's severity vocabulary consistently. If the caller asks for

Critical / Warning / Info, map reliability risks to Warning and maintenance/cosmetic concerns to Info rather than silently collapsing every item into Critical. Severity describes the demonstrated failure mode, not how much prose a finding receives.

• Separate a systemic finding from its instances. Coverage touching across a

facade is one Critical systemic finding whose evidence lists every affected test. All assertion-free instances, including the last facade method, retain the same false-confidence severity. Report 1 finding / 6 affected tests, not six findings plus a seventh summary finding, and do not downgrade one instance merely to manufacture multiple tiers.

• Do not severity-rank ordinary missing cases as anti-patterns. Adjacent

untested branches, exception paths, and boundaries may be useful coverage opportunities, but list them separately from the anti-pattern counts unless a weak existing test specifically creates the gap. They are not Critical merely because the suite has a systemic Critical issue.

• Not an issue (per-language nuance):

- Go and Rust table-driven loops with sub-tests (t.Run / for case in cases { ... }) are *idiomatic*, not "Conditional Test Logic". Do NOT flag. - pytest bare `assert` is the canonical assertion form, not a missing assertion library. Do NOT flag. - Go tests use if got != want { t.Errorf(...) } as canonical equality. Do NOT flag as ad-hoc. - Separate tests for distinct boundary conditions (zero vs. negative vs. null). Do NOT flag as duplicates. - Explicit per-test setup instead of [TestInitialize] / beforeEach (this *improves* isolation). - Tests that are short and clear but could theoretically be consolidated. - Round-trip or serialization equality with non-trivial input. It is valid metamorphic evidence; suggest an independent representation assertion when two implementations could share the same bug. - Clone value equality. Keep it, and add distinct-reference or mutation- independence evidence when the contract promises a deep copy. - A validator or accessor returning the original value when pass-through is the production contract. Missing invalid-input cases are a coverage gap, not proof that the existing assertion is tautological.

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 5: Report findings

Depth bar — a tidy report that is shallower than an unassisted review is a failure. Before writing, satisfy all five:

1. Account for every test in scope. Build the complete method/field inventory

before summarizing. For a systematic pattern such as coverage touching, enumerate every affected test at least once rather than giving representative examples. A finding table that silently skips tests (or fixtures like an unused static HttpClient field) is incomplete. State the number reviewed.

1. Verify the production contract before judging the oracle. Inspect the

actual transformation, DTO fields, and promised identity/clone semantics. Never invent fields or require lossless round-tripping when production is intentionally lossy.

1. Make every Critical/High fix complete and specific. Give the replacement assertion with the *exact expected value* (the computed discount, the exact CSV line, the full expected object), not a // assert something here placeholder.
2. Name the adjacent gaps the tests should also cover — untested error paths, boundary values, and round-trip/culture-sensitivity risks in the same class. These are part of "what's wrong with my tests", and omitting them is the most common way this review loses to an unassisted one.
3. Keep the report internally consistent. Summary counts must equal the enumerated findings. Publish a settled conclusion: do all reconsidering before you write, and never leave "wait, that's wrong" / "this should fail but doesn't" reasoning in the output.

Present findings in this structure:

1. Summary -- Total issues found, broken down by severity (Critical / High / Medium / Low). If tests are well-written, lead with that assessment.
2. 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)

1. Medium and Low findings -- Summarize in a table unless the user wants full detail
2. 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.

Before publishing, assign each finding a stable identity. A grouped row counts as one finding regardless of how many methods it lists; separate rows count separately. Recompute the summary from those rows. Keep affected tests as a different number so a bundled finding cannot create a hidden count mismatch.

Step 6: Prioritize recommendations

If there are many findings, recommend which to fix first:

1. Critical -- Fix immediately, these tests may be giving false confidence
2. High -- Fix soon, these cause flakiness or maintenance burden
3. Medium/Low -- Fix opportunistically during related edits

Related skills