From afb57045b80365cc7e82a75ed5bf08ff55a11aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 19 May 2026 11:55:00 +0200 Subject: [PATCH] test-anti-patterns: merge test-smell-detection in (Variant C) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both skills suffer plugin-mode activation failures: in plugin runs the model bypasses the skill tool entirely for ~5/6 of test-anti-patterns audit prompts and ~2/3 of test-smell-detection prompts, answering directly from training instead. Variant C eliminates sibling competition by merging the two skills: * test-anti-patterns becomes the single umbrella audit skill — its description absorbs every smell-audit trigger phrase and its body references the moved testsmells.org 19-smell catalog for deeper smell-audit needs. * plugins/dotnet-test/skills/test-smell-detection/ is deleted. * references/test-smell-catalog.md is moved under test-anti-patterns. * The 4 test-smell-detection eval scenarios (smelly OrderProcessor, clean Calculator, integration UserRepository, decline-write) are ported into the test-anti-patterns eval suite so coverage is retained, with their fixtures copied under tests/dotnet-test/test-anti-patterns/fixtures/. * tests/dotnet-test/test-smell-detection/ is deleted. * plugins/dotnet-test/README.md and the test-quality-auditor agent are updated to drop test-smell-detection references. Variant C of a 3-PR experiment to fix activation. Companion PRs: * Variant A = activation-only rewrite, both skills keep overlap; * Variant B = narrow specialist for test-smell-detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- plugins/dotnet-test/README.md | 3 +- .../agents/test-quality-auditor.agent.md | 14 +- .../skills/test-anti-patterns/SKILL.md | 34 +-- .../references/test-smell-catalog.md | 0 .../skills/test-smell-detection/SKILL.md | 198 ------------------ .../test-anti-patterns/eval.vally.yaml | 113 ++++++++++ .../dotnet-test/test-anti-patterns/eval.yaml | 100 +++++++++ .../Calculator.Tests/Calculator.Tests.csproj | 0 .../ScientificCalculatorTests.cs | 0 .../ShoppingCart.cs | 0 .../DataAccess.IntegrationTests.csproj | 0 .../UserRepositoryIntegrationTests.cs | 0 .../OrderService.Tests/OrderProcessorTests.cs | 0 .../OrderService.Tests.csproj | 0 .../test-smell-detection/eval.vally.yaml | 117 ----------- .../test-smell-detection/eval.yaml | 120 ----------- 16 files changed, 241 insertions(+), 458 deletions(-) rename plugins/dotnet-test/skills/{test-smell-detection => test-anti-patterns}/references/test-smell-catalog.md (100%) delete mode 100644 plugins/dotnet-test/skills/test-smell-detection/SKILL.md rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs (100%) rename tests/dotnet-test/{test-smell-detection => test-anti-patterns}/fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj (100%) delete mode 100644 tests/dotnet-test/test-smell-detection/eval.vally.yaml delete mode 100644 tests/dotnet-test/test-smell-detection/eval.yaml diff --git a/plugins/dotnet-test/README.md b/plugins/dotnet-test/README.md index 731fdd46f6..06247b9eca 100644 --- a/plugins/dotnet-test/README.md +++ b/plugins/dotnet-test/README.md @@ -40,8 +40,7 @@ Skills and agents for running, generating, analyzing, migrating, and improving . | Skill | Description | |---|---| -| **test-anti-patterns** | Quick pragmatic scan for ~15 common test quality issues with severity ranking | -| **test-smell-detection** | Deep formal audit using academic test smell taxonomy (19 smell types) | +| **test-anti-patterns** | Comprehensive audit of existing tests for anti-patterns and smells (severity-ranked findings with concrete fixes; includes the testsmells.org-based formal smell catalog as an internal reference) | | **assertion-quality** | Measure assertion variety and depth — find shallow tests that barely verify anything | | **test-gap-analysis** | Pseudo-mutation analysis to find test blind spots that coverage numbers miss | | **test-tagging** | Tag tests with standardized traits (smoke, regression, boundary, critical-path, etc.) | diff --git a/plugins/dotnet-test/agents/test-quality-auditor.agent.md b/plugins/dotnet-test/agents/test-quality-auditor.agent.md index 9d5065b382..0111d774f4 100644 --- a/plugins/dotnet-test/agents/test-quality-auditor.agent.md +++ b/plugins/dotnet-test/agents/test-quality-auditor.agent.md @@ -60,8 +60,7 @@ Classify the user's request and route to the appropriate skill: | User Intent | Route To | Plugin | |---|---|---| | "Are my assertions good enough?" / shallow testing / assertion diversity | `assertion-quality` skill | dotnet-test | -| "Find test smells" / comprehensive formal audit | `test-smell-detection` skill | dotnet-test | -| "Pragmatic anti-pattern check" within a broader audit context | `test-anti-patterns` skill | dotnet-test | +| "Find test smells" / "audit my tests" / comprehensive test audit | `test-anti-patterns` skill (includes the testsmells.org-based formal smell catalog) | dotnet-test | | "Find test duplication" / boilerplate / DRY up tests | `exp-test-maintainability` skill | dotnet-experimental | | "Are my mocks needed?" / over-mocking / mock audit | `exp-mock-usage-analysis` skill | dotnet-experimental | | "Would my tests catch bugs?" / mutation analysis / test gaps | `test-gap-analysis` skill | dotnet-test | @@ -78,8 +77,8 @@ When the user asks for a broad quality assessment (e.g., "audit my test suite", Run these in order. Each step builds context for the next. Stop early if the user's scope is narrow or the codebase is small. -1. **Anti-patterns** — `test-anti-patterns` skill - - Quick pragmatic scan for the most impactful issues +1. **Anti-patterns and smells** — `test-anti-patterns` skill + - Comprehensive scan for the most impactful issues (anti-patterns + smell catalog) - Produces severity-ranked findings (Critical → Low) 2. **Assertion quality** — `assertion-quality` skill @@ -96,10 +95,9 @@ Run these in order. Each step builds context for the next. Stop early if the use ### Optional follow-ups (offer but don't run automatically) -5. **Test smells** — `test-smell-detection` skill (if step 1 found many issues and the user wants a deeper formal audit) -6. **Maintainability** — `exp-test-maintainability` skill (if the test suite is large and duplication is suspected) -7. **Mock audit** — `exp-mock-usage-analysis` skill (if over-mocking was flagged in step 1) -8. **Test tagging** — `test-tagging` skill (if the user wants to understand test type distribution) +5. **Maintainability** — `exp-test-maintainability` skill (if the test suite is large and duplication is suspected) +6. **Mock audit** — `exp-mock-usage-analysis` skill (if over-mocking was flagged in step 1) +7. **Test tagging** — `test-tagging` skill (if the user wants to understand test type distribution) ### Synthesizing results diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index 762da91553..558159cbbb 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -1,18 +1,22 @@ --- name: test-anti-patterns description: > - Detection-focused review of .NET test code for anti-patterns that - undermine reliability and diagnostic value. - USE FOR: audit test quality, review test code, find test anti-patterns, - tests pass but don't verify anything, flaky tests, ordering dependency, - duplicate tests, magic values, missing/no assertions, swallowed - exceptions, always-true assertions, over-mocking, test coupling, coverage - touching, coverage inflation. - DO NOT USE FOR: writing new tests (use writing-mstest-tests), direct - MSTest API rewrites or implementation-only fixes such as swapped - Assert.AreEqual argument order, running tests (use run-tests), migrating - between frameworks (use migration skills), deep formal audit based on - academic test smell taxonomy (use test-smell-detection). + Audits existing .NET test code (MSTest, xUnit, NUnit, TUnit) for + anti-patterns and smells that undermine reliability and diagnostic + value — produces a severity-ranked report (Critical / Warning / Info) + with concrete code-level fixes and acknowledgement of what the tests + do well. Includes the testsmells.org 19-smell academic catalog as a + reference. + INVOKE THIS SKILL when the user asks to audit, review, rank, or find + problems in existing tests — including: "audit my tests", "audit for + .NET test anti-patterns", "test smell audit", "formal smell audit", + "rank by severity", "are these tests good", no/missing assertions, + swallowed exceptions, always-true / self-comparing / self-referential + assertions, broad exception types, flakiness (Thread.Sleep, + DateTime.Now), ordering dependency, shared static state, reflection + coupling, duplicated tests, magic values, coverage touching. + DO NOT USE FOR: writing new tests (use writing-mstest-tests); running + tests (use run-tests); framework migration (use migration skills). license: MIT --- @@ -37,7 +41,7 @@ Quick, pragmatic analysis of .NET test code for anti-patterns and quality issues - 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) -- User wants a deep formal test smell audit with academic taxonomy and extended catalog (use `test-smell-detection`) +- User wants raw academic-only smell catalog research data without an audit context (this skill includes a smell catalog reference, but is audit-focused — not a literature export) ## Inputs @@ -153,3 +157,7 @@ If there are many findings, recommend which to fix first: | 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 | + +## References + +For deeper smell audits using the academic testsmells.org research taxonomy (Assertion Roulette, Duplicate Assert, Constructor Initialization, Default Test, Lazy Test, Redundant Print, Resource Optimism, Empty Test, and others not covered in the catalogs above), see [`references/test-smell-catalog.md`](references/test-smell-catalog.md). Use it when the user explicitly asks for the formal/research-backed 19-smell catalog or when the standard catalogs above leave a gap. diff --git a/plugins/dotnet-test/skills/test-smell-detection/references/test-smell-catalog.md b/plugins/dotnet-test/skills/test-anti-patterns/references/test-smell-catalog.md similarity index 100% rename from plugins/dotnet-test/skills/test-smell-detection/references/test-smell-catalog.md rename to plugins/dotnet-test/skills/test-anti-patterns/references/test-smell-catalog.md diff --git a/plugins/dotnet-test/skills/test-smell-detection/SKILL.md b/plugins/dotnet-test/skills/test-smell-detection/SKILL.md deleted file mode 100644 index 148f85f6ea..0000000000 --- a/plugins/dotnet-test/skills/test-smell-detection/SKILL.md +++ /dev/null @@ -1,198 +0,0 @@ ---- -name: test-smell-detection -description: "Deep formal test smell audit based on academic research taxonomy (testsmells.org). Detects 19 categorized smell types — conditional logic, mystery guests, sensitive equality, eager tests, and more — with calibrated severity and research-backed remediation. Use for comprehensive test suite health assessments. For a quick pragmatic review, use test-anti-patterns instead. DO NOT USE FOR: writing new tests (use writing-mstest-tests), evaluating assertion quality specifically (use assertion-quality), or finding test duplication and boilerplate (use exp-test-maintainability)." -license: MIT ---- - -# Test Smell Detection - -Deep formal audit of test code using an academic test smell taxonomy. Detects symptoms of bad design or implementation decisions that make tests harder to understand, more fragile, less effective at catching bugs, or more expensive to maintain. Produces a severity-ranked report with specific locations and actionable fixes. - -## Why Test Smells Matter - -Test smells erode confidence in a test suite and inflate maintenance costs: - -| Problem | Consequence | -|---------|-------------| -| Tests with conditional logic | Some paths never execute — hidden testing gaps | -| Tests that depend on external resources | Flaky failures, slow execution, environment coupling | -| Tests that sleep to wait for results | Non-deterministic timing, slow suites, false failures | -| Tests without assertions | False confidence — coverage looks good but nothing is verified | -| Tests that call many production methods | Hard to diagnose failures, unclear what's being tested | -| Tests with magic numbers | Unreadable intent, unclear boundary conditions | -| Tests relying on ToString for comparison | Brittle to formatting changes, obscure failure messages | -| Tests with exception handling logic | Swallowed failures, tests that pass when they shouldn't | - -## When to Use - -- User asks for a comprehensive or formal test smell audit -- User asks "are my tests well-written?" and wants a thorough analysis -- User wants a test quality health check with academic rigor -- User asks for a review of test design or structure using standard smell categories -- User suspects tests are fragile, flaky, or giving false confidence and wants a deep investigation - -## When Not to Use - -- User wants a quick pragmatic test review (use `test-anti-patterns` — faster, covers the most common issues) -- User wants to evaluate assertion diversity specifically (use `assertion-quality`) -- User wants to find duplicated boilerplate across tests (use `exp-test-maintainability`) -- User wants to write new tests from scratch (help them directly) -- User wants to fix a specific failing test (diagnose and fix directly) - -## Inputs - -| Input | Required | Description | -|-------|----------|-------------| -| Test code | Yes | One or more test files or a test project directory to analyze | -| Production code | No | The code under test, for context on whether patterns are justified | - -## Workflow - -### Step 1: Gather the test code - -Read all test files the user provides. If the user points to a directory or project, scan for all test files by looking for test framework markers — see the `dotnet-test-frameworks` skill for .NET-specific markers. - -For a thorough audit, also consult the [extended smell catalog](references/test-smell-catalog.md) which covers 9 additional smell types beyond the core 10 below. - -### Step 2: Scan for test smells - -For each test method and class, check for the following smell categories: - -#### Smell 1: Conditional Test Logic - -Test methods containing `if`, `else`, `switch`, ternary (`? :`), `for`, `foreach`, or `while` statements. Control flow in tests means some paths may never execute, hiding gaps. - -**Severity:** High -**Detection:** Any control flow statement inside a test method body. -**Exception:** `foreach` used solely to assert every item in a known collection is acceptable when the assertion is the loop body. - -#### Smell 2: Mystery Guest - -Tests that depend on external resources — files on disk, databases, network endpoints, environment variables — without making the dependency explicit or using test doubles. - -**Severity:** High -**Detection:** Test methods that read files, open database connections, make HTTP requests (without a test handler), read environment variables, or use hard-coded file paths. -**Exception:** In-memory fakes or test-specific handlers are fine. - -#### Smell 3: Sleepy Test - -Tests that call sleep or delay functions to wait for a condition. These introduce non-deterministic timing and slow down the suite. - -**Severity:** High -**Detection:** Calls to sleep/delay functions inside test methods. See the `dotnet-test-frameworks` skill for .NET-specific patterns. - -#### Smell 4: Assertion-Free Test (Unknown Test) - -Tests that execute code but never assert anything. Test frameworks report these as passing even if the code is completely broken, as long as no exception is thrown. - -**Severity:** High -**Detection:** A test method with no assertion calls (framework-specific: `Assert.*`, `expect()`, `assert`, `Should*`, etc.) and no expected-exception annotation. -**Calibration:** A method named `*_DoesNotThrow` or `*_NoException` is implicitly asserting no exception — still flag it but note it may be intentional. - -#### Smell 5: Eager Test - -A test method that calls many different production methods, making it unclear what behavior is being tested. When it fails, diagnosis is difficult because the failure could stem from any of the calls. - -**Severity:** Medium -**Detection:** A test method that calls 4+ distinct methods on the production object (excluding setup/construction). Count unique method names, not call count. -**Calibration:** Integration tests or workflow tests may legitimately call multiple methods — note this as a possible exception for end-to-end scenarios. - -#### Smell 6: Magic Number Test - -Assertions that contain unexplained numeric literals. The intent of `Assert.AreEqual(42, result)` is unclear without context — what does 42 represent? - -**Severity:** Medium -**Detection:** Numeric literals (other than 0, 1, -1, and the literal used in the test name) appearing as `expected` parameters in assertion methods. -**Calibration:** Small integers in context (like count checks `Assert.AreEqual(3, list.Count)` where 3 items were just added) are acceptable — only flag when the number's meaning is genuinely unclear. - -#### Smell 7: Sensitive Equality - -Tests that use `ToString()` for comparison or assertion. If the `ToString()` implementation changes, the test breaks even though the actual behavior is correct. - -**Severity:** Medium -**Detection:** `Assert.AreEqual(expected, obj.ToString())`, or `.ToString()` appearing inside an assertion parameter. - -#### Smell 8: Exception Handling in Tests - -Tests that contain `try`/`catch` blocks or `throw` statements. This typically means the test is manually managing exceptions rather than using the framework's built-in exception assertion facilities. - -**Severity:** Medium -**Detection:** `try`/`catch` or `throw`/`raise` statements inside a test method. -**Exception:** `catch` blocks that capture an exception for further assertion are a lesser concern — note but don't flag as high severity. - -#### Smell 9: General Fixture (Over-broad Setup) - -The test setup method or constructor initializes fields that are not used by every test method. This means each test pays the cost of setting up objects it doesn't need. - -**Severity:** Low -**Detection:** Fields initialized in setup that are referenced by fewer than half the test methods in the class. - -#### Smell 10: Ignored/Disabled Test - -Tests marked as skipped or disabled. These add overhead and clutter, and the underlying issue they were disabled for may never be addressed. - -**Severity:** Low -**Detection:** Skip/ignore annotations or conditional compilation that disables a test. See the `dotnet-test-frameworks` skill for framework-specific skip attributes. - -### Step 3: Apply calibration rules - -Before reporting, calibrate findings to avoid false positives: - -- **Integration tests have different norms.** A test class clearly marked as integration (by name, annotation, or category) legitimately uses external resources, calls multiple methods, and may use delays for async coordination. Downgrade Mystery Guest, Eager Test, and Sleepy Test severity for integration tests — note them but don't flag as problems. -- **Simple loop-assert patterns are fine.** Iterating a collection to assert on every item is readable and correct. Only flag loops with complex branching logic. -- **Context matters for magic numbers.** A count assertion right after adding a known number of items is self-documenting. Only flag numbers whose meaning requires looking at production code to understand. -- **Inconclusive/pending markers are not assertion-free.** Tests explicitly marked as incomplete should be flagged as Ignored Test, not Assertion-Free. -- **Capture-and-assert exception patterns are borderline.** Try/catch patterns that capture an exception then assert on its properties are ugly but functional. Note as a smell and suggest the framework's built-in exception assertion instead of calling it broken. -- **If the test suite is clean, say so.** A report finding few or no smells is perfectly valid. - -### Step 4: Report findings - -Present the analysis in this structure: - -1. **Summary Dashboard** — Quick overview: - ``` - | Severity | Smell Count | Affected Tests | - |----------|-------------|----------------| - | High | 3 | 7 | - | Medium | 2 | 4 | - | Low | 1 | 2 | - | Total | 6 | 13 | - ``` - -2. **Findings by Severity** — For each smell found: - - Smell name and category - - Severity level with rationale - - Affected test methods (file and method name) - - Code snippet showing the smell - - Concrete fix: show what the code should look like after remediation - - Risk if left unfixed - -3. **Smell-Free Patterns** — If any test methods are well-written, briefly acknowledge this. Highlighting what's good helps the user understand the contrast. - -4. **Prioritized Remediation Plan** — Rank fixes by: - - Impact (high-severity smells affecting many tests first) - - Effort (quick fixes before refactoring) - - Risk (fixes that prevent false-passes before cosmetic improvements) - -## Validation - -- [ ] Every finding includes the specific test method name and file location -- [ ] Every finding includes a code snippet showing the smell in context -- [ ] Every finding includes a concrete fix example (not just "fix this") -- [ ] Integration tests are not penalized for patterns that are appropriate for their scope -- [ ] Simple foreach-assert loops are not flagged as conditional test logic -- [ ] Contextually obvious numbers are not flagged as magic numbers -- [ ] If the test suite is clean, the report says so upfront -- [ ] Severity levels are justified, not arbitrary - -## Common Pitfalls - -| Pitfall | Solution | -|---------|----------| -| Flagging integration tests for using real resources | Check for integration test markers and adjust severity accordingly | -| Flagging loop-over-collection-assert as conditional logic | Only flag loops with branching or complex logic, not assertion iterations | -| Flagging obvious count assertions after adding N items | Consider the immediate context — self-documenting numbers are fine | -| Missing framework-specific assertion syntax | Consult the `dotnet-test-frameworks` skill for .NET framework assertion and skip APIs | -| Over-flagging try/catch that captures for assertion | Distinguish swallowed exceptions from capture-and-assert patterns | -| Treating skip annotations with reasons same as bare skips | Note that reasoned skips are less concerning than unexplained ones | -| Flagging `DoesNotThrow`-style tests as assertion-free | These implicitly assert no exception — note but acknowledge the intent | diff --git a/tests/dotnet-test/test-anti-patterns/eval.vally.yaml b/tests/dotnet-test/test-anti-patterns/eval.vally.yaml index 276657c36a..56c93e3e91 100644 --- a/tests/dotnet-test/test-anti-patterns/eval.vally.yaml +++ b/tests/dotnet-test/test-anti-patterns/eval.vally.yaml @@ -283,3 +283,116 @@ stimuli: - bash - edit - create + + - name: Detect multiple test smells in order processing test suite + prompt: | + I'm worried about the quality of our OrderProcessor test suite. + The tests all pass but some of them feel off — can you do a test + smell audit and rank any findings by severity? + environment: + files: + - src: fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj + dest: OrderService.Tests/OrderService.Tests.csproj + - src: fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs + dest: OrderService.Tests/OrderProcessorTests.cs + graders: + - type: output-matches + config: + pattern: (conditional|if.else|control flow|branch) + - type: output-matches + config: + pattern: (no assert|assertion.free|without.*assert|zero assert|missing.*assert) + - type: output-matches + config: + pattern: (Thread\.Sleep|sleep|delay|sleepy) + - type: exit-success + - type: prompt + - type: pairwise + rubric: + - Identified the conditional test logic in ProcessOrder_SetsCorrectStatus — the if/else means one branch is always + untested + - Flagged ProcessOrder_CompletesWithoutError as having no assertions — it only calls the method without verifying + anything + - Identified the Thread.Sleep(2000) in ProcessOrder_AsyncNotification_IsSent as a flakiness risk + - Noted the try/catch pattern in ProcessOrder_EmptyOrder_ThrowsValidationError and suggested using + Assert.ThrowsException instead + - Identified the ToString() comparison in GetOrderSummary_ReturnsFormattedString as fragile + - Noted the File.ReadAllText with a hard-coded path in ImportOrders_FromCsv_ParsesCorrectly as an external + dependency problem + - Provided concrete fix suggestions showing how to rewrite at least some of the smelly tests + - name: Recognize well-written tests with no significant smells + prompt: | + Can you do a formal test smell audit of my Calculator tests? I want + to make sure they're clean before we use them as a template for + other test suites. + environment: + files: + - src: fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj + dest: Calculator.Tests/Calculator.Tests.csproj + - src: fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs + dest: Calculator.Tests/ScientificCalculatorTests.cs + graders: + - type: output-matches + config: + pattern: (clean|good|well.written|no.*(major|significant).*(smell|issue|problem)|solid|well.structured) + - type: exit-success + - type: prompt + - type: pairwise + rubric: + - Recognized that the test suite is well-structured with clear Arrange-Act-Assert patterns + - Acknowledged proper use of Assert.ThrowsException for exception testing instead of try/catch + - Noted the use of parameterized tests with [DataTestMethod] as a good practice + - Did not invent false problems or flag correct patterns as smells + - If any minor suggestions were made, they were presented as optional improvements rather than problems + - name: Recognize integration tests and avoid false positives for external resources + prompt: | + Our team has been arguing about whether our integration tests are + well-designed. Some people say they have too many test smells. + Can you do a formal smell audit and give us an objective + severity-ranked assessment? + environment: + files: + - src: fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj + dest: DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj + - src: fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs + dest: DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs + graders: + - type: output-matches + config: + pattern: (integration|appropriate|expected|legitimate|acceptable) + - type: output-matches + config: + pattern: (Sleep|delay|sleepy|conditional|if.else|assert|no assert) + - type: exit-success + - type: prompt + - type: pairwise + rubric: + - Recognized that these are integration tests and did not flag database usage or insert-then-query patterns as + smells + - Identified the Thread.Sleep(3000) in NotifyOnInsert_SendsEventAfterDelay as a real smell even in an integration + test context + - Flagged the if/else conditional logic in GetUser_ReturnsCorrectType as a test smell — one branch is untested + - Identified BulkInsert_RunsWithoutErrors as having no assertions — it exercises code but verifies nothing + - Distinguished between the legitimate integration patterns (setup/teardown, multi-step persistence) and the + actual smells (sleep, conditional, assertion-free) + - name: Decline request to write new tests from scratch + prompt: | + I need to write unit tests for my ShoppingCart class. It supports + adding items, removing items, calculating totals, and applying + coupon codes. Can you write a complete MSTest test suite for me? + environment: + files: + - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs + dest: ShoppingCart.cs + graders: + - type: output-matches + config: + pattern: (TestMethod|TestClass|\[Fact\]|test) + - type: prompt + - type: pairwise + rubric: + - Wrote test methods for the ShoppingCart class + - Covered multiple methods of the class + constraints: + reject_skills: + - "*" \ No newline at end of file diff --git a/tests/dotnet-test/test-anti-patterns/eval.yaml b/tests/dotnet-test/test-anti-patterns/eval.yaml index aacbba27fd..56b588e6a8 100644 --- a/tests/dotnet-test/test-anti-patterns/eval.yaml +++ b/tests/dotnet-test/test-anti-patterns/eval.yaml @@ -215,3 +215,103 @@ scenarios: - "Noted that Clone_ProducesEqualObject is a legitimate equality check (testing deep copy), not self-referential" reject_tools: ["bash", "edit", "create"] timeout: 180 + + # ========================================================================== + # Smell-audit scenarios (merged from former test-smell-detection skill) + # ========================================================================== + + - name: "Detect multiple test smells in order processing test suite" + prompt: | + I'm worried about the quality of our OrderProcessor test suite. + The tests all pass but some of them feel off — can you do a test + smell audit and rank any findings by severity? + setup: + files: + - path: "OrderService.Tests/OrderService.Tests.csproj" + source: "fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj" + - path: "OrderService.Tests/OrderProcessorTests.cs" + source: "fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs" + assertions: + - type: "output_matches" + pattern: "(conditional|if.else|control flow|branch)" + - type: "output_matches" + pattern: "(no assert|assertion.free|without.*assert|zero assert|missing.*assert)" + - type: "output_matches" + pattern: "(Thread\\.Sleep|sleep|delay|sleepy)" + - type: "exit_success" + rubric: + - "Identified the conditional test logic in ProcessOrder_SetsCorrectStatus — the if/else means one branch is always untested" + - "Flagged ProcessOrder_CompletesWithoutError as having no assertions — it only calls the method without verifying anything" + - "Identified the Thread.Sleep(2000) in ProcessOrder_AsyncNotification_IsSent as a flakiness risk" + - "Noted the try/catch pattern in ProcessOrder_EmptyOrder_ThrowsValidationError and suggested using Assert.ThrowsException instead" + - "Identified the ToString() comparison in GetOrderSummary_ReturnsFormattedString as fragile" + - "Noted the File.ReadAllText with a hard-coded path in ImportOrders_FromCsv_ParsesCorrectly as an external dependency problem" + - "Provided concrete fix suggestions showing how to rewrite at least some of the smelly tests" + timeout: 180 + + - name: "Recognize well-written tests with no significant smells" + prompt: | + Can you do a formal test smell audit of my Calculator tests? I want + to make sure they're clean before we use them as a template for + other test suites. + setup: + files: + - path: "Calculator.Tests/Calculator.Tests.csproj" + source: "fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj" + - path: "Calculator.Tests/ScientificCalculatorTests.cs" + source: "fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs" + assertions: + - type: "output_matches" + pattern: "(clean|good|well.written|no.*(major|significant).*(smell|issue|problem)|solid|well.structured)" + - type: "exit_success" + rubric: + - "Recognized that the test suite is well-structured with clear Arrange-Act-Assert patterns" + - "Acknowledged proper use of Assert.ThrowsException for exception testing instead of try/catch" + - "Noted the use of parameterized tests with [DataTestMethod] as a good practice" + - "Did not invent false problems or flag correct patterns as smells" + - "If any minor suggestions were made, they were presented as optional improvements rather than problems" + timeout: 180 + + - name: "Recognize integration tests and avoid false positives for external resources" + prompt: | + Our team has been arguing about whether our integration tests are + well-designed. Some people say they have too many test smells. + Can you do a formal smell audit and give us an objective + severity-ranked assessment? + setup: + files: + - path: "DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj" + source: "fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj" + - path: "DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs" + source: "fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs" + assertions: + - type: "output_matches" + pattern: "(integration|appropriate|expected|legitimate|acceptable)" + - type: "output_matches" + pattern: "(Sleep|delay|sleepy|conditional|if.else|assert|no assert)" + - type: "exit_success" + rubric: + - "Recognized that these are integration tests and did not flag database usage or insert-then-query patterns as smells" + - "Identified the Thread.Sleep(3000) in NotifyOnInsert_SendsEventAfterDelay as a real smell even in an integration test context" + - "Flagged the if/else conditional logic in GetUser_ReturnsCorrectType as a test smell — one branch is untested" + - "Identified BulkInsert_RunsWithoutErrors as having no assertions — it exercises code but verifies nothing" + - "Distinguished between the legitimate integration patterns (setup/teardown, multi-step persistence) and the actual smells (sleep, conditional, assertion-free)" + timeout: 180 + + - name: "Decline request to write new tests from scratch" + prompt: | + I need to write unit tests for my ShoppingCart class. It supports + adding items, removing items, calculating totals, and applying + coupon codes. Can you write a complete MSTest test suite for me? + expect_activation: false + setup: + files: + - path: "ShoppingCart.cs" + source: "fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs" + assertions: + - type: "output_matches" + pattern: "(TestMethod|TestClass|\\[Fact\\]|test)" + rubric: + - "Wrote test methods for the ShoppingCart class" + - "Covered multiple methods of the class" + timeout: 300 diff --git a/tests/dotnet-test/test-smell-detection/fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj b/tests/dotnet-test/test-anti-patterns/fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj rename to tests/dotnet-test/test-anti-patterns/fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj diff --git a/tests/dotnet-test/test-smell-detection/fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs b/tests/dotnet-test/test-anti-patterns/fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs rename to tests/dotnet-test/test-anti-patterns/fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs diff --git a/tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs b/tests/dotnet-test/test-anti-patterns/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs rename to tests/dotnet-test/test-anti-patterns/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs diff --git a/tests/dotnet-test/test-smell-detection/fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj b/tests/dotnet-test/test-anti-patterns/fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj rename to tests/dotnet-test/test-anti-patterns/fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj diff --git a/tests/dotnet-test/test-smell-detection/fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs b/tests/dotnet-test/test-anti-patterns/fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs rename to tests/dotnet-test/test-anti-patterns/fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs diff --git a/tests/dotnet-test/test-smell-detection/fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs b/tests/dotnet-test/test-anti-patterns/fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs rename to tests/dotnet-test/test-anti-patterns/fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs diff --git a/tests/dotnet-test/test-smell-detection/fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj b/tests/dotnet-test/test-anti-patterns/fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj similarity index 100% rename from tests/dotnet-test/test-smell-detection/fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj rename to tests/dotnet-test/test-anti-patterns/fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj diff --git a/tests/dotnet-test/test-smell-detection/eval.vally.yaml b/tests/dotnet-test/test-smell-detection/eval.vally.yaml deleted file mode 100644 index dfe31dc442..0000000000 --- a/tests/dotnet-test/test-smell-detection/eval.vally.yaml +++ /dev/null @@ -1,117 +0,0 @@ -name: test-smell-detection -description: Evaluates the dotnet-test/test-smell-detection skill -type: capability -config: - timeout: 5m -stimuli: - - name: Detect multiple test smells in order processing test suite - prompt: | - I'm worried about the quality of our OrderProcessor test suite. - The tests all pass but some of them feel off — can you review them - and tell me if there are any problematic patterns? - environment: - files: - - src: fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj - dest: OrderService.Tests/OrderService.Tests.csproj - - src: fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs - dest: OrderService.Tests/OrderProcessorTests.cs - graders: - - type: output-matches - config: - pattern: (conditional|if.else|control flow|branch) - - type: output-matches - config: - pattern: (no assert|assertion.free|without.*assert|zero assert|missing.*assert) - - type: output-matches - config: - pattern: (Thread\.Sleep|sleep|delay|sleepy) - - type: exit-success - - type: prompt - - type: pairwise - rubric: - - Identified the conditional test logic in ProcessOrder_SetsCorrectStatus — the if/else means one branch is always - untested - - Flagged ProcessOrder_CompletesWithoutError as having no assertions — it only calls the method without verifying - anything - - Identified the Thread.Sleep(2000) in ProcessOrder_AsyncNotification_IsSent as a flakiness risk - - Noted the try/catch pattern in ProcessOrder_EmptyOrder_ThrowsValidationError and suggested using - Assert.ThrowsException instead - - Identified the ToString() comparison in GetOrderSummary_ReturnsFormattedString as fragile - - Noted the File.ReadAllText with a hard-coded path in ImportOrders_FromCsv_ParsesCorrectly as an external - dependency problem - - Provided concrete fix suggestions showing how to rewrite at least some of the smelly tests - - name: Recognize well-written tests with no significant smells - prompt: | - Can you check my Calculator tests for any test design problems or - anti-patterns? I want to make sure they follow best practices before - we use them as a template for other test suites. - environment: - files: - - src: fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj - dest: Calculator.Tests/Calculator.Tests.csproj - - src: fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs - dest: Calculator.Tests/ScientificCalculatorTests.cs - graders: - - type: output-matches - config: - pattern: (clean|good|well.written|no.*(major|significant).*(smell|issue|problem)|solid|well.structured) - - type: exit-success - - type: prompt - - type: pairwise - rubric: - - Recognized that the test suite is well-structured with clear Arrange-Act-Assert patterns - - Acknowledged proper use of Assert.ThrowsException for exception testing instead of try/catch - - Noted the use of parameterized tests with [DataTestMethod] as a good practice - - Did not invent false problems or flag correct patterns as smells - - If any minor suggestions were made, they were presented as optional improvements rather than problems - - name: Recognize integration tests and avoid false positives for external resources - prompt: | - Our team has been arguing about whether our integration tests are - well-designed. Some people say they have too many smells. Can you - review them and give us an objective assessment? - environment: - files: - - src: fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj - dest: DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj - - src: fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs - dest: DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs - graders: - - type: output-matches - config: - pattern: (integration|appropriate|expected|legitimate|acceptable) - - type: output-matches - config: - pattern: (Sleep|delay|sleepy|conditional|if.else|assert|no assert) - - type: exit-success - - type: prompt - - type: pairwise - rubric: - - Recognized that these are integration tests and did not flag database usage or insert-then-query patterns as - smells - - Identified the Thread.Sleep(3000) in NotifyOnInsert_SendsEventAfterDelay as a real smell even in an integration - test context - - Flagged the if/else conditional logic in GetUser_ReturnsCorrectType as a test smell — one branch is untested - - Identified BulkInsert_RunsWithoutErrors as having no assertions — it exercises code but verifies nothing - - Distinguished between the legitimate integration patterns (setup/teardown, multi-step persistence) and the - actual smells (sleep, conditional, assertion-free) - - name: Decline request to write new tests from scratch - prompt: | - I need to write unit tests for my ShoppingCart class. It supports - adding items, removing items, calculating totals, and applying - coupon codes. Can you write a complete MSTest test suite for me? - environment: - files: - - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs - dest: ShoppingCart.cs - graders: - - type: output-matches - config: - pattern: (TestMethod|TestClass|\[Fact\]|test) - - type: prompt - - type: pairwise - rubric: - - Wrote test methods for the ShoppingCart class - - Covered multiple methods of the class - constraints: - reject_skills: - - "*" diff --git a/tests/dotnet-test/test-smell-detection/eval.yaml b/tests/dotnet-test/test-smell-detection/eval.yaml deleted file mode 100644 index dc765010b9..0000000000 --- a/tests/dotnet-test/test-smell-detection/eval.yaml +++ /dev/null @@ -1,120 +0,0 @@ -scenarios: - # ========================================================================== - # Scenario 1: Multiple test smells in a single test suite - # ========================================================================== - - - name: "Detect multiple test smells in order processing test suite" - prompt: | - I'm worried about the quality of our OrderProcessor test suite. - The tests all pass but some of them feel off — can you review them - and tell me if there are any problematic patterns? - setup: - files: - - path: "OrderService.Tests/OrderService.Tests.csproj" - source: "fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj" - - path: "OrderService.Tests/OrderProcessorTests.cs" - source: "fixtures/smelly-tests/OrderService.Tests/OrderProcessorTests.cs" - assertions: - - type: "output_matches" - pattern: "(conditional|if.else|control flow|branch)" - - type: "output_matches" - pattern: "(no assert|assertion.free|without.*assert|zero assert|missing.*assert)" - - type: "output_matches" - pattern: "(Thread\\.Sleep|sleep|delay|sleepy)" - - type: "exit_success" - rubric: - - "Identified the conditional test logic in ProcessOrder_SetsCorrectStatus — the if/else means one branch is always untested" - - "Flagged ProcessOrder_CompletesWithoutError as having no assertions — it only calls the method without verifying anything" - - "Identified the Thread.Sleep(2000) in ProcessOrder_AsyncNotification_IsSent as a flakiness risk" - - "Noted the try/catch pattern in ProcessOrder_EmptyOrder_ThrowsValidationError and suggested using Assert.ThrowsException instead" - - "Identified the ToString() comparison in GetOrderSummary_ReturnsFormattedString as fragile" - - "Noted the File.ReadAllText with a hard-coded path in ImportOrders_FromCsv_ParsesCorrectly as an external dependency problem" - - "Provided concrete fix suggestions showing how to rewrite at least some of the smelly tests" - timeout: 120 - - # ========================================================================== - # Scenario 2: Clean test suite — acknowledge good quality - # ========================================================================== - - - name: "Recognize well-written tests with no significant smells" - prompt: | - Can you check my Calculator tests for any test design problems or - anti-patterns? I want to make sure they follow best practices before - we use them as a template for other test suites. - setup: - files: - - path: "Calculator.Tests/Calculator.Tests.csproj" - source: "fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj" - - path: "Calculator.Tests/ScientificCalculatorTests.cs" - source: "fixtures/clean-tests/Calculator.Tests/ScientificCalculatorTests.cs" - assertions: - - type: "output_matches" - pattern: "(clean|good|well.written|no.*(major|significant).*(smell|issue|problem)|solid|well.structured)" - - type: "exit_success" - rubric: - - "Recognized that the test suite is well-structured with clear Arrange-Act-Assert patterns" - - "Acknowledged proper use of Assert.ThrowsException for exception testing instead of try/catch" - - "Noted the use of parameterized tests with [DataTestMethod] as a good practice" - - "Did not invent false problems or flag correct patterns as smells" - - "If any minor suggestions were made, they were presented as optional improvements rather than problems" - timeout: 120 - - # ========================================================================== - # Scenario 3: Integration tests — don't over-flag legitimate patterns - # ========================================================================== - - - name: "Recognize integration tests and avoid false positives for external resources" - prompt: | - Our team has been arguing about whether our integration tests are - well-designed. Some people say they have too many smells. Can you - review them and give us an objective assessment? - setup: - files: - - path: "DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj" - source: "fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj" - - path: "DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs" - source: "fixtures/integration-tests/DataAccess.IntegrationTests/UserRepositoryIntegrationTests.cs" - assertions: - - type: "output_matches" - pattern: "(integration|appropriate|expected|legitimate|acceptable)" - - type: "output_matches" - pattern: "(Sleep|delay|sleepy|conditional|if.else|assert|no assert)" - - type: "exit_success" - rubric: - - "Recognized that these are integration tests and did not flag database usage or insert-then-query patterns as smells" - - "Identified the Thread.Sleep(3000) in NotifyOnInsert_SendsEventAfterDelay as a real smell even in an integration test context" - - "Flagged the if/else conditional logic in GetUser_ReturnsCorrectType as a test smell — one branch is untested" - - "Identified BulkInsert_RunsWithoutErrors as having no assertions — it exercises code but verifies nothing" - - "Distinguished between the legitimate integration patterns (setup/teardown, multi-step persistence) and the actual smells (sleep, conditional, assertion-free)" - timeout: 120 - - # ========================================================================== - # Scenario 4: Non-activation — user wants to write new tests - # ========================================================================== - - - name: "Decline request to write new tests from scratch" - prompt: | - I need to write unit tests for my ShoppingCart class. It supports - adding items, removing items, calculating totals, and applying - coupon codes. Can you write a complete MSTest test suite for me? - expect_activation: false - setup: - files: - - path: "ShoppingCart.cs" - content: | - namespace Store; - - public sealed class ShoppingCart - { - public void AddItem(string sku, int quantity, decimal price) => throw new NotImplementedException(); - public void RemoveItem(string sku) => throw new NotImplementedException(); - public decimal CalculateTotal() => throw new NotImplementedException(); - public void ApplyCoupon(string code) => throw new NotImplementedException(); - } - assertions: - - type: "output_matches" - pattern: "(TestMethod|TestClass|\\[Fact\\]|test)" - rubric: - - "Wrote test methods for the ShoppingCart class" - - "Covered multiple methods of the class" - timeout: 300