Skip to content

Add migrate-xunit-to-mstest skill#718

Open
Evangelink wants to merge 8 commits into
mainfrom
dev/amauryleve/migrate-xunit-to-mstest
Open

Add migrate-xunit-to-mstest skill#718
Evangelink wants to merge 8 commits into
mainfrom
dev/amauryleve/migrate-xunit-to-mstest

Conversation

@Evangelink
Copy link
Copy Markdown
Member

@Evangelink Evangelink commented Jun 3, 2026

Summary

Adds the migrate-xunit-to-mstest skill, which migrates .NET test projects from xUnit.net (v2 or v3) to MSTest v4 — attributes, assertions, fixtures, lifecycle, output, cancellation, parallelization, and companion packages.

Single skill (not two) for both v2 and v3 sources: ~90% of the conversion logic is shared, and v2-vs-v3 deltas are small and well-scoped — handled as conditional steps. Matches the precedent of migrate-mstest-v1v2-to-v3.

Why parallelization gets front-loaded

xUnit parallelizes test classes by default; MSTest serializes by default. This is the single largest source of post-migration regressions: tests that depend on isolation by parallel scheduling, on the lack of it, or on shared static state can pass differently after the switch.

Step 1 inventories shared state and flaky tests. Step 11 enumerates three explicit choices and provides a translation table for CollectionBehavior(DisableTestParallelization), MaxParallelThreads, [Collection], and [CollectionDefinition]. The validation checklist treats unspecified parallelization as an error.

Design constraints (validated with rubber-duck review)

  • Preserves the current test platform (VSTest stays on VSTest; MTP stays on MTP). Bundling a platform migration would violate the test-migration agent's "never mix migration steps" rule.
  • IClassFixture<T> -> mechanical map to [ClassInitialize] / [ClassCleanup].
  • ICollectionFixture<T> requires judgement — it conflates "shared instance across classes" with "serial execution". MSTest decouples them; do not silently widen scope to assembly-level.
  • Exception assertion semantic trap — xUnit Assert.Throws<T> is exact-type and maps to Assert.ThrowsExactly<T>; xUnit Assert.ThrowsAny<T> maps to Assert.Throws<T>. The names invert.
  • MSTest v4 TFM gating — supported: net8.0, net9.0, net462+, netstandard2.0 (test library only), UAP/WinUI. Stop for net5.0-net7.0.

Files

  • plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md — 13-step workflow
  • plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md — full attribute/assertion/fixture/lifecycle tables (progressive disclosure)
  • tests/dotnet-test/migrate-xunit-to-mstest/eval.yaml — 12 scenarios (skill-validator format, inline setup.files)
  • tests/dotnet-test/migrate-xunit-to-mstest/eval.vally.yaml + fixtures/ — Vally-format mirror of the same scenarios (40 fixture files), matching the convention of every sibling migration skill
  • plugins/dotnet-test/README.md — migration table row + intro bullet
  • plugins/dotnet-test/agents/test-migration.agent.md — triage row + two multi-step rules (xUnit→MSTest, xUnit→MSTest+MTP)

Evaluation scenarios (12)

  1. xUnit v2 -> MSTest preserving VSTest
  2. xUnit v3 -> MSTest preserving VSTest
  3. Stop when TFM unsupported by MSTest v4 (net7.0)
  4. IClassFixture<T> -> [ClassInitialize] / [ClassCleanup]
  5. ICollectionFixture<T> -> explicit scope decision ([DoNotParallelize])
  6. ITestOutputHelper -> TestContext
  7. Exception assertions — Throws vs ThrowsAny semantic trap
  8. MemberData / TheoryData -> DynamicData
  9. [Fact(Skip)] / [Trait] / Timeout -> [Ignore] / [TestCategory] / [TestProperty] / Timeout
  10. Parallelization preservation -> [assembly: Parallelize(ClassLevel)]
  11. xUnit v3 TestContext.Current.CancellationToken -> MSTest TestContext.CancellationToken
  12. Project already on MSTest — no-op

Static checks

dotnet run --project eng/skill-validator/src -- check --plugin ./plugins/dotnet-test

✅ All checks passed (24 skill(s), 11 agent(s), 1 plugin(s))

One soft warning: SKILL.md is 8.1k BPE tokens (above the "approaching comprehensive" threshold). Splitting was considered and rejected — the workflow is sequential and benefits from being one document; non-essential content already lives in references/mapping-cheatsheet.md.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

@Evangelink
Add migrate-xunit-to-mstest skill
6506129 
Adds a skill to migrate .NET test projects from xUnit.net (v2 or v3) to

MSTest v4. Covers package replacement, attribute/assertion/fixture/lifecycle

translation, ITestOutputHelper -> TestContext, [Trait] -> [TestCategory]/

[TestProperty], and xUnit v3 TestContext.Current.CancellationToken.

Front-loads parallelization handling because xUnit parallelizes test classes

by default while MSTest serializes — the single largest source of post-

migration regressions. Step 11 enumerates three explicit choices and how to

translate CollectionBehavior/MaxParallelThreads/[Collection] settings.

Preserves the existing test platform (VSTest stays VSTest; MTP stays MTP)

by default — bundling a platform migration would violate the

test-migration agent's 'never mix migration steps' rule.

- plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md (workflow)

- plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md

- tests/dotnet-test/migrate-xunit-to-mstest/eval.yaml (12 scenarios)

- tests/dotnet-test/migrate-xunit-to-mstest/eval.vally.yaml (parity with sibling skill)

- plugins/dotnet-test/README.md (table entry)

- plugins/dotnet-test/agents/test-migration.agent.md (triage routing)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 21:41
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026 •
edited
Loading

Skill Coverage Report

Plugin Skill Covered Coverage
✅ dotnet-test assertion-quality 20/22 90.9%
✅ dotnet-test code-testing-agent 4/5 80%
✅ dotnet-test crap-score 6/6 100%
✅ dotnet-test detect-static-dependencies 15/15 100%
✅ dotnet-test dotnet-test-frameworks 4/5 80%
❌ dotnet-test filter-syntax 0/1 0%
⚠️ dotnet-test generate-testability-wrappers 14/22 63.6%
✅ dotnet-test migrate-mstest-v1v2-to-v3 13/16 81.2%
✅ dotnet-test migrate-mstest-v3-to-v4 38/41 92.7%
⚠️ dotnet-test migrate-static-to-wrapper 15/21 71.4%
✅ dotnet-test migrate-vstest-to-mtp 21/25 84%
⚠️ dotnet-test migrate-xunit-to-mstest 36/46 78.3%
✅ dotnet-test migrate-xunit-to-xunit-v3 14/16 87.5%
✅ dotnet-test mtp-hot-reload 16/16 100%
✅ dotnet-test run-tests 15/16 93.8%
✅ dotnet-test test-anti-patterns 19/21 90.5%
⚠️ dotnet-test test-gap-analysis 16/24 66.7%
✅ dotnet-test test-smell-detection 22/26 84.6%
✅ dotnet-test test-tagging 24/28 85.7%
✅ dotnet-test writing-mstest-tests 38/44 86.4%
Uncovered: dotnet-test/assertion-quality
  • [Validation] Metrics are computed correctly (counts add up) (line 154)
  • [Validation] If the suite has good diversity, the report acknowledges this (line 159)
Uncovered: dotnet-test/code-testing-agent
  • [WorkflowStep] Step 2: Invoke the Test Generator (line 81)
Uncovered: dotnet-test/dotnet-test-frameworks
  • [CodePattern] Assert.AreEqual (line 63)
Uncovered: dotnet-test/filter-syntax
  • [CodePattern] [trait] (line 113)
Uncovered: dotnet-test/generate-testability-wrappers
  • [Validation] DI registration uses AddSingleton for stateless wrappers, AddTransient for stateful ones (line 217)
  • [Validation] Ambient context pattern includes AsyncLocal<T>, scoped disposal, and trade-off explanation (line 220)
  • [Pitfall] Ambient context without AsyncLocal (line 231)
  • [WorkflowStep] Step 5: Generate ambient context alternative (when DI is not available) (line 176)
  • [CodePattern] sealed (line 120)
  • [CodePattern] Assert.True (line 81)
  • [CodePattern] Assert.Equal (line 167)
  • [CodePattern] readonly (line 180)
Uncovered: dotnet-test/migrate-mstest-v1v2-to-v3
  • [Validation] Project builds with zero errors (line 185)
  • [Validation] All tests pass (dotnet test) -- compare pass/fail counts to pre-migration baseline (line 186)
  • [CodePattern] Assert.AreNotEqual (line 144)
Uncovered: dotnet-test/migrate-mstest-v3-to-v4
  • [CodePattern] [TestMethodAttribute] (line 172)
  • [CodePattern] [TestMethod] (line 265)
  • [CodePattern] [Timeout] (line 212)
Uncovered: dotnet-test/migrate-static-to-wrapper
  • [Validation] Required using directives added (line 164)
  • [Validation] Build succeeds after migration (line 166)
  • [Validation] Test files updated with appropriate test doubles (line 167)
  • [Validation] No behavioral changes introduced (wrapper delegates directly to the static) (line 168)
  • [Pitfall] Breaking static classes (line 175)
  • [Pitfall] Migrating too much at once (line 178)
Uncovered: dotnet-test/migrate-vstest-to-mtp
  • [Validation] dotnet build completes with zero errors (line 361)
  • [Validation] Test executable runs directly (e.g., ./bin/Debug/net8.0/MyTests.exe) (line 363)
  • [Validation] No vstest.console.exe invocations remain in CI scripts (line 366)
  • [CodePattern] [trait] (line 252)
Uncovered: dotnet-test/migrate-xunit-to-mstest
  • [Validation] using Xunit; and using Xunit.Abstractions; removed (line 517)
  • [Validation] xunit.runner.json removed; equivalent config in .runsettings / [assembly: Parallelize] (line 518)
  • [Validation] Project builds with zero errors (line 520)
  • [CodePattern] sealed (line 273)
  • [CodePattern] readonly (line 258)
  • [CodePattern] Parallelize (line 401)
  • [CodePattern] DoNotParallelize (line 430)
  • [CodePattern] Assert.False (line 343)
  • [CodePattern] Assert.IsFalse (line 343)
  • [CodePattern] [Fact] (line 304)
Uncovered: dotnet-test/migrate-xunit-to-xunit-v3
  • [WorkflowStep] Step 13: Build and verify (line 199)
  • [CodePattern] sealed (line 118)
Uncovered: dotnet-test/run-tests
  • [Validation] Correct dotnet test invocation was used for the detected platform and SDK version (line 253)
Uncovered: dotnet-test/test-anti-patterns
  • [Validation] Every finding includes a specific location (not just a general warning) (line 155)
  • [Validation] Recommendations are prioritized by severity (line 159)
Uncovered: dotnet-test/test-gap-analysis
  • [Validation] Trivial code (simple getters, auto-properties) is excluded from analysis (line 201)
  • [Validation] Findings are prioritized by risk, not just listed in source order (line 202)
  • [Validation] Report includes strengths (killed mutations) alongside gaps (line 203)
  • [Validation] Mutation categories are correctly labeled (line 204)
  • [Pitfall] Analyzing trivial code (line 210)
  • [Pitfall] Ignoring call chains (line 212)
  • [Pitfall] Over-counting mutations in generated code (line 213)
  • [Pitfall] Forgetting Rust's ? operator (line 219)
Uncovered: dotnet-test/test-smell-detection
  • [Validation] Every finding includes a concrete fix example (not just "fix this") (line 216)
  • [Validation] Contextually obvious numbers are not flagged as magic numbers (line 219)
  • [Validation] Severity levels are justified, not arbitrary (line 221)
  • [Pitfall] Treating skip annotations with reasons same as bare skips (line 238)
Uncovered: dotnet-test/test-tagging
  • [Validation] For report-only frameworks, no source files were modified (line 243)
  • [Pitfall] Guessing traits without reading the test body (line 250)
  • [WorkflowStep] Step 3: Classify each test method (line 92)
  • [CodePattern] [Category] (line 135)
Uncovered: dotnet-test/writing-mstest-tests
  • [CodePattern] Assert.IsNull (line 153)
  • [CodePattern] Assert.IsEmpty (line 177)
  • [CodePattern] Assert.DoesNotContain (line 177)
  • [CodePattern] Assert.AreSame (line 153)
  • [CodePattern] Assert.Contains (line 177)
  • [CodePattern] Assert.IsNotEmpty (line 177)

Copilot AI reviewed Jun 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new migrate-xunit-to-mstest skill to the dotnet-test plugin, providing an end-to-end workflow (plus a mapping reference) for migrating xUnit.net v2/v3 test projects to MSTest v4 while preserving the existing test platform (VSTest vs MTP). The PR also adds skill-validator and Vally eval coverage (with fixtures) and updates plugin documentation + the test-migration agent routing table to recognize the new migration path.

Changes:

  • Introduces the migrate-xunit-to-mstest skill with a 13-step migration workflow and a detailed mapping cheatsheet reference.
  • Adds eval.yaml, eval.vally.yaml, and fixture projects to validate key migration behaviors (TFM gating, fixtures, parallelization, output, exception semantics, etc.).
  • Updates plugins/dotnet-test/README.md and test-migration.agent.md to document and route “xUnit → MSTest” migrations.
Show a summary per file
File Description
plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md New skill workflow document for migrating xUnit v2/v3 → MSTest v4 (preserving platform).
plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Progressive-disclosure reference tables for attribute/assertion/fixture/lifecycle mappings.
plugins/dotnet-test/README.md Adds the new skill to the migration list and updates the migration summary bullet.
plugins/dotnet-test/agents/test-migration.agent.md Routes “Convert xUnit to MSTest” requests to the new skill and documents sequencing rules.
tests/dotnet-test/migrate-xunit-to-mstest/eval.yaml Skill-validator eval scenarios (inline setup.files) covering 12 migration behaviors.
tests/dotnet-test/migrate-xunit-to-mstest/eval.vally.yaml Vally mirror of the eval scenarios using fixture files.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-iclassfixture-to-classinitialize/TestProject.csproj Fixture: xUnit v2 project using IClassFixture<T>.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-iclassfixture-to-classinitialize/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-iclassfixture-to-classinitialize/DbFixture.cs Fixture: disposable class fixture type.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-iclassfixture-to-classinitialize/OrderTests.cs Fixture: test class using IClassFixture<T>.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-itestoutputhelper-to-testcontext/TestProject.csproj Fixture: xUnit v2 project using ITestOutputHelper.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-itestoutputhelper-to-testcontext/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-itestoutputhelper-to-testcontext/LoggingTests.cs Fixture: writes to ITestOutputHelper.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-memberdata-and-theorydata-to-dynamicdata/TestProject.csproj Fixture: xUnit v2 project using MemberData.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-memberdata-and-theorydata-to-dynamicdata/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-memberdata-and-theorydata-to-dynamicdata/DiscountTests.cs Fixture: [Theory] + [MemberData] data-driven test.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-skip-trait-and-timeout/TestProject.csproj Fixture: xUnit v2 project using Skip/Trait/Timeout.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-skip-trait-and-timeout/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-skip-trait-and-timeout/AnnotatedTests.cs Fixture: [Fact(Skip=...)], [Trait], and Timeout.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-xunit-v3-testcontext-current-cancellationtoken/TestProject.csproj Fixture: xUnit v3 project using TestContext.Current.CancellationToken.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-xunit-v3-testcontext-current-cancellationtoken/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-xunit-v3-testcontext-current-cancellationtoken/Directory.Build.props Fixture: pins IsTestingPlatformApplication=false.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/convert-xunit-v3-testcontext-current-cancellationtoken/CancellationTests.cs Fixture: uses xUnit v3 TestContext.Current.CancellationToken.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/handle-icollectionfixture-explicitly-do-not-silently-widen-scope/TestProject.csproj Fixture: xUnit v2 collection fixtures setup.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/handle-icollectionfixture-explicitly-do-not-silently-widen-scope/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/handle-icollectionfixture-explicitly-do-not-silently-widen-scope/DbCollection.cs Fixture: ICollectionFixture<T> + [CollectionDefinition(... DisableParallelization = true)].
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/handle-icollectionfixture-explicitly-do-not-silently-widen-scope/OrdersTests.cs Fixture: member class using [Collection("Db")].
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/handle-icollectionfixture-explicitly-do-not-silently-widen-scope/CustomersTests.cs Fixture: second member class using [Collection("Db")].
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/map-exception-assertions-correctly-xunit-throws-mstest-throwsexactly/TestProject.csproj Fixture: xUnit v2 exception assertion semantics.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/map-exception-assertions-correctly-xunit-throws-mstest-throwsexactly/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/map-exception-assertions-correctly-xunit-throws-mstest-throwsexactly/ExceptionTests.cs Fixture: Throws vs ThrowsAny (+ async variant).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v2-project-to-mstest-v4-preserving-vstest/TestProject.csproj Fixture: baseline xUnit v2 → MSTest migration (VSTest preserved).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v2-project-to-mstest-v4-preserving-vstest/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v2-project-to-mstest-v4-preserving-vstest/CalculatorTests.cs Fixture: basic assertions (Assert.Equal, Assert.True).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v3-project-to-mstest-v4-preserving-vstest/TestProject.csproj Fixture: baseline xUnit v3 → MSTest migration (VSTest preserved).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v3-project-to-mstest-v4-preserving-vstest/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v3-project-to-mstest-v4-preserving-vstest/Directory.Build.props Fixture: pins IsTestingPlatformApplication=false.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/migrate-basic-xunit-v3-project-to-mstest-v4-preserving-vstest/OrderTests.cs Fixture: [Theory] + [InlineData] conversion target.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/preserve-xunit-parallelization-default-with-assembly-parallelize/TestProject.csproj Fixture: baseline xUnit v2 project for parallelization preservation.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/preserve-xunit-parallelization-default-with-assembly-parallelize/global.json Fixture: declares VSTest runner.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/preserve-xunit-parallelization-default-with-assembly-parallelize/AlphaTests.cs Fixture: class-level parallelization scenario (class A).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/preserve-xunit-parallelization-default-with-assembly-parallelize/BetaTests.cs Fixture: class-level parallelization scenario (class B).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/recognize-project-already-on-mstest/TestProject.csproj Fixture: project already on MSTest (no xUnit).
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/recognize-project-already-on-mstest/ExistingTests.cs Fixture: existing MSTest test class/method.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/stop-when-target-framework-is-unsupported-by-mstest-v4/TestProject.csproj Fixture: net7.0 project to validate “stop on unsupported TFM for MSTest v4”.
tests/dotnet-test/migrate-xunit-to-mstest/fixtures/stop-when-target-framework-is-unsupported-by-mstest-v4/SampleTests.cs Fixture: minimal xUnit test source for unsupported-TFM scenario.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 46/46 changed files
  • Comments generated: 2

Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
@Evangelink
Fix step cross-references flagged in PR review
c169d65 
- Step 1.5 'Custom DataAttribute' bullet pointed to Step 6 (assertions);

  custom DataAttribute mapping lives in Step 5 (data-driven tests).

- Step 7 lifecycle table 'Constructor' row pointed to Step 8 for

  ITestOutputHelper; ITestOutputHelper conversion is in Step 9 (output).

- Commit-strategy callout said 'after Step 5 (asserts fixed)' and 'after

  Step 9 (fixtures/lifecycle rewritten)'; asserts are in Step 6 and

  fixtures/lifecycle complete after Step 8.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
@Evangelink
Map [assembly: Xunit.Trait] to [assembly: TestCategory]/[TestProperty]
7e6fc17 
Both TestCategoryAttribute and TestPropertyAttribute target Assembly,

Class, and Method, so an xUnit [assembly: Trait] does not need to be

demoted to per-class/per-method MSTest attributes — assembly scope is

preserved 1:1.

- references/mapping-cheatsheet.md §8 now maps [assembly: Trait] to

  [assembly: TestCategory] / [assembly: TestProperty] instead of 'Remove'.

- SKILL.md Step 12 promoted to 'Convert' (was 'Remove'); the same mapping

  is documented inline.

- Notes after the Step 4 and §1 trait tables now disclose the assembly

  target so users do not assume class/method-only scope.

Addresses PR feedback from @Evangelink.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 22:04
@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

Thanks for the review! Pushed two follow-up commits:

c169d65 — Fix step cross-references (review bot findings)

  • SKILL.md Step 1.5 Custom DataAttribute bullet pointed to Step 6 (assertions); now points to Step 5 (data-driven tests) where the mapping lives.
  • SKILL.md Step 7 lifecycle table Constructor row pointed to Step 8 (fixtures); now points to Step 9 (output and TestContext) where ITestOutputHelper conversion is described.
  • Also noticed the commit-strategy callout had the same drift: it said "after Step 5 (asserts fixed)" and "after Step 9 (fixtures/lifecycle rewritten)". Asserts are in Step 6 and fixtures/lifecycle wrap up at Step 8 — fixed.

7e6fc17 — Map [assembly: Xunit.Trait] to [assembly: TestCategory] / [assembly: TestProperty] (@Evangelink)

You're right — both TestCategoryAttribute and TestPropertyAttribute carry AttributeUsage(AttributeTargets.Assembly | Class | Method), so an xUnit [assembly: Trait] translates 1:1 to the corresponding MSTest assembly attribute without losing scope.

  • references/mapping-cheatsheet.md §8 now maps [assembly: Xunit.Trait("Category", "v")] -> [assembly: TestCategory("v")] and the non-category form -> [assembly: TestProperty("k", "v")] instead of recommending removal.
  • SKILL.md Step 12 renamed from "Remove xUnit assembly attributes" to "Convert xUnit assembly attributes" with the trait mappings inline.
  • The notes after the Step 4 attribute table and §1 of the cheatsheet now disclose that [TestCategory] / [TestProperty] target Assembly, Class, and Method, so users don't assume class/method-only scope.

On vally evals

The skill already ships an eval.vally.yaml at tests/dotnet-test/migrate-xunit-to-mstest/eval.vally.yaml with 40 fixture files mirroring all 12 eval.yaml scenarios — discovered by the dotnet-test suite glob tests/dotnet-test/*/eval.vally.yaml in .vally.yaml. The vally-evaluate: SKIPPED status on the prior run just reflects that the evaluation pipeline is gated on /evaluate / evaluate-now; will trigger below.

dotnet run --project eng/skill-validator/src -- check --plugin ./plugins/dotnet-test → ✅ All checks passed locally.

@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

/evaluate

Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
Copilot AI reviewed Jun 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 46/46 changed files
  • Comments generated: 1

Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
Evangelink
Evangelink commented Jun 3, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
@Evangelink
Address second round of PR review feedback
b962a08 
From @Evangelink:

- [Ignore]/[Timeout] do NOT discover a test on their own — they are

  modifiers. SKILL.md Step 4 + cheatsheet §1 now show [Fact(Skip)] ->

  [TestMethod] + [Ignore(...)] (and same for [Timeout]). Eval scenario 9

  rubric updated to require both attributes.

- TestProperty IS filterable (was incorrectly described as 'metadata that

  does not need filter integration'). Notes after the trait tables in

  both SKILL.md and cheatsheet §1 now show both filter syntaxes.

- For environmental gates (OS-/CI-/arch-conditional), point at MSTest

  3.10+'s condition attributes ([OSCondition], [CICondition],

  [ArchitectureCondition], [NonParallelizableCondition]) as the preferred

  alternative to overloading [TestCategory] or scattering Assert.Inconclusive

  through test bodies. Strengthened in cheatsheet §3.9, SKILL.md Step 6,

  Step 10 SkippableFact row, and the Common Pitfalls table.

- TestDataRow<T> is also a supported MSTest data source (strongly typed

  with per-row DisplayName/Ignore metadata). Added to both SKILL.md Step 5

  and cheatsheet §2 with link to the docs.

- MSTest.Sdk + <UseVSTest>true</UseVSTest> pulls in Microsoft.NET.Test.Sdk

  automatically — the manual PackageReference was incorrect. Removed it

  from both files (Step 2, Step 3 'common mistakes', and Common Pitfalls).

- Added the 'pin MSTest.Sdk in global.json msbuild-sdks' alternative as

  the recommended pattern for multi-project solutions.

- Xunit.Combinatorial -> Combinatorial.MSTest (Youssef1313 community port).

  Updated both SKILL.md Step 10 and cheatsheet §10.

From Copilot reviewer:

- MSTest.Sdk example in SKILL.md Step 2 Option B hardcoded

  <TargetFramework>net9.0</TargetFramework>, contradicting the

  preserve-TFM rule. Replaced with a placeholder + explicit comment.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

Thanks for the detailed review! Second round of fixes is in b962a08.

MSTest semantics

  • [Ignore] and [Timeout] are modifiers — they don't discover a test on their own. [Fact(Skip = ...)] / [Fact(Timeout = N)] now map to [TestMethod] + [Ignore(...)] / [TestMethod] + [Timeout(N)] everywhere (SKILL.md Step 4, cheatsheet §1). Eval scenario 9 rubric tightened to require both.
  • TestPropertyAttribute IS filterable (--filter "Owner=alice") — corrected the note that wrongly described it as non-filterable metadata. Both filter syntaxes shown.
  • TestCategoryAttribute / TestPropertyAttribute are valid at assembly scope too — already fixed in 7e6fc17.

Condition attributes (MSTest 3.10+)

  • [OSCondition], [CICondition], [ArchitectureCondition], [NonParallelizableCondition] now called out as the preferred alternative to Assert.SkipUnless / Assert.Inconclusive for environmental gates. Added explicit mapping rows + Common Pitfalls note.

Data sources

  • TestDataRow<T> added as a strongly-typed alternative alongside the existing IEnumerable<object[]> / DynamicData options (SKILL.md Step 5, cheatsheet §2).

MSTest.Sdk

  • Removed the incorrect "add Microsoft.NET.Test.Sdk PackageReference back" instruction — MSTest.Sdk + <UseVSTest>true</UseVSTest> pulls it in automatically (Step 2 Option B, Step 3, Common Pitfalls).
  • Added the recommended global.json msbuild-sdks pin pattern.
  • Replaced hard-coded <TargetFramework>net9.0</TargetFramework> in the Option B example with a placeholder + comment (Copilot finding).

Companion ports

  • Xunit.Combinatorial → Combinatorial.MSTest (Youssef1313's community port with matching attribute surface), SKILL.md Step 10 + cheatsheet §10.

Validator green locally (24 skills, 11 agents). Re-running /evaluate next.

@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

/evaluate

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Skill Validation Results

Skill Scenario Quality Skills Loaded Overfit Verdict
migrate-xunit-to-mstest Migrate basic xUnit v2 project to MSTest v4 preserving VSTest 4.0/5 → 4.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.10 ❌ [1]
migrate-xunit-to-mstest Migrate basic xUnit v3 project to MSTest v4 preserving VSTest 5.0/5 → 3.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill, read_bash ✅ 0.10 ❌
migrate-xunit-to-mstest Stop when target framework is unsupported by MSTest v4 1.7/5 → 2.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, bash / ✅ migrate-xunit-to-mstest; tools: skill, bash, read_bash ✅ 0.10 ✅ [2]
migrate-xunit-to-mstest Convert IClassFixture to ClassInitialize 3.3/5 → 3.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill, read_bash ✅ 0.10 ❌ [3]
migrate-xunit-to-mstest Handle ICollectionFixture explicitly (do not silently widen scope) 3.0/5 → 2.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill / ✅ migrate-xunit-to-mstest; tools: skill, bash, create ✅ 0.10 ❌ [4]
migrate-xunit-to-mstest Convert ITestOutputHelper to TestContext 4.3/5 → 5.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.10 ✅
migrate-xunit-to-mstest Map exception assertions correctly (xUnit Throws -> MSTest ThrowsExactly) 4.0/5 → 5.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.10 ✅ [5]
migrate-xunit-to-mstest Convert MemberData and TheoryData to DynamicData 4.3/5 → 3.7/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.10 ❌ [6]
migrate-xunit-to-mstest Convert Skip, Trait, and Timeout 3.0/5 → 3.0/5 ✅ migrate-xunit-to-mstest; tools: skill, glob, create / ✅ migrate-xunit-to-mstest; tools: skill, create, read_bash ✅ 0.10 ❌ [7]
migrate-xunit-to-mstest Preserve xUnit parallelization default with [assembly: Parallelize] 3.0/5 → 2.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill / ✅ migrate-xunit-to-mstest; tools: skill, create, bash ✅ 0.10 ✅ [8]
migrate-xunit-to-mstest Convert xUnit v3 TestContext.Current.CancellationToken 4.0/5 → 4.7/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill, grep, create, read_bash ✅ 0.10 ✅
migrate-xunit-to-mstest Recognize project already on MSTest 5.0/5 → 5.0/5 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.10 ❌ [9]

[1] (Isolated) Quality improved but weighted score is -34.3% due to: completion (✓ → ✗), judgment, errors (0 → 1), tokens (69826 → 112590)
[2] ⚠️ High run-to-run variance (CV=631%) — consider re-running with --runs 5
[3] ⚠️ High run-to-run variance (CV=149%) — consider re-running with --runs 5
[4] ⚠️ High run-to-run variance (CV=56%) — consider re-running with --runs 5
[5] ⚠️ High run-to-run variance (CV=275%) — consider re-running with --runs 5
[6] ⚠️ High run-to-run variance (CV=129%) — consider re-running with --runs 5
[7] ⚠️ High run-to-run variance (CV=210%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -32.7% due to: quality, judgment, tokens (50448 → 96081)
[8] ⚠️ High run-to-run variance (CV=121%) — consider re-running with --runs 5. (Isolated) Quality dropped but weighted score is +13.2% due to: time (75.5s → 53.3s), tool calls (10 → 8)
[9] (Plugin) Quality unchanged but weighted score is -7.4% due to: tokens (25736 → 75008), time (10.3s → 16.8s), tool calls (3 → 4)

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

To investigate failures, paste this to your AI coding agent:

For PR 718 in dotnet/skills, download eval artifacts with gh run download 26916428371 --repo dotnet/skills --pattern "skill-validator-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/b962a08595008e03e496c40e36af3b152887da71/eng/skill-validator/src/docs/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@Evangelink
Improve xUnit-to-MSTest skill effectiveness based on eval feedback
89e229a 
Eval results on PR #718 showed regressions on three rubric items where the

agent did the migration correctly but did not COMMUNICATE the judgement-call

decisions, so the judge could not verify the choice was deliberate:

- ICollectionFixture scope decision (rubric: 'Explicitly explains the scope

  decision instead of silently widening' scored 1/5 because the agent said

  'Let me convert everything' with zero scope discussion). Step 8 now requires

  a templated explanation of (source scope, target scope, what is shared, what

  is serialized, why the rejected alternative was rejected) before applying.

- Parallelization 'MSTest default is serial' messaging (rubric scored 1.7/5

  because the agent only implicitly conveyed it). Step 11 Choice A now demands

  a templated sentence explaining that the [assembly: Parallelize] is REQUIRED

  to match xUnit, otherwise the suite silently regresses to serial.

- Response Guidelines now lists the three judgement-call decisions that MUST

  be communicated up-front: fixture scope, parallelization model, and the

  Throws/ThrowsExactly semantic flip.

Additional fixes from this round:

- Step 2: explicit guidance to default to Option A (MSTest metapackage) when

  the user says 'preserve VSTest', so the safer PackageReference path is

  picked over MSTest.Sdk + UseVSTest.

- Step 4: pulled the '[Ignore]/[Timeout] are modifiers, need [TestMethod]'

  callout out of a table footer into a visible note above the table.

- known-domains.txt: allowlist github.com/Youssef1313/Combinatorial.MSTest

  (referenced from SKILL.md Step 10 and cheatsheet §10 after the recent

  Xunit.Combinatorial -> Combinatorial.MSTest switch).

Validator: green (24 skills, 11 agents).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 3, 2026 22:40
@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

/evaluate

Copilot AI reviewed Jun 3, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 47/47 changed files
  • Comments generated: 1

Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

Skill Validation Results

Skill Scenario Quality Skills Loaded Overfit Verdict
migrate-xunit-to-mstest Migrate basic xUnit v2 project to MSTest v4 preserving VSTest 4.0/5 → 4.7/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, read_bash ✅ 0.08 ✅ [1]
migrate-xunit-to-mstest Migrate basic xUnit v3 project to MSTest v4 preserving VSTest 3.7/5 → 3.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill / ✅ migrate-xunit-to-mstest; tools: skill, read_bash ✅ 0.08 ❌ [2]
migrate-xunit-to-mstest Stop when target framework is unsupported by MSTest v4 2.0/5 → 2.0/5 ✅ migrate-xunit-to-mstest; tools: skill, bash ✅ 0.08 ✅ [3]
migrate-xunit-to-mstest Convert IClassFixture to ClassInitialize 4.3/5 → 3.3/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill, create, read_bash ✅ 0.08 ❌ [4]
migrate-xunit-to-mstest Handle ICollectionFixture explicitly (do not silently widen scope) 2.3/5 → 3.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [5]
migrate-xunit-to-mstest Convert ITestOutputHelper to TestContext 5.0/5 → 5.0/5 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [6]
migrate-xunit-to-mstest Map exception assertions correctly (xUnit Throws -> MSTest ThrowsExactly) 4.0/5 → 4.0/5 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [7]
migrate-xunit-to-mstest Convert MemberData and TheoryData to DynamicData 4.0/5 → 4.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [8]
migrate-xunit-to-mstest Convert Skip, Trait, and Timeout 2.7/5 → 3.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [9]
migrate-xunit-to-mstest Preserve xUnit parallelization default with [assembly: Parallelize] 2.3/5 → 2.7/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, bash ✅ 0.08 ❌ [10]
migrate-xunit-to-mstest Convert xUnit v3 TestContext.Current.CancellationToken 1.0/5 → 1.0/5 ⚠️ NOT ACTIVATED / ✅ migrate-xunit-to-mstest; tools: skill, edit, bash, grep, report_intent, view ✅ 0.08 ❌ [11]
migrate-xunit-to-mstest Recognize project already on MSTest 5.0/5 → 5.0/5 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [12]

[1] ⚠️ High run-to-run variance (CV=139%) — consider re-running with --runs 5
[2] ⚠️ High run-to-run variance (CV=107%) — consider re-running with --runs 5
[3] ⚠️ High run-to-run variance (CV=565%) — consider re-running with --runs 5
[4] ⚠️ High run-to-run variance (CV=62%) — consider re-running with --runs 5
[5] ⚠️ High run-to-run variance (CV=50%) — consider re-running with --runs 5
[6] (Plugin) Quality unchanged but weighted score is -8.3% due to: tokens (69497 → 191397), quality, time (38.0s → 54.0s), tool calls (8 → 10)
[7] ⚠️ High run-to-run variance (CV=1196%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -5.9% due to: tokens (68875 → 143214), tool calls (8 → 10)
[8] ⚠️ High run-to-run variance (CV=274%) — consider re-running with --runs 5
[9] ⚠️ High run-to-run variance (CV=9427%) — consider re-running with --runs 5. (Isolated) Quality improved but weighted score is -5.7% due to: tokens (40555 → 98246), tool calls (7 → 9)
[10] ⚠️ High run-to-run variance (CV=96%) — consider re-running with --runs 5. (Isolated) Quality improved but weighted score is -30.9% due to: quality, judgment, tokens (46104 → 72145)
[11] ⚠️ High run-to-run variance (CV=85%) — consider re-running with --runs 5. (Isolated) Quality unchanged but weighted score is -10.5% due to: judgment
[12] (Plugin) Quality unchanged but weighted score is -6.9% due to: tokens (25743 → 76309), time (10.9s → 15.4s), tool calls (3 → 4)

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

To investigate failures, paste this to your AI coding agent:

For PR 718 in dotnet/skills, download eval artifacts with gh run download 26917545671 --repo dotnet/skills --pattern "skill-validator-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/89e229a9bc733420e37429386f60997eb434dbc6/eng/skill-validator/src/docs/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

@github-actions github-actions Bot added the waiting-on-author PR state label label Jun 3, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 3, 2026

👋 @Evangelink — this PR has 1 unresolved review thread(s). When you're ready, please address the feedback and push an update; the triage bot will pick up the next state automatically. (Add the no-stale label to silence further pings.)

@Evangelink
Add concrete TestContext.Current.CancellationToken migration example
fd10bc5 
The CancellationToken scenario was failing/low-scoring because the skill only
contained a one-line mapping with no concrete example. The agent often missed
the requirement to:

  - Add constructor TestContext injection (even when ITestOutputHelper is absent)
  - Map Assert.False -> Assert.IsFalse alongside the rename
  - NOT fabricate a CancellationTokenSource as a replacement

Add a full xUnit v3 -> MSTest worked example next to the TestContext.Current
mapping (Step 9), and reinforce the CancellationTokenSource warning in the
cheatsheet. The example covers all 3 rubric points for the scenario.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 3, 2026

Pushed d10bc59b to address the Convert xUnit v3 TestContext.Current.CancellationToken scenario.

Diagnosis (from inspecting the eval artifacts):

The 1.0/5 score wasn't an activation bug -- in plugin mode the skill was activated and used �dit/bash/grep/view, but still scored 1.0. The judge rejected outputs that:

  • Replaced TestContext.Current.CancellationToken with a freshly-created CancellationTokenSource (rubric: Does NOT introduce CancellationTokenSource as a fake replacement)
  • Failed to inject MSTest's instance TestContext (rubric: Uses MSTest's instance TestContext (constructor injection or property))
  • Forgot to also rename Assert.False -> Assert.IsFalse, so dotnet test failed

The previous wording at Step 9 was just three one-line mappings under xUnit v3 \TestContext.Current\:; no example, no explicit "do not fabricate a token source" guardrail.

Fix:

  • Added a full xUnit v3 -> MSTest worked example for the CancellationToken case at Step 9, showing the constructor-injection scaffold + the Assert.False -> Assert.IsFalse follow-through.
  • Added a REQUIRED callout: add the constructor injection even when the class has no ITestOutputHelper, and never swap for a new CancellationTokenSource.
  • Mirrored the CancellationTokenSource warning in
    eferences/mapping-cheatsheet.md row 6.

The NOT ACTIVATED marker on the isolated run looked like an SDK race (events.jsonl shows skill.invoked after session.shutdown), not a description-match issue -- isolated activation is high-variance (CV=85%) on this scenario. Making the skill more directly actionable should also reduce the chance the agent stalls on its first turn just absorbing the skill.

/evaluate

@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 4, 2026

/evaluate

Evangelink
Evangelink commented Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 4, 2026

Skill Validation Results

Skill Scenario Quality Skills Loaded Overfit Verdict
migrate-xunit-to-mstest Migrate basic xUnit v2 project to MSTest v4 preserving VSTest 4.0/5 → 4.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [1]
migrate-xunit-to-mstest Migrate basic xUnit v3 project to MSTest v4 preserving VSTest 5.0/5 → 4.0/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌
migrate-xunit-to-mstest Stop when target framework is unsupported by MSTest v4 2.0/5 → 2.0/5 ✅ migrate-xunit-to-mstest; tools: skill, read_bash / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [2]
migrate-xunit-to-mstest Convert IClassFixture to ClassInitialize 4.0/5 → 4.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill, create, read_bash ✅ 0.08 ✅
migrate-xunit-to-mstest Handle ICollectionFixture explicitly (do not silently widen scope) 4.0/5 → 4.3/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [3]
migrate-xunit-to-mstest Convert ITestOutputHelper to TestContext 5.0/5 → 4.3/5 🔴 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [4]
migrate-xunit-to-mstest Map exception assertions correctly (xUnit Throws -> MSTest ThrowsExactly) 4.0/5 → 5.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [5]
migrate-xunit-to-mstest Convert MemberData and TheoryData to DynamicData 4.7/5 → 5.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [6]
migrate-xunit-to-mstest Convert Skip, Trait, and Timeout 3.0/5 → 4.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill / ✅ migrate-xunit-to-mstest; tools: skill, create ✅ 0.08 ✅ [7]
migrate-xunit-to-mstest Preserve xUnit parallelization default with [assembly: Parallelize] 3.0/5 → 4.0/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create, glob / ✅ migrate-xunit-to-mstest; tools: skill, create ✅ 0.08 ✅
migrate-xunit-to-mstest Convert xUnit v3 TestContext.Current.CancellationToken 3.7/5 → 4.7/5 🟢 ✅ migrate-xunit-to-mstest; tools: skill, create / ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ✅ [8]
migrate-xunit-to-mstest Recognize project already on MSTest 5.0/5 → 5.0/5 ✅ migrate-xunit-to-mstest; tools: skill ✅ 0.08 ❌ [9]

[1] ⚠️ High run-to-run variance (CV=80%) — consider re-running with --runs 5
[2] ⚠️ High run-to-run variance (CV=87%) — consider re-running with --runs 5
[3] ⚠️ High run-to-run variance (CV=72%) — consider re-running with --runs 5
[4] ⚠️ High run-to-run variance (CV=70%) — consider re-running with --runs 5
[5] ⚠️ High run-to-run variance (CV=173%) — consider re-running with --runs 5
[6] ⚠️ High run-to-run variance (CV=6829%) — consider re-running with --runs 5
[7] ⚠️ High run-to-run variance (CV=378%) — consider re-running with --runs 5
[8] ⚠️ High run-to-run variance (CV=57%) — consider re-running with --runs 5
[9] (Plugin) Quality unchanged but weighted score is -9.5% due to: tokens (25736 → 76992), quality, time (10.5s → 16.9s), tool calls (3 → 4)

Model: claude-opus-4.6 | Judge: claude-opus-4.6

🔍 Full Results - additional metrics and failure investigation steps

To investigate failures, paste this to your AI coding agent:

For PR 718 in dotnet/skills, download eval artifacts with gh run download 26939777674 --repo dotnet/skills --pattern "skill-validator-results-*" --dir ./eval-results, then fetch https://raw.githubusercontent.com/dotnet/skills/fd10bc59b9ea51db25641e586cc5eefbcbd64924/eng/skill-validator/src/docs/InvestigatingResults.md and follow it to analyze the results.json files. Diagnose each failure, suggest fixes to the eval.yaml and skill content, and tell me what to fix first.

▶ Sessions Visualisation -- interactive replay of all evaluation sessions
📊 Session Analytics (preview) -- aggregated metrics across evaluation sessions

Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md
Youssef1313
Youssef1313 reviewed Jun 4, 2026
View reviewed changes
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md Outdated
@Evangelink
Address third round of PR review feedback
60d21b6 
- Remove fictional MSTest condition attributes [ArchitectureCondition] and
  [NonParallelizableCondition] from SKILL.md and cheatsheet (only [OSCondition]
  and [CICondition] exist in MSTest 3.10+; for non-parallel intent use
  [DoNotParallelize], for architecture gating fall back to runtime
  Assert.Inconclusive). (Copilot)
- TestPropertyAttribute targets only [Class|Method] (no AttributeTargets.Assembly),
  so [assembly: TestProperty(...)] does not compile. Update the trait notes in
  SKILL.md Step 4, cheatsheet section 1, Step 12, and cheatsheet section 8 to
  collapse a non-category xUnit [assembly: Trait] to [assembly: TestCategory] or
  push it down to per-class [TestProperty]. (@Evangelink)
- Step 1 platform detection: stop inlining a buggy VSTest-vs-MTP matrix;
  delegate to the platform-detection skill. Note explicitly that
  <UseMicrosoftTestingPlatformRunner> only affects 'dotnet run' and is not a
  reliable runner signal. (@Youssef1313)
- Step 2 Option A: add MTP code-coverage caveat -- Microsoft.NET.Test.Sdk
  pulls VSTest's Microsoft.CodeCoverage transitively, which can interfere with
  MTP's collector. Prefer Option B (MSTest.Sdk without UseVSTest) for MTP
  projects. (@Youssef1313)
- Step 3: remove fictional MSBuild properties (<CaptureConsoleOutput>,
  <UseRoslynCompilers>); restate the bullet about xunit.runner.json -> Step 11
  port instead. Restate the <UseMicrosoftTestingPlatformRunner> bullet to make
  clear it is not a runner switch. (@Youssef1313)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 4, 2026 17:04
@Evangelink
Copy link
Copy Markdown
Member Author

Evangelink commented Jun 4, 2026

Round 3 review feedback addressed in 60d21b6.

Highlights:

  • Removed fictional MSTest condition attributes (only [OSCondition] and [CICondition] ship in 3.10+).
  • Corrected [TestProperty] scope (Class/Method only -- not Assembly).
  • Step 1 platform detection now delegates to the platform-detection skill and stops using as a runner signal (it only affects dotnet run).
  • Step 2 Option A gained an MTP code-coverage caveat re: Microsoft.NET.Test.Sdk pulling VSTest's Microsoft.CodeCoverage transitively.
  • Step 3 dropped references to non-existent xUnit MSBuild properties.

Follow-up: the platform-detection skill itself contains the same incorrect UseMTP-runner-as-signal claim. I'll fix that in a separate PR to keep this one scoped to the xUnit migration.

/evaluate

Copilot AI reviewed Jun 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot's findings

  • Files reviewed: 47/47 changed files
  • Comments generated: 2

Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/SKILL.md
Comment thread plugins/dotnet-test/skills/migrate-xunit-to-mstest/references/mapping-cheatsheet.md Outdated
@Evangelink
Address Copilot review round 4
2d84d3f 
- mapping-cheatsheet.md (Assert.Contains predicate): the suggested fallback
  Assert.Contains(collection.First(x => predicate), collection) is buggy --
  First() throws InvalidOperationException on no-match (changing the failure
  mode), and feeding the result back into Assert.Contains is circular. Keep
  only the correct translation: Assert.IsTrue(collection.Any(x => predicate)).
- SKILL.md Step 4 / cheatsheet table: stop instructing 'mark every class
  sealed'. xUnit projects commonly use base/derived test classes (shared
  setup, generic base fixtures); mechanically sealing them would break
  compilation. Sealing is now framed as an optional follow-up handled by
  writing-mstest-tests, not part of the mechanical migration.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions github-actions Bot added pr-state/ready-for-eval PR is mergeable and awaiting evaluation and removed waiting-on-author PR state label labels Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Youssef1313 Youssef1313 Youssef1313 left review comments

Copilot code review Copilot Copilot left review comments

@ViktorHofer ViktorHofer Awaiting requested review from ViktorHofer ViktorHofer is a code owner

@JanKrivanek JanKrivanek Awaiting requested review from JanKrivanek JanKrivanek is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

pr-state/ready-for-eval PR is mergeable and awaiting evaluation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Evangelink @Youssef1313