Files
dotnet__skills/plugins/dotnet-test-migration/agents
Amaury Levé 030493de5a Fix migrate-mstest-v1v2-to-v3 activation and test-skill eval quality (#974)
* Fix migrate-mstest-v1v2-to-v3 skill activation

The frontmatter description said DO NOT USE FOR: ... projects already on
MSTest v3+, which blocked the skill on every scenario where the packages had
already been bumped to 3.x and only the source or settings still needed the
v1/v2-to-v3 fixes (Assert object overloads, DataRow strict typing,
.testsettings -> .runsettings). It also gated the whole skill behind "the user
asks to upgrade MSTest", so a standalone .testsettings conversion never matched.

- Rewrite the description around both entry points (pre-upgrade migration and
  post-upgrade breaking-change fixes) and add the concrete trigger keywords
  those prompts contain: CS1501/CS1503/CS0121, MSTEST0014, LegacySettings,
  DeploymentEnabled, per-test TestTimeout, net5.0. Note that the current runner
  is preserved so "migrate to v3 but keep VSTest" isn't poached by
  migrate-vstest-to-mtp.
- Narrow migrate-mstest-v3-to-v4, which claimed the generic "tests don't
  compile after upgrading MSTest" phrasing and competed for the same prompts.
- Widen the Boundary Gate: a 3.x package version alone no longer ends the
  migration when v1/v2-era settings or errors remain, so the skill actually
  performs the requested edits instead of reporting "already migrated".
- Add a routing row to the test-migration agent for the same case.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

* Clear the eval-quality gate's test-skill findings

The gate reported three classes of debt against the dotnet-test and
dotnet-test-migration plugins. All three are addressed here; the four ERRORS it
also reports are dotnet-maui allowlist lines and are untouched.

Underpowered evals (5). Below five trials the pass gate's sign test cannot reach
p <= 0.05 at any effect size, so these five evals could never return a verdict.
Each is now at or above the floor and its allowlist line is deleted in the same
change, as the ledger's shrink-only rule requires:

  - coverage-analysis 3 -> 5: adds a refactoring-safety question (the "is this
    safe to change?" use case named in the skill's Purpose but never exercised)
    and a branch-vs-line coverage question. Both reuse the existing
    partial-coverage fixture.
  - find-untested-sources 4 -> 5: adds a mixed C#/TypeScript repository, which
    is the only case that exercises the documented engine choice - polyglot
    tree-sitter rather than the C#-only Roslyn engine. Composed from the two
    existing fixtures.
  - generate-testability-wrappers 4 -> 5: adds the ambient-context path (Step 5)
    for a project with no DI container, where AsyncLocal<T> and scoped disposal
    are the distinguishing content.
  - grade-tests 4 -> 5: adds a C# case with the production code present. Every
    prior C# scenario hides it, so "Unverified" was never tested as a negative,
    and the D band and the swallowed-exception F were never graded at all. New
    production-available fixture.
  - code-testing-agent 3 -> 6 via defaults.runs=2. Scenarios are preferred over
    runs, but each of these drives a full generate-build-test pipeline (npm ci
    plus two Vitest runs, pip install plus pytest, a dotnet test build) under a
    60m budget, which is the documented case for buying trials with runs.

Orphaned fixtures (5). v3-sealed-timeout, mtp-mstest-sdk9, mtp-mstest-sdk10,
mtp-mstest-hotreload-installed and vstest-mstest are all superseded first-
generation copies: their per-scenario successors differ only in whitespace, a
dropped rollForward, or a package version. Both evals are already well above the
floor, so wiring them up would add no power. Deleted.

Skills with no eval (2 of 4). platform-detection and filter-syntax carry real
checkable rules that nothing measured, and several are counterintuitive enough
that a baseline is likely to get them wrong - global.json test.runner outranking
TestingPlatformDotnetTestSupport on .NET 10+, Microsoft.NET.Test.Sdk not being a
VSTest signal, MTP properties living in Directory.Build.props, xUnit v3 dropping
VSTest --filter while MSTest on MTP keeps it. Both get a 5-scenario eval with
small fixtures and no build step. code-testing-extensions and
test-analysis-extensions are left flagged on purpose: their bodies are tables of
paths to extension files, so a head-to-head eval would score path recall rather
than user value. The content those files hold is already exercised through
code-testing-agent's three-language pipeline.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

* Fix the two skill defects behind the v1v2-to-v3 eval losses

The first eval run reached 7W/2T/2L, p=0.090, short of the p<=0.05 gate. Both
losses trace to skill content that actively misled the agent, and the session
transcripts show exactly how.

Loss 1 -- 'Migrate MSTest v1 project with assembly reference', skilled scored
0.00 against a 4.17 baseline. The transcript shows the skill loading correctly
and the agent then replying, in full: 'To give you specific migration steps, I
need to see your project file. Could you share the path to your .csproj?' The
project was already in the working directory. Cause: the Inputs table marked
'Project or solution path' as Required=Yes, which reads as a precondition the
agent must obtain before doing anything. This is the worst kind of failure for a
real user - they describe their project in prose and get a question back instead
of an answer. Path is now optional and discovered by globbing, Step 1 leads with
locating the project, and a note forbids opening with a request for the path.
The same Required=Yes trap was present in migrate-mstest-v3-to-v4 and
migrate-vstest-to-mtp, so both are corrected too.

Loss 2 -- 'Fix DataRow type mismatch errors', skilled 3.96 against a 5.00
baseline. The skill's breaking-change table said the 16-argument DataRow cap was
'fixed in later v3 versions' and suggested 'refactor test / wrap extra params in
array'. On a project already at MSTest 3.8, the agent concluded the valid
17-argument row exceeded the limit and rewrote it - first as new object[] { 17 },
which failed, then second-guessing itself mid-run ('let me check if the latest
3.x actually fixed the 16-arg limit'), finally settling on a (object)17 cast.
Churn plus wasted turns on code that was already correct.

The vague wording was the problem, so it is replaced with the fact: the cap was
introduced in 3.0.1 and removed again in 3.0.3 (microsoft/testfx#1554 and the
maintainer's 'please feel free to update to 3.0.3'). On 3.0.3+ a longer row is
valid and must be left alone. A general guideline is added alongside it -
confirm the diagnostic before editing, because rewriting valid code to dodge a
limit the project is not subject to is a defect rather than caution.

Both fixes are about what the skill tells a real user, not about the graders;
no eval prompt, fixture, or grader is touched. Skill grows ~480 tokens and stays
in the 'standard' tier, below the 5,000-token warning threshold.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

* Correct the Assert/DataRow facts and make the fixtures reproduce them

The two eval runs on this PR compared byte-identical skill content: run 1
(f9dc25d) and run 2 (2fc8ab8) differ in tests/ and eng/ only, git diff on
plugins/ between them is empty, and the run-2 artifact confirms the loaded
SKILL.md lacks the fix from 6b11ad9. So 7W/2T/2L -> 4W/5T/2L is judge noise, not
a regression. The v1-assembly-ref scenario scored 0.00 in BOTH runs with the same
'I need to see your project file' reply, which 6b11ad9 addresses.

Investigating the remaining scenarios against a real MSTest 3.8 project turned up
something worse than a scoring problem: the skill was teaching two things that
are not true, and one eval fixture could not reproduce the bug it was named for.

1. Assert. The skill said the removal of Assert.AreEqual(object, object) causes
   'compile error on untyped assertions'. It does not. MSTest v3 keeps
   AreEqual<T>(T?, T?), so two object-typed arguments infer T = object and
   compile untouched; verified by building the shipped fixture, which passes 3/3
   tests unmodified. The break happens only where T cannot be inferred, and the
   real diagnostics are CS0411 and CS1503 - not the CS1501/CS0121 the earlier
   description claimed. Following the old text, an agent rewrites assertions that
   were already correct, which is the same over-application defect as the DataRow
   one. Table, Step 5 and the description now state the real trigger and codes,
   and say to fix only the call sites the compiler rejects.

2. DataRow. The skill implied compile errors. Verified: a mismatched row builds
   with analyzer warning MSTEST0014 and fails at run time with 'Test data doesn't
   match method parameters'. Widening (int -> long) still binds; narrowing does
   not. Stated explicitly, because a green build is exactly what misleads here.

3. Fixtures. fix-assert-.../ComparisonTests.cs compiled and passed as shipped, so
   its scenario could never discriminate - it scored baseline 5.00/5.00 in both
   runs. It now uses two unrelated interface-typed views of one instance, which
   genuinely fails with CS0411 on all three assertions and passes 4/4 once the
   <object> argument is added. It also gains two already-valid typed assertions
   that must be left alone; widening them still compiles, so only judgement
   prevents it, and two graders now check that.

   v2-nuget/UserServiceTests.cs and v2-complex/InventoryServiceTests.cs had the
   same problem: their graders demanded Assert.AreEqual<object> on assertions
   that never needed it, which now directly contradicts the corrected skill.
   Both fixtures were rebuilt the same way and verified in three states - build
   clean on MSTest 2.2.10 as shipped, fail with CS0411 after the v3 bump, pass
   (5/5 and 7/7) once migrated.

Prompts for the two affected scenarios now describe the real symptoms (CS0411,
and 'builds but fails at run time') instead of the invented CS1501 and 'no longer
compile'. Every claim above was verified by building and running against MSTest
3.8 and 2.2.10 rather than inferred.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

* Stop the complex-migration run from scaffolding a substitute project

Last eval: 6W/4T/1L, p=0.063. The three scenarios fixed in 6b11ad9 and 22103d2
all flipped to wins; one loss remains and the transcript shows a distinct bug.

The agent globbed correctly and got back ./TestProject.csproj,
./InventoryServiceTests.cs and ./local.testsettings. It then rebuilt those into
absolute paths under the skill's own base directory, all three reads failed with
'Path does not exist', it globbed that directory, found only SKILL.md, and
concluded 'There's no actual project on disk'. It then scaffolded a substitute
project from the prose description - fewer tests, no 17-parameter row, and a
self-introduced bug it had to debug. Judge scored it 0.23 against a 3.07
baseline.

Step 1 told it to glob but not what to do with the answer, so add that: open
paths exactly as the search returned them, treat a failed read of a just-found
file as a wrong constructed path, never conclude the project is missing while a
search is still listing it, and never scaffold a replacement. This matters
outside the eval too - a skill loaded from a plugin directory always has a base
directory that is not the user's repo.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

* Address Copilot review: fixture versions and an over-broad grader

Three findings from the latest Copilot review, all valid.

Two package versions in the platform-detection fixtures were invented rather
than copied from the repo: TUnit 0.6.0 and xunit.v3 1.0.0. Both existed nowhere
else in tests/ - the canonical versions are 1.45.8 and 1.0.1 - so they risked a
restore failure the moment anything builds these fixtures. Aligned to the
versions the rest of the repo uses.

The third is a grader I added in the merge commit. output-not-matches:
ThreadStatic forbids the substring anywhere in the response, so it would also
fail a correct answer that warns the user against [ThreadStatic] - which is
exactly the answer the rubric asks for, and the likeliest way a good response
mentions it. No regex separates 'recommends the attribute' from 'warns against
it' reliably, so the grader is removed and the rubric line kept: a judge can
draw that distinction, a substring match cannot.

That leaves main's original grader set for this scenario untouched, plus the one
rubric item this branch contributed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ad6ff32a-d441-4a7b-b474-2bfaee764740
2026-07-31 16:27:36 +02:00
..