mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
030493de5a
* 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 from6b11ad9. 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, which6b11ad9addresses. 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 in6b11ad9and22103d2all 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