OverviewHistoryStatsSecurity
npx skills add ...
Documentation
SKILL.md
npx skills add dotnet/skills --skill test-anti-patterns
Audit a test file or suite; produce a severity-ranked diagnostic report. ALWAYS USE for tests that verify nothing, missing/tautological assertions, swallowed/broad exceptions, flaky/order-dependent tests, duplication, or magic values. Polyglot. DO NOT USE for direct edits: writing-mstest-tests owns supplied MSTest assertions/attributes/lifecycle; code-testing-agent owns new tests. Exclude running tests, migration, assertion metrics (assertion-quality), raw .NET coverage collection (run-tests), non-.NET coverage collection/analysis (native tooling), project-wide .NET coverage/CRAP (coverage-analysis), named-target .NET CRAP (crap-score), behavioral/pseudo-mutation gaps (test-gap-analysis), test-mix/ happy-vs-error classification and trait distributions (test-tagging), or the testsmells.org catalog (test-smell-detection).
npx skills add dotnet/skills --skill test-anti-patterns
Quick, pragmatic analysis of test code in any supported language for anti-patterns and quality issues that undermine test reliability, maintainability, and diagnostic value.
Language-specific guidance: Try
test-analysis-extensionsonce. If it is unavailable, continue immediately with this skill's built-in framework rules; never block the audit on the helper.
code-testing-agent)Assert.AreEqual argument order in MSTest (use writing-mstest-tests)DynamicData from IEnumerable<object[]> to ValueTuple (use writing-mstest-tests)run-tests for .NET)run-tests), non-.NET coverage collection or analysis (use native tooling), project-wide .NET coverage/CRAP metrics (use coverage-analysis), or named-target .NET CRAP (use crap-score)test-gap-analysis)test-tagging)test-smell-detection)| Input | Required | Description |
|---|---|---|
| Test scope | No | Test files, classes, directory, or project to analyze. Discover from the current workspace when omitted. |
| 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 |
Resolve the named test path from the current workspace before asking for input.
When no path is supplied, discover test files under the current directory using
the repository manifests and conventional test markers. The skill context's
Base directory is documentation storage, not the user's workspace; never
resolve target files relative to it.
If one reader says a path is missing but a workspace glob/search finds it,
normalize that exact path and retry. Use a shell text reader (sed/cat on
Unix, Get-Content on PowerShell) only for a confirmed reader availability,
transport, or path-normalization failure and only after verifying the canonical
path remains inside the current workspace. Stop on content-exclusion,
permission/policy, workspace-boundary, or unknown failures. Audit any discovered
file that a permitted reader can access; never ask the user to paste it. If
every permitted reader fails, report the exact blocker without bypassing
security boundaries.
Identify the language and framework. Try the matching
test-analysis-extensions guidance once; if unavailable, use the catalog below.
Read every test file in the resolved scope. Use extension discovery markers
when loaded; otherwise use the built-in markers in this skill (attributes such
as [TestClass]/[Fact]/[Test], test_*.py, *.test.*, *_test.go,
*_spec.rb, #[test], *.Tests.ps1, TEST(...), and TEST_CASE(...)).
If production code is available, read it too -- this is critical for detecting tests that are coupled to implementation details rather than behavior.
Check each test file against the anti-pattern catalog below. Report findings grouped by severity. Use extension mappings when loaded; otherwise use the cross-framework examples in the catalog.
Before drafting the report, make a private completeness ledger with one row for every test method and every class-level fixture/resource. Record its oracle (or absence), exception handling, state/time dependencies, and disposition. Do not publish until every row is either attached to a finding or explicitly judged sound. In particular:
actual != oldValue is a weak mutation oracle: it accepts every wrong new
value. Require the exact expected value.HttpClient.test-gap-analysis.| 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. |
| 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). |
| Weak transformation oracle | A normalization, casing, trimming, mapping, or conversion test supplies an input already in the expected form, so a no-op implementation passes even though the assertion may catch other defects. Use an input that must change and assert an independently derived expected value. A producer/consumer round trip is useful but does not replace an independent format assertion when both sides could share the same defect. |
| Anti-Pattern | What to Look For |
|---|---|
| Poor naming | Test names like Test1, TestMethod, or test that don't describe the scenario or outcome. Use the loaded extension when available; otherwise follow the existing naming convention in the same suite. |
| 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. |
| 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). |
Before reporting, re-check each finding against these severity rules:
Test1 / test / it1.1 finding / 6 affected tests, not
six findings plus a seventh summary finding, and do not downgrade one instance
merely to manufacture multiple tiers.t.Run / for case in cases { ... }) are idiomatic, not "Conditional Test Logic". Do NOT flag.assert is the canonical assertion form, not a missing assertion library. Do NOT flag.if got != want { t.Errorf(...) } as canonical equality. Do NOT flag as ad-hoc.[TestInitialize] / beforeEach (this improves isolation).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.
Depth bar — a tidy report that is shallower than an unassisted review is a failure. Before writing, satisfy all five:
static HttpClient field) is incomplete. State the number reviewed.// assert something here placeholder.test-gap-analysis for exhaustive
branch-by-branch behavioral gaps.Present findings in this structure:
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.
If there are many findings, recommend which to fix first:
| 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 / time.sleep / time.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 | Use the terminology of the framework you loaded from the language extension; don't describe a pytest suite in MSTest terms. |
| Missing the forest for the trees | If 80% of tests have no assertions, lead with that systemic issue rather than listing every instance |
| Trading depth for tidiness | A severity table and positive observations do not substitute for coverage of every test, exact expected values in fixes, and the adjacent error-path/boundary gaps |
| Contradicting yourself in the report | Reason first, then write one settled verdict per finding — never emit "wait, that's wrong" / "should fail but doesn't" reconsiderations |
| Counts that don't add up | The summary's per-severity totals must match the findings you listed |