diff --git a/.gitignore b/.gitignore index 9fdf6b977d..9481074e8e 100644 --- a/.gitignore +++ b/.gitignore @@ -168,6 +168,12 @@ coverage*.json coverage*.xml coverage*.info +# ...but coverage files committed as eval fixtures are inputs, not build output. +# Without this negation `git add` silently skips them and the eval fails at setup. +!tests/**/fixtures/**/coverage*.xml +!tests/**/fixtures/**/coverage*.json +!tests/**/fixtures/**/coverage*.info + # Visual Studio code coverage results *.coverage *.coveragexml diff --git a/plugins/dotnet-test/skills/code-testing-agent/SKILL.md b/plugins/dotnet-test/skills/code-testing-agent/SKILL.md index 5fa883a9b2..87d64989c6 100644 --- a/plugins/dotnet-test/skills/code-testing-agent/SKILL.md +++ b/plugins/dotnet-test/skills/code-testing-agent/SKILL.md @@ -3,12 +3,14 @@ name: code-testing-agent description: >- MANDATORY ENTRY POINT for generating or writing tests. Invoke this skill before editing files whenever the user asks to generate tests, write/add unit - tests, scaffold a test project or pytest/Vitest/Jest suite, create - comprehensive tests, improve/achieve coverage, or test an app, API, service, - repository, route, module, library, or package. Supports C#/.NET, Python, - TypeScript/JavaScript, Go, Rust, Java, and Ruby. For sparse, gutted-looking, - synthetic, or incomplete workspaces, test only the source that remains and + tests, scaffold a test project or suite, create comprehensive tests, + improve/achieve coverage, extend an existing suite to cover an untested + method, or test an app, API, service, repository, route, module, library, or + package. Invoke it even when the request says to use the repository's + "standard test-generation workflow", and when the workspace looks sparse, + gutted or partially deleted — then test only the source that remains and never restore missing source. + Polyglot: C#/.NET, Python, TypeScript/JavaScript, Go, Rust, Java, Ruby. DO NOT USE FOR: running existing tests (use run-tests); analyzing coverage reports (use coverage-analysis or crap-score); MSTest-specific test authoring or modernization (use writing-mstest-tests). @@ -93,7 +95,7 @@ artifacts described below, and apply the same completion contract. For multi-file requests: -1. Turn every explicit user requirement into a checklist before implementation. Include requested layers, collaborators to mock, boundary cases, integrations, coverage thresholds, and report artifacts. +1. Turn every explicit user requirement into a checklist before implementation. Include requested layers, collaborators to mock, boundary cases, integrations, coverage thresholds, and report artifacts. Copy multi-condition requirements verbatim — they must each map to one test that exercises the whole combination. 2. Research only the requested module or project and write the checklist plus a compact target inventory to `.testagent/research.md`. 3. Reuse manifests, symbol references, and deterministic pairing tools instead of reading every source and test file. 4. For multi-file scopes in C#, Python, TypeScript/JavaScript, Go, Java, Rust, or Ruby, run `find-untested-sources` once and consume its pairing and suggested-path output; do not repeat that discovery manually. @@ -129,6 +131,20 @@ Behavioral rows cite exact generated test names. Non-behavioral rows cite the relevant project file, validation command, or coverage report. A generic list of tested areas is not a substitute for requirement-by-requirement evidence. +**Quote the user's requirement verbatim in each row.** When the request names a +specific combination — "a case where a composite discount, regional tax, and +weight-based shipping all apply", "the difference between summed and chained +discounts", "constructor validation for every class" — the row must cite the one +test that demonstrates exactly that. A test that merely exercises the same +collaborators does not satisfy a requirement about their interaction, and +per-class requirements need a citation per class. + +**Cite a clean run, not an attempt.** The commands behind the evidence table must +have finished successfully: quote the final passing test summary and, when +thresholds were requested, the per-module coverage table from a run that exited +0. If the last coverage run exited non-zero, fix it and re-run before reporting; +never infer threshold clearance from a failed or partial run. + ## State Management All pipeline state is stored in `.testagent/` folder: diff --git a/plugins/dotnet-test/skills/coverage-analysis/SKILL.md b/plugins/dotnet-test/skills/coverage-analysis/SKILL.md index a6de1e03ad..58ec55788b 100644 --- a/plugins/dotnet-test/skills/coverage-analysis/SKILL.md +++ b/plugins/dotnet-test/skills/coverage-analysis/SKILL.md @@ -443,11 +443,16 @@ As soon as Phase 3 completes, **your immediately next assistant response must co The response must include, at minimum: +0. **A direct answer to the question that was actually asked, in the first 2–4 sentences.** For "why is my coverage stuck?" / "what's blocking me?", name the blocking members and the lines involved before any table. The standard sections below still follow. 1. Overall line and branch coverage — read directly from the `OVERALL_LINE_COVERAGE:` / `OVERALL_BRANCH_COVERAGE:` lines emitted by `Compute-CrapScores.ps1` (no extra Cobertura parsing required) 2. The Risk Hotspots table built from `Compute-CrapScores.ps1` `HOTSPOTS:` output (CRAP scores, complexity, coverage) 3. Identification of the highest-risk method(s) and what is blocking coverage 4. 1–3 prioritized, specific recommendations (which method to test, expected CRAP/coverage impact) +**Every number must come from the script output, and the arithmetic must reconcile.** Uncovered lines attributed to individual members must not exceed the project's total uncovered lines, and the coverage you project after a recommendation must follow from those counts. + +**List every member below threshold, not just the worst one.** `Extract-MethodCoverage.ps1` returns the full below-threshold set: name the others even if briefly. Only say "the rest is fine / leave it alone" when that set is otherwise empty — claiming one method is the entire gap when the extractor found more is a factual error. + Use `references/output-format.md` verbatim for fixed headings, table structures, symbols, and emoji. Use `references/guidelines.md` for prioritization rules and style. If Phase 5 has not yet run when you compose this summary, mark the `## 📁 Reports` section's HTML/Text/CSV/GitHub-markdown rows as `Not generated (optional — request HTML reports to enable)`. Only the `coverage-analysis.md` and raw Cobertura paths are guaranteed to exist. @@ -531,3 +536,5 @@ After Phase 5 completes successfully, you may follow up with a short message poi - **ReportGenerator install failure** — if `dotnet tool install` fails (no internet) during Phase 5, leave the existing Phase 4 summary as the final output and note that HTML reports were skipped. Do not retry or block on the install. - **Method name mismatches in Cobertura** — async methods, lambdas, and local functions may have compiler-generated names. The scripts use the Cobertura method name/signature directly; verify against source if results look unexpected. - **Mixed coverage providers** — when a solution contains both Coverlet and Microsoft CodeCoverage projects, the skill runs per-project to avoid dual-provider conflicts. This is slower but correct. +- **Numbers that don't reconcile** — per-member uncovered lines that exceed the project total, or a projected coverage figure that doesn't follow from the counts, make the whole analysis untrustworthy. Re-read the script output rather than estimating. +- **Declaring one method "the entire gap"** — check the full below-threshold list from `Extract-MethodCoverage.ps1` first; naming a single blocker while other uncovered members exist misdirects the user's next test. diff --git a/plugins/dotnet-test/skills/crap-score/SKILL.md b/plugins/dotnet-test/skills/crap-score/SKILL.md index 352dba4bf9..c7b0f910f8 100644 --- a/plugins/dotnet-test/skills/crap-score/SKILL.md +++ b/plugins/dotnet-test/skills/crap-score/SKILL.md @@ -71,6 +71,21 @@ Check the test project's `.csproj` for the coverage package, then run the approp | `Microsoft.Testing.Extensions.CodeCoverage` (.NET 9) | `dotnet test -- --coverage --coverage-output-format cobertura --coverage-output ./TestResults` | `--coverage-output` path | | `Microsoft.Testing.Extensions.CodeCoverage` (.NET 10+) | `dotnet test --coverage --coverage-output-format cobertura --coverage-output ./TestResults` | `--coverage-output` path | +#### Never estimate coverage + +**Guessed coverage produces wrong CRAP scores, which is worse than no answer.** If the first command yields no Cobertura XML, work down this list before giving up: + +1. Add a provider if none is referenced: `dotnet add package coverlet.collector`, then re-run. +2. Use the standalone collector, which works even when the test host or a shared assembly blocks the in-proc collector: + `dotnet tool install --global dotnet-coverage` then + `dotnet-coverage collect -f cobertura -o coverage.cobertura.xml "dotnet test "`. +3. Convert or summarize an existing report with ReportGenerator when only binary `.coverage` output exists: + `dotnet tool install --global dotnet-reportgenerator-globaltool` then + `reportgenerator -reports: -targetdir:cov -reporttypes:Cobertura`. +4. Tests fail but still run? Coverage is collected from the tests that executed — continue with that data and note the failures. + +If every path fails, **report that coverage could not be collected, show the commands you tried and their errors, and stop.** Report complexity on its own if useful, but never publish a CRAP number derived from an assumed coverage percentage. + ### Step 2: Compute cyclomatic complexity Analyze the target source files to determine cyclomatic complexity per method. Count the following decision points (each adds 1 to the base complexity of 1): @@ -146,12 +161,16 @@ Report this as: "To bring `ProcessOrder` (complexity 12) below CRAP 15, increase ## Validation - Verify that coverage data was collected successfully (Cobertura XML exists and contains data) +- Confirm every coverage figure came from that XML — no estimated, assumed, or source-comment-derived values - Cross-check that method names in coverage data match the source code - Confirm CRAP scores by spot-checking the formula on one method manually - Ensure a 100%-covered method's CRAP equals its complexity exactly ## Common Pitfalls +- **Estimating coverage when collection fails**: never do it — the resulting CRAP scores are wrong in the direction that matters. Work through the fallbacks in Step 1, then report the blocker instead. +- **Trusting a stale complexity comment in the source**: compute cyclomatic complexity from the current code; a `// complexity: 7` comment left by a previous author is not evidence. +- **Giving up on a shared-assembly or test-host collector error**: `dotnet-coverage collect` runs out of process and usually succeeds where the in-proc collector fails. - **Stale coverage data**: Always regenerate coverage before computing CRAP scores. Old coverage files will produce misleading results. - **Method name mismatches**: Cobertura XML may use mangled/compiler-generated names for async methods, lambdas, or local functions. Match by line ranges when names don't align. - **Generated code**: Exclude auto-generated files (e.g., `*.Designer.cs`, `*.g.cs`) from analysis unless explicitly requested. diff --git a/plugins/dotnet-test/skills/detect-static-dependencies/SKILL.md b/plugins/dotnet-test/skills/detect-static-dependencies/SKILL.md index 46bda03b03..dca80e40a3 100644 --- a/plugins/dotnet-test/skills/detect-static-dependencies/SKILL.md +++ b/plugins/dotnet-test/skills/detect-static-dependencies/SKILL.md @@ -64,7 +64,9 @@ Scan each file for calls matching these categories: | Category | Patterns to search for | Recommended replacement | |----------|----------------------|------------------------| | **Time** | `DateTime.Now`, `DateTime.UtcNow`, `DateTime.Today`, `DateTimeOffset.Now`, `DateTimeOffset.UtcNow`, `Task.Delay(`, `new CancellationTokenSource(TimeSpan` | `TimeProvider` (.NET 8+) | -| **File System** | `File.ReadAllText(`, `File.WriteAllText(`, `File.Exists(`, `File.Delete(`, `File.Copy(`, `File.Move(`, `Directory.Exists(`, `Directory.CreateDirectory(`, `Directory.GetFiles(`, `Directory.Delete(`, `Path.Combine(`, `Path.GetTempPath(` | `IFileSystem` (System.IO.Abstractions NuGet) | +| **File System** | `File.ReadAllText(`, `File.WriteAllText(`, `File.Exists(`, `File.Delete(`, `File.Copy(`, `File.Move(`, `Directory.Exists(`, `Directory.CreateDirectory(`, `Directory.GetFiles(`, `Directory.Delete(`, `Path.GetTempPath(`, and instance members that hit the disk (`new FileInfo(...)`, `new DirectoryInfo(...)`, `.LastWriteTimeUtc`, `new StreamReader(path)`) | `IFileSystem` (System.IO.Abstractions NuGet) | +| **Randomness / identity** | `new Random(`, `Random.Shared`, `Guid.NewGuid(` | `TimeProvider`-style seam: inject `Random` / an `IGuidProvider` | +| **Culture / serialization** | `CultureInfo.CurrentCulture`, `CultureInfo.CurrentUICulture`, `JsonSerializer.Serialize(`, `JsonSerializer.Deserialize(` | Pass culture/options explicitly, or inject a serializer abstraction | | **Environment** | `Environment.GetEnvironmentVariable(`, `Environment.SetEnvironmentVariable(`, `Environment.MachineName`, `Environment.UserName`, `Environment.CurrentDirectory`, `Environment.Exit(` | Custom `IEnvironmentProvider` | | **Network** | `new HttpClient(`, `HttpClient.GetAsync(`, `HttpClient.PostAsync(`, `HttpClient.SendAsync(` | `IHttpClientFactory` (built-in) | | **Console** | `Console.WriteLine(`, `Console.ReadLine(`, `Console.Write(`, `Console.ReadKey(` | `IConsole` wrapper or `ILogger` | @@ -72,7 +74,18 @@ Scan each file for calls matching these categories: ### Step 3: Aggregate and rank results -Count each static call pattern across the entire scan scope. Produce a summary with: +Count each call site across the entire scan scope — including the instance-member call sites covered by the rules below, not only `static` ones. + +**Counting rules — inaccurate totals are the main way this report loses to an ad-hoc scan:** + +- **One authoritative total.** Every call site you found belongs in the category summary and the grand total. Never park real findings in an "additional observations" section that the totals exclude. +- **Classify by what the member touches, not by whether it is `static`.** Instance members that reach the same untestable resource still count and belong in the matching category (`new FileInfo(path).LastWriteTimeUtc` → File System; `httpClient.GetAsync(...)` → Network). Say "hidden dependency", not "static", when the member is an instance call. +- **Exclude deterministic pure helpers from the "needs wrapping" total.** `Path.Combine`, `Path.GetExtension`, `Path.GetFileName`, and `Math.*`/`string.*` statics take no ambient input and are trivially testable. List them, if at all, in a separate "no action needed" note — never as testability blockers. +- **Cover every category before reporting** — time, file system, environment, network, console, process, randomness (`new Random()`, `Guid.NewGuid()`), culture (`CultureInfo.CurrentCulture`), and serialization/statics such as `JsonSerializer`. Omitting a category that is present is an under-count. +- **Give `file:line` for every occurrence** so the user can jump straight to it. +- **Reconcile before publishing.** The category totals, the top-patterns table, and the per-file table must sum to the same grand total. + +Produce a summary with: 1. **Category summary** — total call sites per category (time, filesystem, env, etc.) 2. **Top patterns** — the 10 most frequent individual patterns ranked by count @@ -125,15 +138,18 @@ Format the output as a structured report: ### Step 5: Suggest next steps -Based on the report, recommend: -- Which category to tackle first (fewest dependencies, best built-in support) -- Whether to use `generate-testability-wrappers` for custom wrapper generation -- Whether to use `migrate-static-to-wrapper` for mechanical bulk migration +Based on the report, recommend which category to tackle first (highest count, best built-in support). Keep this to a few lines. + +Mention `generate-testability-wrappers` or `migrate-static-to-wrapper` only when the user's next action clearly needs them — a hand-off note, not a sales pitch. Never end an audit with promotional next-steps that dilute the findings. ## Validation - [ ] All `.cs` files in scope were scanned (check count) - [ ] Report includes category totals, top patterns, and affected files +- [ ] Category totals, top patterns, and per-file counts reconcile to the same grand total +- [ ] Every occurrence carries a `file:line` location +- [ ] No findings are held outside the totals in an "additional" section +- [ ] Deterministic pure helpers (`Path.Combine`, `Math.*`) are not counted as testability blockers - [ ] Each detected pattern has a recommended replacement listed - [ ] `obj/` and `bin/` directories were excluded - [ ] Migration priority is ordered by impact (count × ease of replacement) @@ -147,3 +163,6 @@ Based on the report, recommend: | Missing statics inside lambdas/LINQ | Search covers all code within `.cs` files, including lambdas | | Recommending `TimeProvider` on < .NET 8 | Check `TargetFramework` in `.csproj` — if < net8.0, recommend `NodaTime.IClock` or custom `ISystemClock` | | Ignoring test projects | Only scan production code — exclude `*.Tests.csproj` projects from the scan | +| Under-counting by relegating findings | Real call sites belong in the category totals, not in a trailing "also noticed" paragraph that the totals ignore | +| Calling an instance member a static | `new FileInfo(p).LastWriteTimeUtc` is an instance call but still a hidden file-system dependency — count it under File System and describe it accurately | +| Recommending a wrapper for `Path.Combine` | Pure, deterministic helpers need no seam; listing them as blockers makes the recommendations wrong | diff --git a/plugins/dotnet-test/skills/migrate-static-to-wrapper/SKILL.md b/plugins/dotnet-test/skills/migrate-static-to-wrapper/SKILL.md index 6f41f1e10a..e91d47583b 100644 --- a/plugins/dotnet-test/skills/migrate-static-to-wrapper/SKILL.md +++ b/plugins/dotnet-test/skills/migrate-static-to-wrapper/SKILL.md @@ -2,19 +2,17 @@ name: migrate-static-to-wrapper description: > Replace existing static dependency call sites with a wrapper or built-in - abstraction that already exists or is registered in DI. Codemod-style bulk - replacement of DateTime.Now/UtcNow to TimeProvider, File.ReadAllText to - IFileSystem, and similar, across a bounded scope (file, project, namespace), - adding constructor injection to affected classes and updating their unit tests - to use a test double. + abstraction that already exists or is registered in DI, across a bounded scope + (file, project, namespace). USE FOR: replace DateTime.UtcNow/DateTime.Now with TimeProvider and add the constructor parameter, migrate static call sites to a wrapper already in DI, bulk replace File.* with IFileSystem, scoped migration of statics in only - certain files, migrate a service to TimeProvider and update its unit tests to a - controllable/fake time source, update test doubles when migrating off static - DateTime/File calls. - DO NOT USE FOR: detecting statics (use detect-static-dependencies), creating or - registering the wrapper when it does not exist yet (use + certain files, update unit tests to a fake time source, make an existing + static or utility class testable by adding an ambient + TimeProvider/IFileSystem seam while every current call site keeps compiling, + behavior-preserving time refactors that must keep the same DateTimeKind. + DO NOT USE FOR: detecting statics (use detect-static-dependencies), designing a + brand-new wrapper interface that does not exist yet (use generate-testability-wrappers), migrating between test frameworks. license: MIT --- @@ -29,15 +27,20 @@ Perform mechanical, codemod-style replacement of static dependency call sites wi - Migrating `DateTime.UtcNow` → `TimeProvider.GetUtcNow()` across a project - Migrating `File.*` → `IFileSystem.File.*` across a namespace - Adding constructor injection for the new abstraction to affected classes +- Making a `static` utility class testable by adding an ambient seam (Step 3) while its existing call sites keep + compiling unchanged - Incremental migration: one project or namespace at a time ## When Not to Use -- No wrapper or abstraction exists yet (use `generate-testability-wrappers` first) +- No wrapper or abstraction exists yet and one must be designed from scratch (use `generate-testability-wrappers` first). + A built-in abstraction such as `TimeProvider` or `IFileSystem` always counts as existing. - The user wants to detect statics, not migrate them (use `detect-static-dependencies`) -- The code does not use dependency injection and the user hasn't chosen ambient context - Migrating between test frameworks (use the appropriate migration skill) +> A class that is `static`, or a project with no DI container, is **not** a reason to skip this skill — that is exactly +> what the ambient seam in Step 3 is for. Use it whenever the call sites must keep compiling unchanged. + ## Inputs | Input | Required | Description | @@ -61,6 +64,8 @@ Before modifying any code: ### Step 2: Plan the migration for each file +**Migrate exactly what was asked — nothing adjacent.** If the user named a member (`DateTime.UtcNow`), migrate only that member and leave siblings such as `DateTime.Now` untouched. If the user named files, do not touch other files. Never migrate a call site whose comment or name marks it as deliberate (e.g. `// intentional local time`). List everything you deliberately left alone under "Remaining (out of scope)" so the user can ask for it in a follow-up; suggesting is fine, silently widening the scope is not. + For each file containing the static pattern, determine: 1. **Which class(es) contain the call sites** — identify the class declarations @@ -171,6 +176,8 @@ After all changes in the current scope: dotnet build ``` +**Report the build result you actually observed.** Only write "build succeeded" when the command exited 0; if it failed — including restore/NuGet failures such as "assets file not found" — say so, quote the error, and either fix it (`dotnet restore`, add the missing package) or hand the user a precise blocker. A false success claim is worse than an unfinished migration. + If the build fails: - **Missing using**: Add the required `using` directive - **Missing NuGet package**: Run `dotnet add package ` @@ -205,11 +212,13 @@ Summarize what was done: ## Validation - [ ] All call sites in scope were replaced (none missed) +- [ ] No call site outside the requested member/file scope was modified +- [ ] Call sites documented as intentional (e.g. local time) were left untouched and reported - [ ] Constructor injection added to all affected classes - [ ] Field naming follows existing class conventions - [ ] Required `using` directives added - [ ] Required NuGet packages referenced -- [ ] Build succeeds after migration +- [ ] Build succeeds after migration, and the reported result matches the actual command exit code - [ ] Test files updated with appropriate test doubles - [ ] No behavioral changes introduced (wrapper delegates directly to the static) - [ ] `DateTimeKind` preserved — former `DateTime.UtcNow` stays `Utc` (`.UtcDateTime`), former `DateTime.Now` stays `Local` (`.LocalDateTime`) @@ -223,4 +232,6 @@ Summarize what was done: | Missing `FakeTimeProvider` NuGet | Add `Microsoft.Extensions.TimeProvider.Testing` to test project | | Replacing a `DateTime` value with `.DateTime` off a `DateTimeOffset` | `DateTimeOffset.DateTime` returns `Kind == Unspecified` — use `.UtcDateTime` (for former `DateTime.UtcNow`) or `.LocalDateTime` (for former `DateTime.Now`) to preserve the original `DateTimeKind`. Only change the field/return type to `DateTimeOffset` if the user asked for it. | | Migrating too much at once | Stick to the defined scope — one project or namespace per run | +| Migrating `DateTime.Now` when only `UtcNow` was requested | Respect the literal request; list the other call sites as out-of-scope suggestions instead of rewriting them | +| Claiming "Build succeeded" after a failed restore | Read the exit code and output; report the real failure and fix it or surface it as a blocker | | Forgetting DI registration | Always verify `Program.cs`/`Startup.cs` has the registration before replacing call sites | diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index cdcabca43d..300cebaf17 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -130,6 +130,13 @@ IMPORTANT: If the tests are well-written, say so clearly up front. Do not inflat ### Step 5: Report findings +**Depth bar — a tidy report that is shallower than an unassisted review is a failure.** Before writing, satisfy all four: + +1. **Account for every test in scope.** Walk the full list of test methods and fields; a finding table that silently skips tests (or fixtures like an unused `static HttpClient` field) is incomplete. State the number of tests reviewed. +2. **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. +3. **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. +4. **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. @@ -151,8 +158,11 @@ If there are many findings, recommend which to fix first: ## Validation +- [ ] Every test method in scope is accounted for (reviewed count stated; none silently skipped) - [ ] Every finding includes a specific location (not just a general warning) -- [ ] Every Critical/High finding includes a concrete fix +- [ ] Every Critical/High finding includes a concrete fix with exact expected values +- [ ] Adjacent untested error paths and boundary values are called out +- [ ] Summary counts match the enumerated findings - [ ] Report covers all categories (assertions, isolation, naming, structure) - [ ] Positive observations are included alongside problems - [ ] Recommendations are prioritized by severity @@ -167,7 +177,8 @@ If there are many findings, recommend which to fix first: | 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 correct terminology per the loaded language extension: xUnit `[Fact]`/`[Theory]`, NUnit `[Test]`/`[TestCase]`, MSTest `[TestMethod]`/`[DataRow]`, pytest `def test_*` / `@pytest.mark.parametrize`, Jest `it.each` / `describe`, JUnit `@Test` / `@ParameterizedTest`, Go `func TestXxx(t *testing.T)` + table-driven, RSpec `describe`/`it`, Pester `Describe`/`It`, Rust `#[test]` / `#[rstest]`, Catch2 `TEST_CASE`/`SECTION`. | -| Treating idiomatic patterns as smells | Go/Rust **table-driven loops** are idiomatic. Pytest **bare `assert`** is canonical. Go's `if got != want { t.Errorf(...) }` is canonical. JS/TS `expect(mock).toHaveBeenCalledWith(...)` is a real assertion, not an over-mock. Do NOT flag these. | -| Missing async-test pitfalls | A Jest test that calls `expect(promise).resolves.toBe(x)` without returning/awaiting the promise silently passes; a TUnit/xUnit `async Task` test calling `Assert.ThrowsAsync` without `await` silently passes; pytest-asyncio tests with un-awaited coroutines silently pass. Always flag as Critical. | +| 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 | diff --git a/plugins/dotnet-test/skills/test-gap-analysis/SKILL.md b/plugins/dotnet-test/skills/test-gap-analysis/SKILL.md index 18796759cb..973a20dd33 100644 --- a/plugins/dotnet-test/skills/test-gap-analysis/SKILL.md +++ b/plugins/dotnet-test/skills/test-gap-analysis/SKILL.md @@ -6,7 +6,7 @@ license: MIT # Test Gap Analysis via Pseudo-Mutation -Analyze production code in any supported language by reasoning about hypothetical mutations and checking whether existing tests would catch them. This reveals blind spots where tests pass but would continue to pass even if the code were broken. +Analyze production code in any supported language by reasoning about hypothetical mutations, then confirming them against the real test suite. This reveals blind spots where tests pass but would continue to pass even if the code were broken. > **Language-specific guidance**: Call the `test-analysis-extensions` skill to discover available extension files, then read the file matching the target codebase (e.g., `extensions/dotnet.md`, `extensions/python.md`, `extensions/typescript.md`). The extension file helps you find test files, recognize framework-specific assertion APIs, and identify language-specific null/None/nil patterns and error-handling idioms that map to the mutation catalog below. @@ -22,7 +22,7 @@ Pseudo-mutation analysis asks: _"If I changed this line, would any test fail?"_ | Branch coverage | Which branches taken | Whether both branches produce different asserted outcomes | | **Mutation score** | Whether tests detect code changes | Nothing — this is the gold standard | -This skill performs **static pseudo-mutation** — reasoning about mutations without actually running them — to approximate mutation testing at the speed of code review. +This skill uses **static pseudo-mutation** to find mutation candidates at the speed of code review, then **confirms each reported survivor by actually applying it and re-running the covering tests** (Step 4b). Reasoning finds the candidates; execution decides the verdict. ## When to Use @@ -149,6 +149,18 @@ For each identified mutation point, reason about whether existing tests would de | **No coverage** | No test exercises this code path at all | Worse than survived — the code is untested | | **Equivalent** | The mutation produces identical behavior (e.g., `x * 1` → `x / 1`) | Skip — not a real mutation | +### Step 4b: Verify every reported survivor by running the tests + +Static reasoning alone produces false "survived" verdicts — the most common failure of this skill is telling a user their tests are weak when the tests actually catch the bug. **If the suite can be built and run, verify before you report.** + +1. **Establish a green baseline.** Run the suite (`dotnet test`, `pytest`, `npm test`, `go test ./...`, `cargo test`, …). Record the pass count. If it does not build or run, fix only obvious wiring problems (missing project reference, wrong runner arguments) — never rewrite production behavior. +2. **Apply each candidate survivor as a real edit**, one at a time, to the production file. +3. **Re-run the narrowest covering test(s).** Still green ⇒ genuinely **Survived**. Now red ⇒ **Killed**; drop it from the findings. +4. **Revert immediately** after each check, and confirm the suite is green again before finishing. Never leave a mutation in the working tree. +5. **Report what you did**: state that survivors were empirically confirmed and give the observed kill count (e.g. "8 of 9 injected mutations were caught"). + +When the suite genuinely cannot be executed (no toolchain, no network, compile errors you must not fix), fall back to reasoning — but label every finding **unverified (static reasoning)**, and downgrade its confidence rather than presenting it as a proven gap. + ### Step 5: Calibrate findings Before reporting, apply these calibration rules: @@ -158,6 +170,8 @@ Before reporting, apply these calibration rules: - **Equivalent mutations are not gaps.** If changing `>=` to `>` doesn't alter behavior because the `==` case is impossible given the domain, mark it Equivalent and skip. - **Private methods reached through public API are valid targets.** Trace through the call chain — a private method called from a tested public method may still have survived mutations if the test doesn't assert the specific behavior affected. - **Rate by risk, not count.** A single survived mutation in payment calculation logic is more important than five survived mutations in logging code. +- **When most mutations are killed, lead with that.** A strong suite with one or two minor survivors is a good result: say "the tests are strong" first and present the survivors as minor improvements. Do not attach "critical" / "HIGH RISK" framing to a suite that kills the substantive mutations. +- **Never claim a gap you did not verify.** If Step 4b confirmed the mutation is caught, it is not a finding. Prefer under-claiming: a missed gap costs the user far less than a false alarm that sends them writing redundant tests. ### Step 6: Report findings @@ -195,6 +209,8 @@ Present the analysis in this structure: ## Validation +- [ ] The suite was run and every reported survivor was empirically confirmed (or all findings are explicitly labelled unverified) +- [ ] All applied mutations were reverted and the suite is green again - [ ] Every mutation point was classified (Killed / Survived / No coverage / Equivalent) - [ ] Every survived mutation includes the original code, the hypothetical change, and why tests miss it - [ ] Every survived mutation includes a concrete recommended fix (a test assertion or test case) @@ -208,6 +224,10 @@ Present the analysis in this structure: | Pitfall | Solution | |---------|----------| +| Reporting survivors you never ran | Apply the mutation, re-run the covering tests, revert. An unverified "survived" claim that the tests actually kill is worse than no analysis | +| Overstating severity on a strong suite | If the substantive mutations are killed, open with "tests are strong" and frame the remainder as minor improvements | +| Publishing your reasoning as it changes | Settle the verdict first; never emit "wait, no… this is killed… let me recalibrate" in the report | +| Leaving a mutation applied | Revert every edit and re-run the suite before reporting | | Analyzing trivial code | Skip auto-properties, simple getters, `@dataclass`/`record`/`data class` accessors, `#[derive]` impls — focus on logic | | Reporting equivalent mutations as gaps | If the mutation doesn't change behavior, it's not a gap — mark Equivalent | | Ignoring call chains | A private/internal/unexported helper called from a tested public method is reachable — trace the chain | diff --git a/plugins/dotnet-test/skills/test-smell-detection/SKILL.md b/plugins/dotnet-test/skills/test-smell-detection/SKILL.md index 6f30457e73..4a8517c16d 100644 --- a/plugins/dotnet-test/skills/test-smell-detection/SKILL.md +++ b/plugins/dotnet-test/skills/test-smell-detection/SKILL.md @@ -104,6 +104,7 @@ Tests that depend on external resources — files on disk, databases, network en Tests that call sleep or delay functions to wait for a condition. These introduce non-deterministic timing and slow down the suite. **Severity:** High +**Calibration:** Severity does **not** drop because the test is an integration test — a fixed sleep is still flaky and slow there. Report it as High and recommend polling/awaiting the condition with a timeout. **Detection:** Calls to sleep/delay functions inside test methods: `Thread.Sleep` / `Task.Delay` (.NET); `time.sleep` / `asyncio.sleep` (Python); `setTimeout` / `await new Promise(r => setTimeout(...))` / `jest.advanceTimersByTime` not paired with a wait (JS/TS); `Thread.sleep` / `TimeUnit.SECONDS.sleep` (Java); `time.Sleep` (Go); `sleep` / `Kernel#sleep` (Ruby); `std::thread::sleep` / `tokio::time::sleep` (Rust); `Thread.sleep` / `delay` (Kotlin coroutines); `sleep(_:)` / `Task.sleep` (Swift); `Start-Sleep` (Pester); `std::this_thread::sleep_for` (C++). See the matching language extension file for the full list. #### Smell 4: Assertion-Free Test (Unknown Test) @@ -168,7 +169,7 @@ Tests marked as skipped or disabled. These add overhead and clutter, and the und Before reporting, calibrate findings to avoid false positives: -- **Integration tests have different norms.** A test class clearly marked as integration (by name, annotation, category, or convention — see the loaded language extension file for markers) 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. +- **Integration tests have different norms — but not for sleeps.** A test class clearly marked as integration (by name, annotation, category, or convention — see the loaded language extension file for markers) legitimately uses external resources and calls multiple methods. Downgrade Mystery Guest and Eager Test for integration tests. **Do NOT downgrade Sleepy Test:** a fixed wall-clock sleep is non-deterministic and slow in any test category, so it stays a real High-severity smell — recommend polling/awaiting the condition with a timeout instead. Only treat a sleep as acceptable when it is bounded by a documented external constraint (e.g. a third-party rate limit) *and* paired with a condition check. - **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. - **Idiomatic table-driven and parametrized patterns are NOT Conditional Test Logic.** Go's `for _, tt := range tests { t.Run(...) }`, Rust's `#[rstest]`, pytest's `@parametrize`, Jest/Vitest `.each`, JUnit `@ParameterizedTest`, RSpec `where`, Pester `-ForEach`, Catch2 `SECTION`/`GENERATE`, GoogleTest `INSTANTIATE_TEST_SUITE_P` are canonical and must NOT be flagged. - **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. @@ -214,7 +215,8 @@ Present the analysis in this structure: - [ ] 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 +- [ ] Integration tests are not penalized for using real resources, but their fixed sleeps are still reported as High +- [ ] Each smell is reported under its own taxonomy name (Unknown Test, Empty Test, Assertion Roulette are distinct — do not merge them) - [ ] 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 @@ -224,12 +226,11 @@ Present the analysis in this structure: | Pitfall | Solution | |---------|----------| -| Flagging integration tests for using real resources | Check for integration test markers (per the loaded language extension) and adjust severity accordingly | +| Flagging integration tests for using real resources | Check for integration test markers (per the loaded language extension) and adjust severity accordingly — external resources and multi-step flows are expected there | +| Calibrating away a real sleep as an "integration style issue" | `Thread.Sleep(3000)` in an integration test is still a High-severity Sleepy Test; recommend a polled wait with timeout | | Flagging loop-over-collection-assert as conditional logic | Only flag loops with branching or complex logic, not assertion iterations | | Flagging Go/Rust table-driven loops as Conditional Test Logic | `for _, tt := range tests { t.Run(...) }` (Go) and `#[rstest]` loops (Rust) are canonical and must NOT be flagged | | Flagging parametrized tests as Duplicate Assert | `@pytest.mark.parametrize`, `it.each`, `[Theory]+[InlineData]`, `@ParameterizedTest`, RSpec `where`, Pester `-ForEach`, Catch2 `SECTION`/`GENERATE` are correct deduplication, not smells | -| Flagging pytest bare `assert` as missing framework | Bare `assert` is canonical pytest assertion — count it | -| Flagging Go's `if err != nil { t.Fatal(err) }` as Exception Handling in Tests | This is canonical Go error checking — do NOT flag | | Flagging obvious count assertions after adding N items | Consider the immediate context — self-documenting numbers are fine | | Missing framework-specific assertion syntax | Always read the matching language extension file first; each framework has distinct assertion APIs (xUnit `Assert.Equal`, MSTest `Assert.AreEqual`, NUnit `Is.EqualTo`, pytest bare `assert`, Jest `expect().toBe()`, etc.) | | Treating mock-call verifications as assertion-free | `mock.Verify(...)`, `expect(mock).toHaveBeenCalledWith(...)`, `Should -Invoke`, `verify(mock).method(...)`, `mock.assert_called_with(...)` are real assertions | diff --git a/tests/dotnet-test/assertion-quality/eval.yaml b/tests/dotnet-test/assertion-quality/eval.yaml index 5a970304e7..8e07c971f7 100644 --- a/tests/dotnet-test/assertion-quality/eval.yaml +++ b/tests/dotnet-test/assertion-quality/eval.yaml @@ -33,7 +33,8 @@ stimuli: negative amounts) - Identified GetReceipt test as having only a trivial Assert.IsNotNull without checking the receipt's content - Noted that no tests verify collection contents in GetTransactionHistory — only the count is checked - - Recommended specific additional assertion types that would improve coverage (exception, collection, structural) + - Recommended additional assertions that would catch bugs the current suite cannot — covering error/exception + behaviour, collection contents, and object state — regardless of how those categories are labelled - Provided concrete examples of assertions that could be added to specific test methods - name: Flag assertion-free tests and trivial-only assertions prompt: | @@ -66,8 +67,8 @@ stimuli: verify content - Explained that these tests give false security — code coverage may look good but correctness is not verified - Recommended specific meaningful assertions for at least some of the assertion-free tests - - Distinguished between intentional smoke tests (acceptable if labeled as such) and tests masquerading as real - verification + - Separated tests that merely execute code without verifying it from tests that are deliberately only checking + "this does not throw", instead of lumping every assertion-free test together - name: Recognize well-diversified assertion usage prompt: | Can you evaluate the assertion quality in my UserService.Tests project? @@ -88,8 +89,8 @@ stimuli: - Recognized that the test suite uses a variety of assertion types including equality, boolean, null, exception, and collection assertions - "Noted positive patterns: tests check multiple properties on returned objects, not just one field" - - Acknowledged the negative assertions (Assert.IsNull for non-existent user, Assert.IsFalse for failed delete, - Assert.AreNotEqual for role change) + - Credited the tests that verify absence and failure outcomes (no user found, delete returning false, role + actually changing) as genuine verification rather than counting them as weak - Acknowledged the exception assertions with specific exception types and parameter name verification - If any gaps were identified, they were presented as minor enhancement opportunities rather than problems - name: Identify self-referential assertions in identity and round-trip tests @@ -129,8 +130,8 @@ stimuli: string's actual structure or content - Flagged ValidateKey_Valid_ReturnsSameKey as not exercising the validation logic — it only proves valid input passes through unchanged - - Noted that Serialize_Deserialize round-trip is potentially valid but should be accompanied by tests with - edge-case inputs (special characters, nulls, empty strings) + - Noted that the Serialize_Deserialize round-trip would only become meaningful with inputs that can actually + break it (special characters, nulls, empty strings), rather than dismissing or endorsing it outright - "Recommended concrete improvements: test actual default values, test invalid keys, test formatted string structure, test edge-case serialization inputs" constraints: @@ -143,6 +144,15 @@ stimuli: I need to write unit tests for my InventoryTracker class. It handles stock levels, reorder alerts, and warehouse transfers. Can you help me write a full MSTest test suite from scratch? + # Off-target request: this skill reviews existing assertions, it does not + # author suites. The skill IS loaded here on purpose — the property under + # test is that it stays dormant and does not hijack the request. Do not add + # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it + # identical to the baseline arm, so the head-to-head score becomes pure + # judge noise instead of a measurement (it swung to -40% in run + # 30217274869). `expect_activation: false` alone records the expected + # dormancy without polluting the verdict. + expect_activation: false environment: files: - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs @@ -155,6 +165,36 @@ stimuli: rubric: - Wrote test methods for the InventoryTracker class - Covered multiple methods of the class - constraints: - reject_skills: - - "*" + - Did not derail into an assertion-quality audit of code the user never asked about + + - name: Judge assertion strength in a shallow Jest suite + prompt: | + These Jest tests for our OrderService all pass and the file looks busy, + but I don't trust them. Are these assertions actually verifying anything? + environment: + files: + - src: fixtures/jest-shallow/package.json + dest: package.json + - src: fixtures/jest-shallow/src/order.ts + dest: src/order.ts + - src: fixtures/jest-shallow/tests/order.test.ts + dest: tests/order.test.ts + graders: + - type: output-matches + config: + pattern: (toBeDefined|toBeTruthy|toBeNull|shallow|weak|trivial) + - type: output-matches + config: + pattern: (await|resolves|async) + - type: prompt + rubric: + - Identified that the create/list tests assert only existence or type rather than the returned order's actual + field values + - Flagged the always-true assertion that cannot fail regardless of the code's behavior + - Flagged the un-awaited promise assertion as silently passing even when the underlying promise rejects, + which is the most dangerous defect in the file + - Flagged the assertion that compares a value to itself as incapable of detecting any bug + - Flagged the not-equal-to-an-unrelated-string assertion as passing for almost any possible value + - Recognized that the two tests pinning the returned customer and asserting the thrown error message do verify + real behavior, instead of condemning the whole file + - Recommended concrete replacement assertions pinning the actual expected values diff --git a/tests/dotnet-test/code-testing-agent/eval.yaml b/tests/dotnet-test/code-testing-agent/eval.yaml index 551154ce70..802d3f8f3c 100644 --- a/tests/dotnet-test/code-testing-agent/eval.yaml +++ b/tests/dotnet-test/code-testing-agent/eval.yaml @@ -93,8 +93,9 @@ stimuli: - At least one Cart test exercises the *intersection* of multiple pricing dimensions (e.g., a CompositeDiscountPolicy combined with a RegionalTaxCalculator and a WeightBasedShippingCalculator above the threshold) rather than only one dimension at a time, exercising property intersections - - The completion summary cites concrete generated test names for every requested policy/calculator boundary, async - rejection path, mocked seam, and the multi-dimension pricing intersection, plus the coverage artifact + - Every requested behaviour is individually traceable to a named generated test — each policy/calculator + boundary, async rejection path, mocked seam and the multi-dimension pricing intersection is backed by a + specific test name rather than a broad claim of coverage — and the coverage artifact is cited - Avoided redundant tests after all requested behaviors and coverage thresholds were satisfied; a smaller focused suite is not penalized merely for having fewer tests than another passing suite @@ -144,7 +145,74 @@ stimuli: - Accepted the sparse workspace as authoritative instead of treating it as corruption - Tests assert concrete behavior of reverse, is_palindrome, word_count, and truncate, including negative-limit ValueError and ellipsis truncation - - The completion summary contains a `Requirement | Evidence` table mapping each required synthstr behavior and - validation result to exact test names or command evidence rather than only listing broad tested areas + - Each required synthstr behaviour and validation result is individually traceable to a named test or a cited + command, rather than being covered by a broad list of tested areas - Once every requested behavior is covered, evaluates suite quality by meaningful, nonredundant cases rather than rewarding a higher raw test count + + # Compact C# scenario: the existing suite already covers BillableWeight, so the + # agent must extend coverage to QuoteAsync without duplicating or rewriting it. + - name: Extend an existing suite to the untested method only + prompt: | + The ShippingQuotes project under fixtures/csharp-shipping-quotes/ has a test project that + already covers BillableWeight, but QuoteAsync has no tests at all. Use the repository's + standard test-generation workflow to add tests for QuoteAsync only — put them in a new test + file and leave BillableWeightTests.cs exactly as it is, without re-testing BillableWeight. + + QuoteAsync must be tested with IRateProvider and ISurchargeTable mocked or stubbed, never + against real I/O, following the existing xUnit conventions in the project. Please cover the + fixed pricing pipeline (the fuel surcharge applies to the base price and the handling fee is + added afterwards so it is never surcharged), the QuoteUnavailableException paths, and the + blank-destination guard. The suite should pass with + `dotnet test fixtures/csharp-shipping-quotes/tests/ShippingQuotes.Tests.csproj`. + environment: + files: + - src: fixtures/csharp-shipping-quotes + dest: fixtures/csharp-shipping-quotes + # Snapshot the pre-existing suite so the graders can assert it is + # byte-for-byte unmodified. Checking that one test method name still + # appears in the file would let a run rewrite it, or duplicate the + # BillableWeight cases into it, and still pass a scope-containment + # scenario. The snapshot is kept inside the workspace (not /tmp) because + # graders are only guaranteed to see workspace-relative paths, and it + # lives outside the test project so it is never compiled. + commands: + - mkdir -p .eval-baseline && cp fixtures/csharp-shipping-quotes/tests/BillableWeightTests.cs .eval-baseline/BillableWeightTests.cs + graders: + - type: run-command + config: + command: sh -c "dotnet test fixtures/csharp-shipping-quotes/tests/ShippingQuotes.Tests.csproj" + expected_exit_code: 0 + timeout: 10m + # Scope containment: the existing suite must be untouched, byte for byte. + - type: run-command + config: + command: sh -c "diff -u .eval-baseline/BillableWeightTests.cs fixtures/csharp-shipping-quotes/tests/BillableWeightTests.cs" + expected_exit_code: 0 + timeout: 1m + # The QuoteAsync tests must therefore live in a new file, and must exist. + - type: run-command + config: + command: sh -c "grep -rl QuoteAsync fixtures/csharp-shipping-quotes/tests --include='*.cs' | grep -v BillableWeightTests.cs | grep -q ." + expected_exit_code: 0 + timeout: 1m + - type: output-matches + config: + pattern: \|\s*Requirement\s*\|\s*Evidence\s*\| + - type: prompt + rubric: + - Added tests for QuoteAsync that compile and pass + - Left the existing BillableWeightTests.cs byte-for-byte unmodified and did not duplicate its BillableWeight + cases into the new tests + - Substituted IRateProvider and ISurchargeTable with mocks, fakes, or stubs rather than performing real I/O + - "Asserts the pipeline order with a concrete expected value: the fuel surcharge multiplies the base price + only, and the 4.50 handling fee is added afterwards so it is never surcharged" + - Covers the QuoteUnavailableException paths for a non-positive rate and for a rate provider that throws + HttpRequestException, asserting the exception type rather than just that something threw + - Covers the blank/whitespace destination guard as ArgumentException + - "Asserts numeric results with exact expected decimals rather than existence-only checks such as + Assert.IsNotNull" + - Each requested behaviour, including the pipeline-order requirement, is individually traceable to a named + generated test rather than to a general statement that the method is covered + - Once every requested behaviour is covered, judged on meaningful, nonredundant cases; a smaller focused suite + is not penalized merely for containing fewer tests than another passing suite diff --git a/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/QuoteCalculator.cs b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/QuoteCalculator.cs new file mode 100644 index 0000000000..f1d91ed406 --- /dev/null +++ b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/QuoteCalculator.cs @@ -0,0 +1,85 @@ +namespace ShippingQuotes; + +public interface IRateProvider +{ + Task GetRatePerKgAsync(string destination, CancellationToken cancellationToken); +} + +public interface ISurchargeTable +{ + decimal FuelSurchargeFor(string destination); +} + +public sealed class QuoteUnavailableException : Exception +{ + public QuoteUnavailableException(string destination, string reason) + : base($"No quote for '{destination}': {reason}") + { + Destination = destination; + Reason = reason; + } + + public string Destination { get; } + + public string Reason { get; } +} + +/// +/// Prices a shipment. The pipeline is fixed: base = rate * billable weight, +/// then the fuel surcharge is applied to that base, then the handling fee is +/// added last so it is never surcharged. +/// +public sealed class QuoteCalculator +{ + private const decimal HandlingFee = 4.50m; + + private readonly IRateProvider _rateProvider; + private readonly ISurchargeTable _surchargeTable; + + public QuoteCalculator(IRateProvider rateProvider, ISurchargeTable surchargeTable) + { + _rateProvider = rateProvider ?? throw new ArgumentNullException(nameof(rateProvider)); + _surchargeTable = surchargeTable ?? throw new ArgumentNullException(nameof(surchargeTable)); + } + + /// + /// Parcels under 1kg are billed at a 1kg minimum; above 30kg the shipment + /// is refused. A weight of exactly 30kg is still accepted. + /// + public decimal BillableWeight(decimal actualKg) + { + if (actualKg <= 0m) + throw new ArgumentOutOfRangeException(nameof(actualKg)); + + if (actualKg > 30m) + throw new QuoteUnavailableException("*", "over the 30kg limit"); + + return actualKg < 1m ? 1m : actualKg; + } + + public async Task QuoteAsync(string destination, decimal weightKg, CancellationToken cancellationToken = default) + { + if (string.IsNullOrWhiteSpace(destination)) + throw new ArgumentException("Destination is required.", nameof(destination)); + + var billable = BillableWeight(weightKg); + + decimal ratePerKg; + try + { + ratePerKg = await _rateProvider.GetRatePerKgAsync(destination, cancellationToken).ConfigureAwait(false); + } + catch (HttpRequestException ex) + { + throw new QuoteUnavailableException(destination, ex.Message); + } + + if (ratePerKg <= 0m) + throw new QuoteUnavailableException(destination, "rate provider returned a non-positive rate"); + + var basePrice = ratePerKg * billable; + var surcharged = basePrice + (basePrice * _surchargeTable.FuelSurchargeFor(destination)); + + return decimal.Round(surcharged + HandlingFee, 2); + } +} diff --git a/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/ShippingQuotes.csproj b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/ShippingQuotes.csproj new file mode 100644 index 0000000000..fbcf8b6668 --- /dev/null +++ b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/src/ShippingQuotes.csproj @@ -0,0 +1,9 @@ + + + net10.0 + enable + enable + false + ShippingQuotes + + diff --git a/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/BillableWeightTests.cs b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/BillableWeightTests.cs new file mode 100644 index 0000000000..e5bdf1195e --- /dev/null +++ b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/BillableWeightTests.cs @@ -0,0 +1,37 @@ +namespace ShippingQuotes.Tests; + +using Xunit; + +/// +/// Existing suite. It covers BillableWeight only. QuoteAsync has no tests yet. +/// +public class BillableWeightTests +{ + private static QuoteCalculator NewCalculator() => + new(new StubRateProvider(2m), new StubSurchargeTable(0m)); + + [Fact] + public void BillableWeight_UnderOneKilo_BillsTheOneKiloMinimum() => + Assert.Equal(1m, NewCalculator().BillableWeight(0.4m)); + + [Fact] + public void BillableWeight_AboveMinimum_BillsActualWeight() => + Assert.Equal(12.5m, NewCalculator().BillableWeight(12.5m)); + + [Theory] + [InlineData(0)] + [InlineData(-1)] + public void BillableWeight_ZeroOrNegative_Throws(int actualKg) => + Assert.Throws(() => NewCalculator().BillableWeight(actualKg)); + + private sealed class StubRateProvider(decimal rate) : IRateProvider + { + public Task GetRatePerKgAsync(string destination, CancellationToken cancellationToken) => + Task.FromResult(rate); + } + + private sealed class StubSurchargeTable(decimal surcharge) : ISurchargeTable + { + public decimal FuelSurchargeFor(string destination) => surcharge; + } +} diff --git a/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/ShippingQuotes.Tests.csproj b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/ShippingQuotes.Tests.csproj new file mode 100644 index 0000000000..eb233246ff --- /dev/null +++ b/tests/dotnet-test/code-testing-agent/fixtures/csharp-shipping-quotes/tests/ShippingQuotes.Tests.csproj @@ -0,0 +1,20 @@ + + + + net10.0 + enable + enable + false + Exe + true + + + + + + + + + + + diff --git a/tests/dotnet-test/coverage-analysis/fixtures/partial-coverage/coverage.cobertura.xml b/tests/dotnet-test/coverage-analysis/fixtures/partial-coverage/coverage.cobertura.xml index aae5f2e2f2..aef8e383d6 100644 --- a/tests/dotnet-test/coverage-analysis/fixtures/partial-coverage/coverage.cobertura.xml +++ b/tests/dotnet-test/coverage-analysis/fixtures/partial-coverage/coverage.cobertura.xml @@ -1,9 +1,9 @@ - - + + - + - + @@ -21,18 +21,18 @@ - - + + - + - - + + @@ -53,7 +53,7 @@ - + @@ -62,7 +62,7 @@ - + @@ -73,4 +73,4 @@ - + \ No newline at end of file diff --git a/tests/dotnet-test/crap-score/eval.yaml b/tests/dotnet-test/crap-score/eval.yaml index e608dddccb..51662559bf 100644 --- a/tests/dotnet-test/crap-score/eval.yaml +++ b/tests/dotnet-test/crap-score/eval.yaml @@ -73,3 +73,87 @@ stimuli: - Extracts per-method coverage from the generated Cobertura XML - Computes CRAP scores using both cyclomatic complexity and code coverage for each method in OrderService - Provides risk categorization and actionable recommendations for the riskiest methods + + - name: Recompute complexity instead of trusting a stale source comment + prompt: What is the CRAP score for ApplySurcharges in InvoiceEngine.cs? Coverage is already in + TestResults/coverage.cobertura.xml. + environment: + files: + - src: fixtures/refactor-required/InvoiceEngine.cs + dest: InvoiceEngine.cs + - src: fixtures/refactor-required/coverage.cobertura.xml + dest: TestResults/coverage.cobertura.xml + - src: fixtures/refactor-required/BillingTests.csproj + dest: BillingTests.csproj + graders: + - type: output-matches + config: + pattern: CRAP + - type: output-matches + config: + pattern: complexity|cyclomatic + - type: prompt + rubric: + - 'Counted the decision points in the current body of ApplySurcharges rather than reusing the stale + "Complexity: 4" comment above it' + - Reported a complexity in the low-to-mid teens, counting the && and || operators, the ?? and the two ternaries + as decision points — not 4 + - Used the coverage recorded for ApplySurcharges in the Cobertura XML (roughly half the lines covered), whether + read from line-rate or recomputed from the line hits — both agree + - Reported a CRAP score around 30 and classified it as high risk + - Stated the coverage level required to bring the method under a CRAP threshold of 15 + + - name: Recognize when complexity alone blocks the CRAP threshold + prompt: > + ClassifyAccount in InvoiceEngine.cs keeps getting flagged in review. We want every method under a + CRAP score of 15. How much more test coverage do we need to get there? Coverage data is in + TestResults/coverage.cobertura.xml. + environment: + files: + - src: fixtures/refactor-required/InvoiceEngine.cs + dest: InvoiceEngine.cs + - src: fixtures/refactor-required/coverage.cobertura.xml + dest: TestResults/coverage.cobertura.xml + - src: fixtures/refactor-required/BillingTests.csproj + dest: BillingTests.csproj + graders: + - type: output-matches + config: + pattern: CRAP + - type: output-matches + config: + pattern: (refactor|extract|reduce.*complexity|simplif) + - type: prompt + rubric: + - Computed a cyclomatic complexity for ClassifyAccount above 15 + - Stated directly that no amount of additional coverage can bring this method below a CRAP score of 15, + because a fully covered method's CRAP equals its complexity + - Did not invent a target coverage percentage that would supposedly reach the threshold + - Recommended reducing complexity (for example extracting the balance-tier branches) as the required fix + - Did not simply answer with "increase coverage to 100%" + + - name: Report a fully covered method at its complexity floor + prompt: Give me CRAP scores for every method in InvoiceEngine.cs using TestResults/coverage.cobertura.xml, sorted + by risk. + environment: + files: + - src: fixtures/refactor-required/InvoiceEngine.cs + dest: InvoiceEngine.cs + - src: fixtures/refactor-required/coverage.cobertura.xml + dest: TestResults/coverage.cobertura.xml + - src: fixtures/refactor-required/BillingTests.csproj + dest: BillingTests.csproj + graders: + - type: output-matches + config: + pattern: CRAP + - type: output-matches + config: + pattern: RoundToCurrency + - type: prompt + rubric: + - Analyzed all three methods (ApplySurcharges, ClassifyAccount, RoundToCurrency) rather than only the worst one + - Reported RoundToCurrency's CRAP score as exactly its complexity (3) because it is 100% covered + - Ranked ClassifyAccount as the highest risk, ahead of ApplySurcharges + - Did not recommend adding tests for RoundToCurrency, which is already fully covered + - Presented the results as a sorted table with complexity, coverage, and CRAP per method diff --git a/tests/dotnet-test/crap-score/fixtures/partial-coverage/coverage.cobertura.xml b/tests/dotnet-test/crap-score/fixtures/partial-coverage/coverage.cobertura.xml index a71d7ac5bc..65e1018dbc 100644 --- a/tests/dotnet-test/crap-score/fixtures/partial-coverage/coverage.cobertura.xml +++ b/tests/dotnet-test/crap-score/fixtures/partial-coverage/coverage.cobertura.xml @@ -1,65 +1,65 @@ - - + + - + - + - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + - + - - - - - - - + + + + + + + - + - - - - - - - - - + + + + + + + + + - - - - - + + + + + @@ -67,4 +67,4 @@ - + \ No newline at end of file diff --git a/tests/dotnet-test/crap-score/fixtures/refactor-required/BillingTests.csproj b/tests/dotnet-test/crap-score/fixtures/refactor-required/BillingTests.csproj new file mode 100644 index 0000000000..844af60f0e --- /dev/null +++ b/tests/dotnet-test/crap-score/fixtures/refactor-required/BillingTests.csproj @@ -0,0 +1,11 @@ + + + net10.0 + enable + enable + false + + + + + diff --git a/tests/dotnet-test/crap-score/fixtures/refactor-required/InvoiceEngine.cs b/tests/dotnet-test/crap-score/fixtures/refactor-required/InvoiceEngine.cs new file mode 100644 index 0000000000..b2a352368f --- /dev/null +++ b/tests/dotnet-test/crap-score/fixtures/refactor-required/InvoiceEngine.cs @@ -0,0 +1,109 @@ +namespace Billing; + +public class InvoiceEngine +{ + private readonly ITaxTable _taxTable; + + public InvoiceEngine(ITaxTable taxTable) + { + _taxTable = taxTable; + } + + // Complexity: 4 + // + // NOTE: this comment is stale. It dates from when the method only validated + // the invoice argument, and was never updated after the region, tier, + // expedite and floor rules below were added. + public decimal ApplySurcharges(Invoice invoice, string region, string tier) + { + if (invoice == null) + throw new ArgumentNullException(nameof(invoice)); + + decimal amount = invoice.Subtotal; + + if (region == "EU" || region == "UK") + amount += _taxTable.VatFor(region) * amount; + else if (region == "US") + amount += _taxTable.SalesTaxFor(invoice.StateCode ?? "") * amount; + + if (tier == "gold" && invoice.Subtotal > 1000m) + amount -= amount * 0.10m; + else if (tier == "silver" && invoice.Subtotal > 500m) + amount -= amount * 0.05m; + + if (invoice.IsExpedited) + amount += invoice.Weight > 20m ? 45m : 20m; + + var floor = invoice.MinimumCharge ?? 0m; + return amount < floor ? floor : Math.Round(amount, 2); + } + + // Branch-heavy classifier: complexity here is high enough that testing + // alone cannot pull the CRAP score under the usual threshold of 15. + public string ClassifyAccount(Account account) + { + if (account == null) + return "unknown"; + + if (account.Balance < 0) + { + if (account.DaysOverdue > 90) + return account.IsCorporate ? "write-off" : "collections"; + if (account.DaysOverdue > 30) + return "delinquent"; + return "arrears"; + } + + if (account.Balance == 0) + return account.IsDormant ? "dormant" : "settled"; + + if (account.Balance > 100000m && account.IsCorporate) + return "strategic"; + + if (account.Balance > 50000m || account.LifetimeValue > 250000m) + return "premium"; + + if (account.IsCorporate && account.EmployeeCount > 500) + return "enterprise"; + + if (account.YearsActive > 10 && account.MissedPayments == 0) + return "loyal"; + + return account.IsCorporate ? "standard-corporate" : "standard"; + } + + public decimal RoundToCurrency(decimal amount, int places) + { + if (places < 0) + throw new ArgumentOutOfRangeException(nameof(places)); + + return places == 0 ? Math.Round(amount) : Math.Round(amount, places); + } +} + +public class Invoice +{ + public decimal Subtotal { get; set; } + public string? StateCode { get; set; } + public bool IsExpedited { get; set; } + public decimal Weight { get; set; } + public decimal? MinimumCharge { get; set; } +} + +public class Account +{ + public decimal Balance { get; set; } + public int DaysOverdue { get; set; } + public bool IsCorporate { get; set; } + public bool IsDormant { get; set; } + public decimal LifetimeValue { get; set; } + public int EmployeeCount { get; set; } + public int YearsActive { get; set; } + public int MissedPayments { get; set; } +} + +public interface ITaxTable +{ + decimal VatFor(string region); + decimal SalesTaxFor(string stateCode); +} diff --git a/tests/dotnet-test/crap-score/fixtures/refactor-required/coverage.cobertura.xml b/tests/dotnet-test/crap-score/fixtures/refactor-required/coverage.cobertura.xml new file mode 100644 index 0000000000..5a92b829d1 --- /dev/null +++ b/tests/dotnet-test/crap-score/fixtures/refactor-required/coverage.cobertura.xml @@ -0,0 +1,114 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/dotnet-test/find-untested-sources/eval.yaml b/tests/dotnet-test/find-untested-sources/eval.yaml index 1e178e667a..1904cebe28 100644 --- a/tests/dotnet-test/find-untested-sources/eval.yaml +++ b/tests/dotnet-test/find-untested-sources/eval.yaml @@ -131,3 +131,28 @@ stimuli: - Correctly identified Beta/Advanced/Options.cs as unpaired despite its duplicate short type name - Suggested NamespaceCollision/tests/NamespaceCollision.Tests/Beta/Advanced/OptionsTests.cs - Clearly distinguished static source-to-test evidence from actual line or branch coverage + + - name: Pair sources to tests across a src/tests directory split + prompt: > + Which classes in this repository don't have unit tests yet? The sources + live under src/ and the tests under tests/. + environment: + files: + - src: fixtures/pairing-repo + dest: . + graders: + - type: output-matches + config: + pattern: OrderProcessor + - type: output-not-matches + config: + pattern: (?im)^\s*(?:[$>]\s*)?dotnet\s+(?:build|test)\b + - type: prompt + rubric: + - Identified OrderProcessor.cs as having no corresponding test file + - Recognised CustomerService.cs as already paired with CustomerServiceTests.cs, and did not report it as + untested + - Resolved the pairing across the src/ and tests/ directory split rather than only looking for a sibling file + - Suggested a concrete test path following the repository's existing tests/Store.Tests/ convention + - Did not run dotnet build or dotnet test — this is static pairing analysis + - Did not claim a line-coverage percentage; source-to-test pairing is not coverage diff --git a/tests/dotnet-test/migrate-static-to-wrapper/eval.yaml b/tests/dotnet-test/migrate-static-to-wrapper/eval.yaml index 8b406288c3..008326cba8 100644 --- a/tests/dotnet-test/migrate-static-to-wrapper/eval.yaml +++ b/tests/dotnet-test/migrate-static-to-wrapper/eval.yaml @@ -56,7 +56,8 @@ stimuli: - Migrated DateTime.UtcNow to TimeProvider in CouponService.cs - Left AuditLogger.cs completely untouched with DateTime.UtcNow calls still present - Respected the scope boundary the user specified - - Reported what was migrated and what remains to be done + - Told the user which file was migrated and which was deliberately left alone, so the remaining work is not + silently dropped - name: Decline migration when wrapper does not exist yet prompt: > @@ -77,3 +78,61 @@ stimuli: - Identified that System.IO.Abstractions NuGet package is not referenced in the project - Did not blindly replace File.ReadAllText with IFileSystem calls that would not compile - Recommended installing the package and registering IFileSystem before migrating + + - name: Preserve DateTimeKind when migrating to TimeProvider + prompt: > + Migrate the DateTime.UtcNow calls in CouponService.cs to the TimeProvider that is + already registered in DI. This is a pure refactor — the values stored on Coupon must + behave exactly as they do today. + environment: + files: + - src: ./fixtures/ready-to-migrate + dest: ReadyToMigrate + graders: + - type: file-contains + config: + path: "**/CouponService.cs" + value: UtcDateTime + - type: file-not-contains + config: + path: "**/CouponService.cs" + value: DateTime.UtcNow + - type: exit-success + - type: prompt + rubric: + - Converted the DateTimeOffset from GetUtcNow() back to DateTime in a way that leaves the stored values with + the same DateTimeKind they had before (Utc), so no consumer observes a behavior change + - Did not use the plain .DateTime property, which would silently downgrade the Kind to Unspecified + - Left CreatedAt and ExpiresAt typed as DateTime instead of widening them to DateTimeOffset + - Kept the behavior of CreateCoupon, IsValid and TimeUntilExpiry identical apart from the time source + - Restricted the change to the DateTime.UtcNow call sites the user named, leaving the intentional local-time + call untouched + + - name: Migrate a static helper class without breaking its callers + prompt: > + RetentionPolicy is a static class that calls DateTime.UtcNow in three places, which makes + it impossible to unit test. Make it testable using TimeProvider. About forty call sites + still call RetentionPolicy.IsExpired(...) and similar directly, and I cannot update them + in this change. + environment: + files: + - src: ./fixtures/static-helper + dest: StaticHelper + graders: + - type: file-contains + config: + path: "**/RetentionPolicy.cs" + value: TimeProvider + - type: file-contains + config: + path: "**/RetentionPolicy.cs" + value: static + - type: exit-success + - type: prompt + rubric: + - Kept RetentionPolicy a static class and kept every method callable as RetentionPolicy.Method(...) + - Made the time source swappable on RetentionPolicy itself, so a test can substitute a fake clock and assert + against the static API directly without routing through a new type + - Defaulted the seam to TimeProvider.System so existing callers behave exactly as before + - Replaced all three DateTime.UtcNow call sites with the seam + - Did not convert the static class into an instance class or change any public method signature diff --git a/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/StaticHelper.csproj b/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/StaticHelper.csproj new file mode 100644 index 0000000000..1a57862983 --- /dev/null +++ b/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/StaticHelper.csproj @@ -0,0 +1,7 @@ + + + net10.0 + enable + enable + + diff --git a/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/Utilities/RetentionPolicy.cs b/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/Utilities/RetentionPolicy.cs new file mode 100644 index 0000000000..0d2e486cff --- /dev/null +++ b/tests/dotnet-test/migrate-static-to-wrapper/fixtures/static-helper/Utilities/RetentionPolicy.cs @@ -0,0 +1,27 @@ +namespace StaticHelper.Utilities; + +/// +/// Called from ~40 call sites across the codebase as RetentionPolicy.IsExpired(...). +/// Callers must keep working unchanged. +/// +public static class RetentionPolicy +{ + public static bool IsExpired(DateTime createdAtUtc, int retentionDays) + { + return DateTime.UtcNow > createdAtUtc.AddDays(retentionDays); + } + + public static DateTime NextPurgeWindow() + { + var now = DateTime.UtcNow; + return now.Date.AddDays(1).AddHours(2); + } + + public static string DescribeAge(DateTime createdAtUtc) + { + var age = DateTime.UtcNow - createdAtUtc; + return age.TotalDays >= 1 + ? $"{(int)age.TotalDays} day(s) old" + : $"{(int)age.TotalHours} hour(s) old"; + } +} diff --git a/tests/dotnet-test/test-anti-patterns/eval.yaml b/tests/dotnet-test/test-anti-patterns/eval.yaml index 9829f2284d..b6c682d681 100644 --- a/tests/dotnet-test/test-anti-patterns/eval.yaml +++ b/tests/dotnet-test/test-anti-patterns/eval.yaml @@ -273,3 +273,67 @@ stimuli: - bash - edit - create + + - name: Audit a pytest suite using Python-specific anti-pattern markers + prompt: > + Review the tests in this Python project and tell me what's wrong with them. Rank what you + find by how badly it hurts us. + environment: + files: + - src: fixtures/pytest-mixed/pyproject.toml + dest: pyproject.toml + - src: fixtures/pytest-mixed/src/user_service.py + dest: src/user_service.py + - src: fixtures/pytest-mixed/tests/test_user_service.py + dest: tests/test_user_service.py + graders: + - type: output-matches + config: + pattern: (Critical|High|Medium|Low|severity) + - type: output-matches + config: + pattern: (test_add_user_runs_without_error|no assertion|assertion-free|without.*assert) + - type: output-matches + config: + pattern: (sleep|time\.sleep|flak) + - type: prompt + rubric: + - Flagged test_add_user_runs_without_error as having no assertion at all + - Flagged test_get_missing_swallows_exception for swallowing the exception so the test passes either way + - Flagged test_delete_compares_count_to_itself as a tautological self-comparison + - Flagged the wall-clock time.sleep and the un-injected datetime.datetime.now as flakiness sources + - Flagged the shared _SHARED_SERVICE module-level state that makes the two ordering tests order-dependent + - Flagged the four near-identical test_repeated_value_assertion_* tests as duplicates that should be + parametrized with @pytest.mark.parametrize + - Flagged pytest.raises(Exception) as too broad and recommended the specific exception type + - Did NOT flag Python's bare assert statements as a missing assertion library — bare assert is canonical in + pytest + - Acknowledged test_get_user_returns_correct_name as a well-written test + - Used pytest terminology and Python APIs throughout rather than describing the suite in MSTest terms + + - name: Separate false-confidence assertions from cosmetic ones + prompt: My CalculatorTests suite is green, but I don't trust it. What's actually wrong, and what should I fix first? + environment: + files: + - src: fixtures/assertion-problems/Calculator.cs + dest: Calculator.cs + - src: fixtures/assertion-problems/CalculatorTests.cs + dest: CalculatorTests.cs + graders: + - type: output-matches + config: + pattern: (Critical|High|Medium|Low|severity) + - type: output-matches + config: + pattern: (Assert\.IsTrue\(true\)|always.true|tautolog|proves nothing) + - type: prompt + rubric: + - Flagged Add_TwoNumbers_ReturnsSum as having no assertion, only a TODO comment + - Flagged Assert.IsTrue(true) in Subtract_TwoNumbers_ReturnsDifference as an always-true assertion + - Flagged Assert.AreEqual(result, result) in Divide_TwoNumbers_ReturnsQuotient as a self-comparison + - Flagged Divide_ByZero_ThrowsException for swallowing the exception in a catch block instead of using + Assert.ThrowsException, so it passes even when nothing is thrown + - Rated the assertion message "Expected and actual are not equal" in Add_Negative_Works as a Low-severity + issue, clearly separated from the Critical false-confidence problems + - Recommended fixing the tests that can never fail before the cosmetic message issue + - Gave a concrete replacement assertion with the exact expected value for each broken test diff --git a/tests/dotnet-test/test-gap-analysis/eval.yaml b/tests/dotnet-test/test-gap-analysis/eval.yaml index e34ca71105..cd81027f21 100644 --- a/tests/dotnet-test/test-gap-analysis/eval.yaml +++ b/tests/dotnet-test/test-gap-analysis/eval.yaml @@ -79,6 +79,73 @@ stimuli: - Identified that ElevateRole is not tested with null or empty tokens — removing the null guard would survive - Identified that Editor on system resources getting read-only (CanWrite=false) is not tested - Recommended concrete test cases targeting the specific survived mutations + + - name: Skip trivial and generated code while tracing private call chains + prompt: > + Are the tests for InvoiceProcessor strong enough to catch a subtle bug? I care about the + money math. Give me the mutation points that matter and skip anything that isn't worth + analysing. + environment: + files: + - src: fixtures/report-quality/Billing/Billing.csproj + dest: Billing/Billing.csproj + - src: fixtures/report-quality/Billing/InvoiceProcessor.cs + dest: Billing/InvoiceProcessor.cs + - src: fixtures/report-quality/Billing/InvoiceProcessor.g.cs + dest: Billing/InvoiceProcessor.g.cs + - src: fixtures/report-quality/Billing.Tests/Billing.Tests.csproj + dest: Billing.Tests/Billing.Tests.csproj + - src: fixtures/report-quality/Billing.Tests/InvoiceProcessorTests.cs + dest: Billing.Tests/InvoiceProcessorTests.cs + graders: + - type: output-matches + config: + pattern: (ApplyLateFee|late.fee|30|0\.10|0\.05) + - type: output-matches + config: + pattern: (surviv|not.*caught|not.*detected|gap|miss|blind|weak) + - type: prompt + rubric: + - Analysed the private ApplyLateFee and ComputeTax helpers by tracing the call chain from the public + ComputeAmountDue, rather than skipping them because they are private + - Identified that the late-fee tier boundary (30 days, 5% vs 10%) is never pinned by any test, so changing + those tiers would survive + - Identified that the tax-exempt path is never exercised + - Explained that Assert.IsTrue(result > 100m) is too weak to kill arithmetic mutations because almost any + wrong amount still satisfies it + - Excluded the auto-generated InvoiceProcessor.g.cs from the analysis, or explicitly noted it is generated + and out of scope + - Did not report the trivial auto-properties (CustomerName, InvoiceId) or the IsPaid/FormatReceipt + one-liners as meaningful gaps + + - name: Analyse error propagation gaps in a Rust crate + prompt: > + This Rust crate has a small test module. Would those tests actually catch a bug if someone + changed the parsing or the threshold check? + environment: + files: + - src: fixtures/rust-error-propagation/Cargo.toml + dest: Cargo.toml + - src: fixtures/rust-error-propagation/src/lib.rs + dest: src/lib.rs + graders: + - type: output-matches + config: + pattern: (\?|propagat|Err|error path|ParseIntError) + - type: output-matches + config: + pattern: (surviv|not.*caught|not.*detected|gap|miss|blind|weak) + - type: prompt + rubric: + - Identified that no test exercises the error path of parse_line_total, so the `?` propagation is never + observed and replacing it with unwrap/expect would go undetected by the suite + - Identified the `<=` comparison in first_below_threshold as an untested boundary — no test pins the + at-threshold case against the below-threshold case + - Used Rust-appropriate mutation reasoning (Result/Option variants, `?` short-circuit) rather than + describing the code in .NET terms + - Recommended concrete Rust test cases, such as asserting an Err for a non-numeric field and a case where + the only matching level equals the threshold + - Did not claim the crate is untested overall — it acknowledged the two passing tests as real coverage - name: Acknowledge well-tested code with few surviving mutations prompt: | Can you check if my inventory management tests would actually catch @@ -111,6 +178,13 @@ stimuli: I need to write unit tests for my ShoppingCart class. It handles adding items, removing items, calculating totals, and applying promo codes. Can you write a complete MSTest test suite for me? + # Off-target request: this skill analyses gaps in existing tests, it does not + # author suites. The skill IS loaded here on purpose — the property under + # test is that it stays dormant and does not hijack the request. Do not add + # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it + # identical to the baseline arm, so the head-to-head score becomes pure + # judge noise rather than a measurement. + expect_activation: false environment: files: - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs @@ -123,6 +197,6 @@ stimuli: rubric: - Wrote test methods for the ShoppingCart class - Covered the AddItem and GetTotal methods - constraints: - reject_skills: - - "*" + - Did not derail into a mutation/test-gap analysis of code the user never asked about + - Judged on whether the requested behaviours are actually covered, not on which response happened to emit more + test methods diff --git a/tests/dotnet-test/test-smell-detection/eval.yaml b/tests/dotnet-test/test-smell-detection/eval.yaml index 66bf95a750..174846eefb 100644 --- a/tests/dotnet-test/test-smell-detection/eval.yaml +++ b/tests/dotnet-test/test-smell-detection/eval.yaml @@ -103,6 +103,13 @@ stimuli: 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? + # Off-target request: this skill audits existing tests, it does not author + # suites. The skill IS loaded here on purpose — the property under test is + # that it stays dormant and does not hijack the request. Do not add + # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it + # identical to the baseline arm, so the head-to-head score becomes pure + # judge noise (it cost this skill a pass at -40% in run 30247614857). + expect_activation: false environment: files: - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs @@ -115,6 +122,71 @@ stimuli: rubric: - Wrote test methods for the ShoppingCart class - Covered multiple methods of the class - constraints: - reject_skills: - - "*" + - Did not derail into a test-smell audit of code the user never asked about + - Judged on whether the requested behaviours are actually covered, not on which response happened to emit more + test methods + + - name: Audit a JUnit suite using Java-specific smell markers + prompt: > + Review the tests in this Java project and tell me which test smells they + have, with severity. I want to know what to fix first. + environment: + files: + - src: fixtures/junit-smells/pom.xml + dest: pom.xml + - src: fixtures/junit-smells/src/main/java/com/example/Catalog.java + dest: src/main/java/com/example/Catalog.java + - src: fixtures/junit-smells/src/test/java/com/example/CatalogTest.java + dest: src/test/java/com/example/CatalogTest.java + graders: + - type: output-matches + config: + pattern: (High|Medium|Low|severity) + - type: output-matches + config: + pattern: (addAndDoNothing|assertion.free|no assertion|Unknown Test) + - type: output-matches + config: + pattern: (sleep|Sleepy) + - type: prompt + rubric: + - Flagged addAndDoNothing as an assertion-free test that verifies nothing + - Flagged the Thread.sleep in addSleepy as a Sleepy Test + - Flagged the branching inside addConditionalLogic as Conditional Test Logic + - Flagged addLoadsFromFile as a Mystery Guest for depending on the file system + - Flagged doEverythingAtOnce as an Eager Test that exercises many distinct methods + - Flagged the unexplained 42 in quantityAfterAdds as a Magic Number Test + - Flagged toStringFormat as Sensitive Equality for depending on the toString format + - Flagged addNullSkuManualCatch for hand-rolling try/catch instead of using assertThrows + - Flagged the @Disabled test as an Ignored Test + - Recognised addThrowsOnNegativeCount as a well-written test rather than condemning the whole file + - Used JUnit terminology and Java APIs throughout rather than describing the suite in MSTest terms + + - name: Calibrate skips and contextually obvious numbers + prompt: > + Audit these xUnit tests for smells. I care about real problems — please + don't flag things that are actually fine. + environment: + files: + - src: fixtures/skip-and-magic/Inventory.Tests/Inventory.Tests.csproj + dest: Inventory.Tests/Inventory.Tests.csproj + - src: fixtures/skip-and-magic/Inventory.Tests/InventoryServiceTests.cs + dest: Inventory.Tests/InventoryServiceTests.cs + graders: + - type: output-matches + config: + pattern: (Skip|skipped|Ignored|disabled) + - type: output-matches + config: + pattern: (sleep|Sleepy|Audit_RunsWithoutError|assertion.free|no assertion) + - type: prompt + rubric: + - Flagged the Thread.Sleep(3000) in Replenish_AsyncJob_Completes as a real Sleepy Test + - Flagged Audit_RunsWithoutError as an assertion-free test + - Flagged the bare Skip = "skip" on Restock_FromSupplier_UpdatesQuantities as an Ignored Test with no + justification + - Distinguished the documented skip on Reserve_AcrossWarehouses_BalancesStock, which cites a tracking issue and + a reason, as materially less concerning than the bare one + - Did NOT flag the counts in AddItems_ThreeAdded_CountIsThree or RemoveItem_FromTwo_LeavesOne as magic numbers — + adding three items and asserting three is self-documenting + - Kept the report proportionate instead of inflating severity to justify the review diff --git a/tests/dotnet-test/test-tagging/eval.yaml b/tests/dotnet-test/test-tagging/eval.yaml index fc85f79fce..d8314ee4c1 100644 --- a/tests/dotnet-test/test-tagging/eval.yaml +++ b/tests/dotnet-test/test-tagging/eval.yaml @@ -145,6 +145,13 @@ stimuli: - name: Decline request to write new tests prompt: I need to write unit tests for my PaymentGateway class. It handles credit card charges, refunds, and webhook processing. Can you help me write comprehensive MSTest tests? + # Off-target request: this skill tags existing tests, it does not author + # suites. The skill IS loaded here on purpose — the property under test is + # that it stays dormant and does not hijack the request. Do not add + # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it + # identical to the baseline arm, so the head-to-head score becomes pure + # judge noise rather than a measurement. + expect_activation: false environment: files: - src: ./fixtures/decline-request-to-write-new-tests/PaymentGateway.cs @@ -157,9 +164,9 @@ stimuli: rubric: - Wrote test methods for the PaymentGateway class - Covered both success and failure scenarios - constraints: - reject_skills: - - "*" + - Did not derail into tagging or trait-auditing work the user never asked about + - Judged on whether the requested behaviours are actually covered, not on which response happened to emit more + test methods - name: Tag a partially-tagged MSTest suite without duplicating existing traits prompt: My MSTest project at OrderService.Tests/ already has some tests tagged with [TestCategory] but most are still untagged. Can you fill in the missing category attributes for the untagged tests? Make sure you don't duplicate @@ -277,3 +284,37 @@ stimuli: changes - Used only valid trait values from the taxonomy — no invented category names that would cause logical errors - Every test method received at least one positive or negative tag + + # Go has no attribute-based trait mechanism, so the skill must classify and + # report rather than reach for a [TestCategory] equivalent that does not exist. + - name: Classify Go tests where no trait attribute mechanism exists + prompt: > + Here is a small Go package with tests. I'd like to know how the suite + breaks down between happy-path and error-path coverage. Don't change any + of my files — just tell me. + environment: + files: + - src: fixtures/go-report-only/calculator/go.mod + dest: calculator/go.mod + - src: fixtures/go-report-only/calculator/calculator.go + dest: calculator/calculator.go + - src: fixtures/go-report-only/calculator/calculator_test.go + dest: calculator/calculator_test.go + graders: + - type: output-matches + config: + pattern: (TestDivide_ByZero_ReturnsError|by.?zero|error path|negative) + - type: run-command + config: + command: sh -c "git diff --quiet -- calculator/calculator_test.go || test ! -d .git" + expected_exit_code: 0 + timeout: 1m + - type: prompt + rubric: + - Classified TestDivide_ByZero_ReturnsError as the error/negative-path test + - Classified the remaining three tests as happy-path/positive coverage + - Reported the distribution between the two groups + - Did not modify calculator_test.go — the user asked for analysis only + - Did not invent a C#-style attribute such as [TestCategory] or [Trait]; recognised that Go expresses this + through naming, build tags, or testing.Short rather than attributes, or reported without proposing a + mechanism at all