From 0bd8b3742b42a91fa678e16e36925f3b0b8ccea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 14:07:12 +0200 Subject: [PATCH 1/7] Deduplicate test skill references and clarify skill boundaries - Move platform-detection.md and filter-syntax.md to plugins/dotnet-test/shared/, removing 3 identical copies of each from run-tests, mtp-hot-reload, and migrate-vstest-to-mtp reference directories. - Move dotnet.md from exp-test-smell-detection/extensions/ to shared/ as dotnet-test-frameworks.md in both dotnet-test and dotnet-experimental plugins. Update exp-assertion-quality, exp-test-boilerplate-detection, exp-test-tagging, and test-anti-patterns to reference the shared file instead of inlining framework detection tables. - Differentiate test-anti-patterns (quick pragmatic review) from exp-test-smell-detection (deep formal audit with academic taxonomy) by updating descriptions and cross-referencing each other in When Not to Use sections. - Update skill-validator to allow ../../shared/ file references while still blocking other parent-directory traversals. Add tests for the new rule. --- .../src/Check/SkillProfiler.cs | 24 ++- .../tests/Check/SkillProfileTests.cs | 24 +++ .../dotnet-test-frameworks.md} | 0 .../skills/exp-assertion-quality/SKILL.md | 2 +- .../exp-test-boilerplate-detection/SKILL.md | 2 +- .../skills/exp-test-smell-detection/SKILL.md | 23 +-- .../skills/exp-test-tagging/SKILL.md | 8 +- .../shared/dotnet-test-frameworks.md | 111 ++++++++++++ .../references => shared}/filter-syntax.md | 0 .../platform-detection.md | 0 .../skills/migrate-vstest-to-mtp/SKILL.md | 4 +- .../skills/mtp-hot-reload/SKILL.md | 4 +- .../references/filter-syntax.md | 166 ------------------ .../references/platform-detection.md | 53 ------ plugins/dotnet-test/skills/run-tests/SKILL.md | 4 +- .../run-tests/references/filter-syntax.md | 166 ------------------ .../references/platform-detection.md | 53 ------ .../skills/test-anti-patterns/SKILL.md | 7 +- 18 files changed, 182 insertions(+), 469 deletions(-) rename plugins/dotnet-experimental/{skills/exp-test-smell-detection/extensions/dotnet.md => shared/dotnet-test-frameworks.md} (100%) create mode 100644 plugins/dotnet-test/shared/dotnet-test-frameworks.md rename plugins/dotnet-test/{skills/migrate-vstest-to-mtp/references => shared}/filter-syntax.md (100%) rename plugins/dotnet-test/{skills/migrate-vstest-to-mtp/references => shared}/platform-detection.md (100%) delete mode 100644 plugins/dotnet-test/skills/mtp-hot-reload/references/filter-syntax.md delete mode 100644 plugins/dotnet-test/skills/mtp-hot-reload/references/platform-detection.md delete mode 100644 plugins/dotnet-test/skills/run-tests/references/filter-syntax.md delete mode 100644 plugins/dotnet-test/skills/run-tests/references/platform-detection.md diff --git a/eng/skill-validator/src/Check/SkillProfiler.cs b/eng/skill-validator/src/Check/SkillProfiler.cs index 6fa05010a7..564c198c58 100644 --- a/eng/skill-validator/src/Check/SkillProfiler.cs +++ b/eng/skill-validator/src/Check/SkillProfiler.cs @@ -127,10 +127,30 @@ public static SkillProfile AnalyzeSkill(SkillInfo skill) var segments = refPath.Split('/'); - // Reject parent-directory traversals + // Allow parent-directory traversals only when resolving to a plugin-level shared/ directory. + // Skills live at plugins//skills//, so ../../shared/ resolves to + // the plugin's shared/ directory. This enables deduplication of reference files that + // multiple skills within the same plugin need (e.g., platform-detection.md). if (segments.Any(s => s == "..")) { - errors.Add($"File reference '{refMatch.Groups[1].Value}' uses parent-directory traversal — references must stay within the skill directory."); + bool isAllowedSharedRef = + segments.Length >= 4 && + segments[0] == ".." && segments[1] == ".." && segments[2] == "shared" && + segments.Take(2).All(s => s == "..") && + !segments.Skip(2).Any(s => s == ".."); + + if (!isAllowedSharedRef) + { + errors.Add($"File reference '{refMatch.Groups[1].Value}' uses parent-directory traversal — references must stay within the skill directory or use ../../shared/."); + continue; + } + + // Depth inside shared/ (exclude the "../../shared" prefix and the filename) + int sharedDirDepth = segments.Length - 4; // segments: [.., .., shared, , filename] + if (sharedDirDepth > 0) + { + errors.Add($"File reference '{refMatch.Groups[1].Value}' is {sharedDirDepth + 1} directories deep inside shared/ — files must be directly inside shared/."); + } continue; } diff --git a/eng/skill-validator/tests/Check/SkillProfileTests.cs b/eng/skill-validator/tests/Check/SkillProfileTests.cs index eb73d562c9..09109a4093 100644 --- a/eng/skill-validator/tests/Check/SkillProfileTests.cs +++ b/eng/skill-validator/tests/Check/SkillProfileTests.cs @@ -322,6 +322,30 @@ public void ParentDirectoryTraversalErrors() Assert.Contains(profile.Errors, e => e.Contains("parent-directory traversal")); } + [Fact] + public void SharedDirectoryTraversalAllowed() + { + var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../shared/platform-detection.md)\n" + new string('x', 4000); + var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); + Assert.DoesNotContain(profile.Errors, e => e.Contains("parent-directory traversal") || e.Contains("shared/")); + } + + [Fact] + public void SharedDirectoryDeepTraversalErrors() + { + var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../shared/sub/file.md)\n" + new string('x', 4000); + var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); + Assert.Contains(profile.Errors, e => e.Contains("inside shared/")); + } + + [Fact] + public void NonSharedParentTraversalStillErrors() + { + var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../other/file.md)\n" + new string('x', 4000); + var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); + Assert.Contains(profile.Errors, e => e.Contains("parent-directory traversal")); + } + [Fact] public void AnchorFragmentStrippedFromDepthCheck() { diff --git a/plugins/dotnet-experimental/skills/exp-test-smell-detection/extensions/dotnet.md b/plugins/dotnet-experimental/shared/dotnet-test-frameworks.md similarity index 100% rename from plugins/dotnet-experimental/skills/exp-test-smell-detection/extensions/dotnet.md rename to plugins/dotnet-experimental/shared/dotnet-test-frameworks.md diff --git a/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md b/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md index b515fac791..5bb05cd2d3 100644 --- a/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md @@ -46,7 +46,7 @@ Low assertion diversity signals shallow testing. Tests may pass while bugs hide ### Step 1: Gather the test code -Read all test files the user provides. If the user points to a directory or project, scan for all test files (files containing `[TestClass]`, `[TestMethod]`, `[Fact]`, `[Test]`, or `[Theory]` attributes). +Read all test files the user provides. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers. ### Step 2: Classify every assertion diff --git a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md index d4090f67f1..b7cfec04bb 100644 --- a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md @@ -34,7 +34,7 @@ Analyze .NET test code to find duplicated boilerplate patterns across test metho ### Step 1: Gather the test code -Read all test files the user provides or references. If the user points to a directory or project, scan for all test files (files containing `[TestClass]`, `[TestMethod]`, `[Fact]`, `[Test]`, or `[Theory]` attributes). +Read all test files the user provides or references. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers. ### Step 2: Identify boilerplate categories diff --git a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md index a54906aac8..b18499ae60 100644 --- a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md @@ -1,11 +1,11 @@ --- name: exp-test-smell-detection -description: "Detects test smells — bad programming practices in test code that indicate design problems and reduce test effectiveness. Use when the user asks to find test smells, review test quality, audit test health, identify problematic test patterns, or detect anti-patterns in test suites. Produces a categorized report with severity, locations, and concrete fix suggestions. Works with any test framework and language. DO NOT USE FOR: writing new tests (use writing-mstest-tests), evaluating assertion quality specifically (use exp-assertion-quality), or detecting boilerplate duplication (use exp-test-boilerplate-detection)." +description: "Deep formal test smell audit based on academic research taxonomy (testsmells.org). Detects 19 categorized smell types — conditional logic, mystery guests, sensitive equality, eager tests, and more — with calibrated severity and research-backed remediation. Use for comprehensive test suite health assessments. For a quick pragmatic review, use test-anti-patterns instead. DO NOT USE FOR: writing new tests (use writing-mstest-tests), evaluating assertion quality specifically (use exp-assertion-quality), or detecting boilerplate duplication (use exp-test-boilerplate-detection)." --- # Test Smell Detection -Analyze test code to detect test smells — symptoms of bad design or implementation decisions that make tests harder to understand, more fragile, less effective at catching bugs, or more expensive to maintain. Produce a severity-ranked report of findings with specific locations and actionable fixes. +Deep formal audit of test code using an academic test smell taxonomy. Detects symptoms of bad design or implementation decisions that make tests harder to understand, more fragile, less effective at catching bugs, or more expensive to maintain. Produces a severity-ranked report with specific locations and actionable fixes. ## Why Test Smells Matter @@ -24,14 +24,15 @@ Test smells erode confidence in a test suite and inflate maintenance costs: ## When to Use -- User asks to find test smells or anti-patterns in test code -- User asks "are my tests well-written?" or "what's wrong with my tests?" -- User wants a test quality audit or health check -- User asks for a review of test design or structure -- User suspects tests are fragile, flaky, or giving false confidence +- User asks for a comprehensive or formal test smell audit +- User asks "are my tests well-written?" and wants a thorough analysis +- User wants a test quality health check with academic rigor +- User asks for a review of test design or structure using standard smell categories +- User suspects tests are fragile, flaky, or giving false confidence and wants a deep investigation ## When Not to Use +- User wants a quick pragmatic test review (use `test-anti-patterns` — faster, covers the most common issues) - User wants to evaluate assertion diversity specifically (use `exp-assertion-quality`) - User wants to find duplicated boilerplate across tests (use `exp-test-boilerplate-detection`) - User wants to write new tests from scratch (help them directly) @@ -48,7 +49,7 @@ Test smells erode confidence in a test suite and inflate maintenance costs: ### Step 1: Gather the test code -Read all test files the user provides. If the user points to a directory or project, scan for all test files by looking for test framework markers — see [extensions/dotnet.md](extensions/dotnet.md) for .NET-specific markers. +Read all test files the user provides. If the user points to a directory or project, scan for all test files by looking for test framework markers — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET-specific markers. For a thorough audit, also consult the [extended smell catalog](references/test-smell-catalog.md) which covers 9 additional smell types beyond the core 10 below. @@ -77,7 +78,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 -**Detection:** Calls to sleep/delay functions inside test methods. See [extensions/dotnet.md](extensions/dotnet.md) for .NET-specific patterns. +**Detection:** Calls to sleep/delay functions inside test methods. See [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET-specific patterns. #### Smell 4: Assertion-Free Test (Unknown Test) @@ -130,7 +131,7 @@ The test setup method or constructor initializes fields that are not used by eve Tests marked as skipped or disabled. These add overhead and clutter, and the underlying issue they were disabled for may never be addressed. **Severity:** Low -**Detection:** Skip/ignore annotations or conditional compilation that disables a test. See [extensions/dotnet.md](extensions/dotnet.md) for framework-specific skip attributes. +**Detection:** Skip/ignore annotations or conditional compilation that disables a test. See [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific skip attributes. ### Step 3: Apply calibration rules @@ -190,7 +191,7 @@ Present the analysis in this structure: | Flagging integration tests for using real resources | Check for integration test markers and adjust severity accordingly | | Flagging loop-over-collection-assert as conditional logic | Only flag loops with branching or complex logic, not assertion iterations | | Flagging obvious count assertions after adding N items | Consider the immediate context — self-documenting numbers are fine | -| Missing framework-specific assertion syntax | Consult [extensions/dotnet.md](extensions/dotnet.md) for .NET framework assertion and skip APIs | +| Missing framework-specific assertion syntax | Consult [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET framework assertion and skip APIs | | Over-flagging try/catch that captures for assertion | Distinguish swallowed exceptions from capture-and-assert patterns | | Treating skip annotations with reasons same as bare skips | Note that reasoned skips are less concerning than unexplained ones | | Flagging `DoesNotThrow`-style tests as assertion-free | These implicitly assert no exception — note but acknowledge the intent | diff --git a/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md index bb3dfe7fad..4a1cfa20db 100644 --- a/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md @@ -56,13 +56,7 @@ A single test may have **multiple traits** (e.g., both `negative` and `boundary` ### Step 1: Detect the test framework -Examine project files (`.csproj` / `.fsproj`) and `using` directives to determine the framework: - -| Signal | Framework | -|--------|-----------| -| `` or `using Microsoft.VisualStudio.TestTools.UnitTesting` | MSTest | -| `` or `using Xunit` | xUnit | -| `` or `using NUnit.Framework` | NUnit | +Examine project files and source code to determine the framework — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for the complete detection table (package references, test markers, assertion APIs, and skip annotations). ### Step 2: Scan existing traits diff --git a/plugins/dotnet-test/shared/dotnet-test-frameworks.md b/plugins/dotnet-test/shared/dotnet-test-frameworks.md new file mode 100644 index 0000000000..7ebaa9c839 --- /dev/null +++ b/plugins/dotnet-test/shared/dotnet-test-frameworks.md @@ -0,0 +1,111 @@ +# .NET Extension + +Language-specific detection patterns for .NET test frameworks (MSTest, xUnit, NUnit, TUnit). + +## Test File Identification + +| Framework | Test class markers | Test method markers | +| --------- | ------------------ | ------------------- | +| MSTest | `[TestClass]` | `[TestMethod]`, `[DataTestMethod]` | +| xUnit | _(none — convention-based)_ | `[Fact]`, `[Theory]` | +| NUnit | `[TestFixture]` | `[Test]`, `[TestCase]`, `[TestCaseSource]` | +| TUnit | `[ClassDataSource]` | `[Test]` | + +## Assertion APIs by Framework + +| Category | MSTest | xUnit | NUnit | +| -------- | ------ | ----- | ----- | +| Equality | `Assert.AreEqual` | `Assert.Equal` | `Assert.That(x, Is.EqualTo(y))` | +| Boolean | `Assert.IsTrue` / `Assert.IsFalse` | `Assert.True` / `Assert.False` | `Assert.That(x, Is.True)` | +| Null | `Assert.IsNull` / `Assert.IsNotNull` | `Assert.Null` / `Assert.NotNull` | `Assert.That(x, Is.Null)` | +| Exception | `Assert.Throws()` / `Assert.ThrowsExactly()` | `Assert.Throws()` | `Assert.That(() => ..., Throws.TypeOf())` | +| Collection | `CollectionAssert.Contains` | `Assert.Contains` | `Assert.That(col, Has.Member(x))` | +| String | `StringAssert.Contains` | `Assert.Contains(str, sub)` | `Assert.That(str, Does.Contain(sub))` | +| Type | `Assert.IsInstanceOfType` | `Assert.IsAssignableFrom` | `Assert.That(x, Is.InstanceOf())` | +| Inconclusive | `Assert.Inconclusive()` | _skip via `[Fact(Skip)]`_ | `Assert.Inconclusive()` | +| Fail | `Assert.Fail()` | `Assert.Fail()` (.NET 10+) | `Assert.Fail()` | + +Third-party assertion libraries: `Should*` (Shouldly), `.Should()` (FluentAssertions / AwesomeAssertions), `Verify()` (Verify). + +## Sleep/Delay Patterns + +| Pattern | Example | +| ------- | ------- | +| Thread sleep | `Thread.Sleep(2000)` | +| Task delay | `await Task.Delay(1000)` | +| SpinWait | `SpinWait.SpinUntil(() => condition, timeout)` | + +## Skip/Ignore Annotations + +| Framework | Annotation | With reason | +| --------- | ---------- | ----------- | +| MSTest | `[Ignore]` | `[Ignore("reason")]` | +| xUnit | `[Fact(Skip = "reason")]` | _(reason is required)_ | +| NUnit | `[Ignore("reason")]` | _(reason is required)_ | +| TUnit | `[Skip("reason")]` | _(reason is required)_ | +| Conditional | `#if false` / `#if NEVER` | _(no reason possible)_ | + +## Exception Handling — Idiomatic Alternatives + +When a test uses `try`/`catch` to verify exceptions, suggest the framework-native alternative: + +**MSTest:** + +```csharp +// Instead of try/catch (matches exact type): +var ex = Assert.ThrowsExactly( + () => processor.ProcessOrder(emptyOrder)); +Assert.AreEqual("Order must contain at least one item", ex.Message); + +// Or (also matches derived types): +var ex = Assert.Throws( + () => processor.ProcessOrder(emptyOrder)); +Assert.AreEqual("Order must contain at least one item", ex.Message); +``` + +**xUnit:** + +```csharp +var ex = Assert.Throws( + () => processor.ProcessOrder(emptyOrder)); +Assert.Equal("Order must contain at least one item", ex.Message); +``` + +**NUnit:** + +```csharp +var ex = Assert.Throws( + () => processor.ProcessOrder(emptyOrder)); +Assert.That(ex.Message, Is.EqualTo("Order must contain at least one item")); +``` + +## Mystery Guest — Common .NET Patterns + +| Smell indicator | What to look for | +| --------------- | ---------------- | +| File system | `File.ReadAllText`, `File.Exists`, `File.WriteAllBytes`, `Directory.GetFiles`, `Path.Combine` with hard-coded paths | +| Database | `SqlConnection`, `DbContext` (without in-memory provider), `SqlCommand` | +| Network | `HttpClient` without `HttpMessageHandler` override, `WebRequest`, `TcpClient` | +| Environment | `Environment.GetEnvironmentVariable`, `Environment.CurrentDirectory` | +| Acceptable | `MemoryStream`, `StringReader`, `InMemory` database providers, custom `DelegatingHandler` | + +## Integration Test Markers + +Recognize these as integration tests (adjust smell severity accordingly): + +- Class name contains `Integration`, `E2E`, `EndToEnd`, or `Acceptance` +- `[TestCategory("Integration")]` (MSTest) +- `[Trait("Category", "Integration")]` (xUnit) +- `[Category("Integration")]` (NUnit) +- Project name ending in `.IntegrationTests` or `.E2ETests` + +## Setup/Teardown Methods + +| Framework | Setup | Teardown | +| --------- | ----- | -------- | +| MSTest | `[TestInitialize]` or constructor | `[TestCleanup]` or `IDisposable.Dispose` / `IAsyncDisposable.DisposeAsync` | +| xUnit | constructor | `IDisposable.Dispose` / `IAsyncDisposable.DisposeAsync` | +| NUnit | `[SetUp]` | `[TearDown]` | +| MSTest (class) | `[ClassInitialize]` | `[ClassCleanup]` | +| NUnit (class) | `[OneTimeSetUp]` | `[OneTimeTearDown]` | +| xUnit (class) | `IClassFixture` | fixture's `Dispose` | diff --git a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/references/filter-syntax.md b/plugins/dotnet-test/shared/filter-syntax.md similarity index 100% rename from plugins/dotnet-test/skills/migrate-vstest-to-mtp/references/filter-syntax.md rename to plugins/dotnet-test/shared/filter-syntax.md diff --git a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/references/platform-detection.md b/plugins/dotnet-test/shared/platform-detection.md similarity index 100% rename from plugins/dotnet-test/skills/migrate-vstest-to-mtp/references/platform-detection.md rename to plugins/dotnet-test/shared/platform-detection.md diff --git a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md b/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md index 19584873ea..48eab919cc 100644 --- a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md +++ b/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md @@ -52,7 +52,7 @@ Migrate a .NET test solution from VSTest to Microsoft.Testing.Platform (MTP). Th ### Step 1: Assess the solution -1. Identify the test framework for each test project -- see [references/platform-detection.md](references/platform-detection.md) for the package-to-framework mapping. Key indicators: +1. Identify the test framework for each test project -- see [platform-detection.md](../../shared/platform-detection.md) for the package-to-framework mapping. Key indicators: - **MSTest**: References `MSTest` or `MSTest.TestAdapter`, or uses `MSTest.Sdk` (with `` not set to `false`). Note: `MSTest.TestFramework` alone is a library dependency, not a test project. - **NUnit**: References `NUnit3TestAdapter` - **xUnit.net**: References `xunit` and `xunit.runner.visualstudio` @@ -197,7 +197,7 @@ VSTest-specific arguments must be translated to MTP equivalents. Build-related a **MSTest, NUnit, and xUnit.net v2 (with `YTest.MTP.XUnit2`)**: The VSTest `--filter` syntax is identical on both VSTest and MTP. No changes needed. -**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. See the **VSTest -> MTP filter translation** section in [references/filter-syntax.md](references/filter-syntax.md) for the complete translation table. Key translation example: +**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. See the **VSTest -> MTP filter translation** section in [filter-syntax.md](../../shared/filter-syntax.md) for the complete translation table. Key translation example: ```shell # VSTest diff --git a/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md b/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md index 0d6ce60e89..38c8f6e417 100644 --- a/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md +++ b/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md @@ -44,7 +44,7 @@ Set up and use Microsoft Testing Platform hot reload to rapidly iterate fixes on Hot reload requires MTP. It does **not** work with VSTest. -Follow the detection procedure in [references/platform-detection.md](references/platform-detection.md) to determine the test platform. +Follow the detection procedure in [platform-detection.md](../../shared/platform-detection.md) to determine the test platform. If the project uses VSTest, inform the user that MTP hot reload is not available and suggest migrating to MTP first (see `migrate-vstest-to-mtp`), or using Visual Studio's built-in Test Explorer hot reload feature instead. @@ -97,7 +97,7 @@ Run the test project directly (not through `dotnet test`) to use hot reload in c dotnet run --project ``` -To filter to specific failing tests, pass the filter after `--`. The syntax depends on the test framework -- see [references/filter-syntax.md](references/filter-syntax.md) for full details. Quick examples: +To filter to specific failing tests, pass the filter after `--`. The syntax depends on the test framework -- see [filter-syntax.md](../../shared/filter-syntax.md) for full details. Quick examples: | Framework | Filter syntax | |-----------|--------------| diff --git a/plugins/dotnet-test/skills/mtp-hot-reload/references/filter-syntax.md b/plugins/dotnet-test/skills/mtp-hot-reload/references/filter-syntax.md deleted file mode 100644 index 03d23ec57d..0000000000 --- a/plugins/dotnet-test/skills/mtp-hot-reload/references/filter-syntax.md +++ /dev/null @@ -1,166 +0,0 @@ -# Test Filter Syntax Reference - -Filter syntax depends on the **platform** and **test framework**. - -## VSTest filters (MSTest, xUnit v2, NUnit on VSTest) - -```bash -dotnet test --filter -``` - -Expression syntax: `[|&]` - -**Operators:** - -| Operator | Meaning | -|----------|---------| -| `=` | Exact match | -| `!=` | Not exact match | -| `~` | Contains | -| `!~` | Does not contain | - -**Combinators:** `|` (OR), `&` (AND). Parentheses for grouping: `(A|B)&C` - -**Supported properties by framework:** - -| Framework | Properties | -|-----------|-----------| -| MSTest | `FullyQualifiedName`, `Name`, `ClassName`, `Priority`, `TestCategory` | -| xUnit | `FullyQualifiedName`, `DisplayName`, `Traits` | -| NUnit | `FullyQualifiedName`, `Name`, `Priority`, `TestCategory` | - -An expression without an operator is treated as `FullyQualifiedName~`. - -**Examples (VSTest):** - -```bash -# Run tests whose name contains "LoginTest" -dotnet test --filter "Name~LoginTest" - -# Run a specific test class -dotnet test --filter "ClassName=MyNamespace.MyTestClass" - -# Run tests in a category -dotnet test --filter "TestCategory=Integration" - -# Exclude a category -dotnet test --filter "TestCategory!=Slow" - -# Combine: class AND category -dotnet test --filter "ClassName=MyNamespace.MyTestClass&TestCategory=Unit" - -# Either of two classes -dotnet test --filter "ClassName=MyNamespace.ClassA|ClassName=MyNamespace.ClassB" -``` - -## MTP filters — MSTest and NUnit - -MSTest and NUnit on MTP use the **same `--filter` syntax** as VSTest (same properties, operators, and combinators). The only difference is how the flag is passed: - -```bash -# .NET SDK 8/9 (after --) -dotnet test -- --filter "Name~LoginTest" - -# .NET SDK 10+ (direct) -dotnet test --filter "Name~LoginTest" -``` - -## MTP filters — xUnit (v3) - -xUnit v3 on MTP uses **framework-specific filter flags** instead of the generic `--filter` expression: - -| Flag | Description | -|------|-------------| -| `--filter-class "name"` | Run all tests in a given class | -| `--filter-not-class "name"` | Exclude all tests in a given class | -| `--filter-method "name"` | Run a specific test method | -| `--filter-not-method "name"` | Exclude a specific test method | -| `--filter-namespace "name"` | Run all tests in a namespace | -| `--filter-not-namespace "name"` | Exclude all tests in a namespace | -| `--filter-trait "name=value"` | Run tests with a matching trait | -| `--filter-not-trait "name=value"` | Exclude tests with a matching trait | - -Multiple values can be specified with a single flag: `--filter-class Foo Bar`. - -```bash -# .NET SDK 8/9 -dotnet test -- --filter-class "MyNamespace.LoginTests" - -# .NET SDK 10+ -dotnet test --filter-class "MyNamespace.LoginTests" - -# Combine: namespace + trait -dotnet test --filter-namespace "MyApp.Tests.Integration" --filter-trait "Category=Smoke" -``` - -### xUnit v3 query filter language - -For complex expressions, use `--filter-query` with a path-segment syntax: - -``` -////[traitName=traitValue] -``` - -Each segment matches against: assembly name, namespace, class name, method name. Use `*` for "match all" in any segment. Documentation: https://xunit.net/docs/query-filter-language - -```shell -# xUnit.net v3 MTP — using query language (assembly/namespace/class/method[trait]) -dotnet test -- --filter-query "/*/*/*IntegrationTests*/*[Category=Smoke]" -``` - -## MTP filters — TUnit - -TUnit uses `--treenode-filter` with a path-based syntax: - -``` ---treenode-filter "////" -``` - -Wildcards (`*`) are supported in any segment. Filter operators can be appended to test names for property-based filtering. - -| Operator | Meaning | -|----------|---------| -| `*` | Wildcard match | -| `=` | Exact property match (e.g., `[Category=Unit]`) | -| `!=` | Exclude property value | -| `&` | AND (combine conditions) | -| `\|` | OR (within a segment, requires parentheses) | - -**Examples (TUnit):** - -```bash -# All tests in a class -dotnet run --treenode-filter "/*/*/LoginTests/*" - -# A specific test -dotnet run --treenode-filter "/*/*/*/AcceptCookiesTest" - -# By namespace prefix (wildcard) -dotnet run --treenode-filter "/*/MyProject.Tests.Api*/*/*" - -# By custom property -dotnet run --treenode-filter "/*/*/*/*[Category=Smoke]" - -# Exclude by property -dotnet run --treenode-filter "/*/*/*/*[Category!=Slow]" - -# OR across classes -dotnet run --treenode-filter "/*/*/(LoginTests)|(SignupTests)/*" - -# Combined: namespace + property -dotnet run --treenode-filter "/*/MyProject.Tests.Integration/*/*/*[Priority=Critical]" -``` - -## VSTest → MTP filter translation (for migration) - -**MSTest, NUnit, and xUnit.net v2 (with `YTest.MTP.XUnit2`)**: The VSTest `--filter` syntax is identical on both VSTest and MTP. No changes needed. - -**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. Translate filters using xUnit.net v3's native options: - -| VSTest `--filter` syntax | xUnit.net v3 MTP equivalent | Notes | -|---|---|---| -| `FullyQualifiedName~ClassName` | `--filter-class *ClassName*` | Wildcards required for substring match | -| `FullyQualifiedName=Ns.Class.Method` | `--filter-method Ns.Class.Method` | Exact match on fully qualified method | -| `Name=MethodName` | `--filter-method *MethodName*` | Wildcards for substring match | -| `Category=Value` (trait) | `--filter-trait "Category=Value"` | Filter by trait name/value pair | -| Complex expressions | `--filter-query "expr"` | Uses xUnit.net query filter language (see above) | diff --git a/plugins/dotnet-test/skills/mtp-hot-reload/references/platform-detection.md b/plugins/dotnet-test/skills/mtp-hot-reload/references/platform-detection.md deleted file mode 100644 index 501113b638..0000000000 --- a/plugins/dotnet-test/skills/mtp-hot-reload/references/platform-detection.md +++ /dev/null @@ -1,53 +0,0 @@ -# Test Platform and Framework Detection - -Determine **which test platform** (VSTest or Microsoft.Testing.Platform) and **which test framework** (MSTest, xUnit, NUnit, TUnit) a project uses. - -**Detection files to always check** (in order): `global.json` → `.csproj` → `Directory.Build.props` → `Directory.Packages.props` - -## Detecting the test framework - -Read the `.csproj` file **and** `Directory.Build.props` / `Directory.Packages.props` (for centrally managed dependencies) and look for: - -| Package or SDK reference | Framework | -|--------------------------|-----------| -| `MSTest` (metapackage, recommended) or `` | MSTest | -| `MSTest.TestFramework` + `MSTest.TestAdapter` | MSTest (also valid for v3/v4) | -| `xunit`, `xunit.v3`, `xunit.v3.mtp-v1`, `xunit.v3.mtp-v2`, `xunit.v3.core.mtp-v1`, `xunit.v3.core.mtp-v2` | xUnit | -| `NUnit` + `NUnit3TestAdapter` | NUnit | -| `TUnit` | TUnit (MTP only) | - -## Detecting the test platform - -The detection logic depends on the .NET SDK version. Run `dotnet --version` to determine it. - -### .NET SDK 10+ - -On .NET 10+, the `global.json` `test.runner` setting is the **authoritative source**: - -- If `global.json` contains `"test": { "runner": "Microsoft.Testing.Platform" }` → **MTP** -- If `global.json` has `"runner": "VSTest"`, or no `test` section exists → **VSTest** - -> **Important**: On .NET 10+, `` alone does **not** switch to MTP. The `global.json` runner setting takes precedence. If the runner is VSTest (or unset), the project uses VSTest regardless of `TestingPlatformDotnetTestSupport`. - -### .NET SDK 8 or 9 - -On older SDKs, check these signals in priority order: - -**1. Check the `` MSBuild property.** Look in the `.csproj`, `Directory.Build.props`, **and** `Directory.Packages.props`. If set to `true` in **any** of these files, the project uses **MTP**. - -> **Critical**: Always read `Directory.Build.props` and `Directory.Packages.props` if they exist. MTP properties are frequently set there instead of in the `.csproj`, so checking only the project file will miss them. - -**2. Check project-level signals:** - -| Signal | Platform | -|--------|----------| -| `` as project SDK | **MTP** by default | -| `true` | **MTP** runner (xUnit) | -| `true` | **MTP** runner (MSTest) | -| `true` | **MTP** runner (NUnit) | -| `Microsoft.Testing.Platform` package referenced directly | **MTP** | -| `TUnit` package referenced | **MTP** (TUnit is MTP-only) | - -> **Note**: The presence of `Microsoft.NET.Test.Sdk` does **not** necessarily mean VSTest. Some frameworks (e.g., MSTest) pull it in transitively for compatibility, even when MTP is enabled. Do not use this package as a signal on its own — always check the MTP signals above first. - -> **Key distinction**: VSTest is the classic platform that uses `vstest.console` under the hood. Microsoft.Testing.Platform (MTP) is the newer, faster platform. Both can be invoked via `dotnet test`, but their filter syntax and CLI options differ. diff --git a/plugins/dotnet-test/skills/run-tests/SKILL.md b/plugins/dotnet-test/skills/run-tests/SKILL.md index e9bd6957dd..e2cfa2dd7e 100644 --- a/plugins/dotnet-test/skills/run-tests/SKILL.md +++ b/plugins/dotnet-test/skills/run-tests/SKILL.md @@ -56,7 +56,7 @@ Detect the test platform and framework, run tests, and apply filters using `dotn 1. Run `dotnet --version` to determine the .NET SDK version 2. Read `global.json`, `.csproj`, `Directory.Build.props`, and `Directory.Packages.props` -3. Follow the detection procedure in [references/platform-detection.md](references/platform-detection.md) to determine: +3. Follow the detection procedure in [platform-detection.md](../../shared/platform-detection.md) to determine: - **Test framework**: MSTest, xUnit, NUnit, or TUnit - **Test platform**: VSTest or Microsoft.Testing.Platform (MTP) @@ -169,7 +169,7 @@ These alternative invocations accept MTP command line arguments directly (no `-- ### Step 3: Run filtered tests -See [references/filter-syntax.md](references/filter-syntax.md) for the complete filter syntax for each platform and framework combination. Key points: +See [filter-syntax.md](../../shared/filter-syntax.md) for the complete filter syntax for each platform and framework combination. Key points: - **VSTest** (MSTest, xUnit v2, NUnit): `dotnet test --filter ` with `=`, `!=`, `~`, `!~` operators - **MTP -- MSTest and NUnit**: Same `--filter` syntax as VSTest; pass after `--` on SDK 8/9, directly on SDK 10+ diff --git a/plugins/dotnet-test/skills/run-tests/references/filter-syntax.md b/plugins/dotnet-test/skills/run-tests/references/filter-syntax.md deleted file mode 100644 index 03d23ec57d..0000000000 --- a/plugins/dotnet-test/skills/run-tests/references/filter-syntax.md +++ /dev/null @@ -1,166 +0,0 @@ -# Test Filter Syntax Reference - -Filter syntax depends on the **platform** and **test framework**. - -## VSTest filters (MSTest, xUnit v2, NUnit on VSTest) - -```bash -dotnet test --filter -``` - -Expression syntax: `[|&]` - -**Operators:** - -| Operator | Meaning | -|----------|---------| -| `=` | Exact match | -| `!=` | Not exact match | -| `~` | Contains | -| `!~` | Does not contain | - -**Combinators:** `|` (OR), `&` (AND). Parentheses for grouping: `(A|B)&C` - -**Supported properties by framework:** - -| Framework | Properties | -|-----------|-----------| -| MSTest | `FullyQualifiedName`, `Name`, `ClassName`, `Priority`, `TestCategory` | -| xUnit | `FullyQualifiedName`, `DisplayName`, `Traits` | -| NUnit | `FullyQualifiedName`, `Name`, `Priority`, `TestCategory` | - -An expression without an operator is treated as `FullyQualifiedName~`. - -**Examples (VSTest):** - -```bash -# Run tests whose name contains "LoginTest" -dotnet test --filter "Name~LoginTest" - -# Run a specific test class -dotnet test --filter "ClassName=MyNamespace.MyTestClass" - -# Run tests in a category -dotnet test --filter "TestCategory=Integration" - -# Exclude a category -dotnet test --filter "TestCategory!=Slow" - -# Combine: class AND category -dotnet test --filter "ClassName=MyNamespace.MyTestClass&TestCategory=Unit" - -# Either of two classes -dotnet test --filter "ClassName=MyNamespace.ClassA|ClassName=MyNamespace.ClassB" -``` - -## MTP filters — MSTest and NUnit - -MSTest and NUnit on MTP use the **same `--filter` syntax** as VSTest (same properties, operators, and combinators). The only difference is how the flag is passed: - -```bash -# .NET SDK 8/9 (after --) -dotnet test -- --filter "Name~LoginTest" - -# .NET SDK 10+ (direct) -dotnet test --filter "Name~LoginTest" -``` - -## MTP filters — xUnit (v3) - -xUnit v3 on MTP uses **framework-specific filter flags** instead of the generic `--filter` expression: - -| Flag | Description | -|------|-------------| -| `--filter-class "name"` | Run all tests in a given class | -| `--filter-not-class "name"` | Exclude all tests in a given class | -| `--filter-method "name"` | Run a specific test method | -| `--filter-not-method "name"` | Exclude a specific test method | -| `--filter-namespace "name"` | Run all tests in a namespace | -| `--filter-not-namespace "name"` | Exclude all tests in a namespace | -| `--filter-trait "name=value"` | Run tests with a matching trait | -| `--filter-not-trait "name=value"` | Exclude tests with a matching trait | - -Multiple values can be specified with a single flag: `--filter-class Foo Bar`. - -```bash -# .NET SDK 8/9 -dotnet test -- --filter-class "MyNamespace.LoginTests" - -# .NET SDK 10+ -dotnet test --filter-class "MyNamespace.LoginTests" - -# Combine: namespace + trait -dotnet test --filter-namespace "MyApp.Tests.Integration" --filter-trait "Category=Smoke" -``` - -### xUnit v3 query filter language - -For complex expressions, use `--filter-query` with a path-segment syntax: - -``` -////[traitName=traitValue] -``` - -Each segment matches against: assembly name, namespace, class name, method name. Use `*` for "match all" in any segment. Documentation: https://xunit.net/docs/query-filter-language - -```shell -# xUnit.net v3 MTP — using query language (assembly/namespace/class/method[trait]) -dotnet test -- --filter-query "/*/*/*IntegrationTests*/*[Category=Smoke]" -``` - -## MTP filters — TUnit - -TUnit uses `--treenode-filter` with a path-based syntax: - -``` ---treenode-filter "////" -``` - -Wildcards (`*`) are supported in any segment. Filter operators can be appended to test names for property-based filtering. - -| Operator | Meaning | -|----------|---------| -| `*` | Wildcard match | -| `=` | Exact property match (e.g., `[Category=Unit]`) | -| `!=` | Exclude property value | -| `&` | AND (combine conditions) | -| `\|` | OR (within a segment, requires parentheses) | - -**Examples (TUnit):** - -```bash -# All tests in a class -dotnet run --treenode-filter "/*/*/LoginTests/*" - -# A specific test -dotnet run --treenode-filter "/*/*/*/AcceptCookiesTest" - -# By namespace prefix (wildcard) -dotnet run --treenode-filter "/*/MyProject.Tests.Api*/*/*" - -# By custom property -dotnet run --treenode-filter "/*/*/*/*[Category=Smoke]" - -# Exclude by property -dotnet run --treenode-filter "/*/*/*/*[Category!=Slow]" - -# OR across classes -dotnet run --treenode-filter "/*/*/(LoginTests)|(SignupTests)/*" - -# Combined: namespace + property -dotnet run --treenode-filter "/*/MyProject.Tests.Integration/*/*/*[Priority=Critical]" -``` - -## VSTest → MTP filter translation (for migration) - -**MSTest, NUnit, and xUnit.net v2 (with `YTest.MTP.XUnit2`)**: The VSTest `--filter` syntax is identical on both VSTest and MTP. No changes needed. - -**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. Translate filters using xUnit.net v3's native options: - -| VSTest `--filter` syntax | xUnit.net v3 MTP equivalent | Notes | -|---|---|---| -| `FullyQualifiedName~ClassName` | `--filter-class *ClassName*` | Wildcards required for substring match | -| `FullyQualifiedName=Ns.Class.Method` | `--filter-method Ns.Class.Method` | Exact match on fully qualified method | -| `Name=MethodName` | `--filter-method *MethodName*` | Wildcards for substring match | -| `Category=Value` (trait) | `--filter-trait "Category=Value"` | Filter by trait name/value pair | -| Complex expressions | `--filter-query "expr"` | Uses xUnit.net query filter language (see above) | diff --git a/plugins/dotnet-test/skills/run-tests/references/platform-detection.md b/plugins/dotnet-test/skills/run-tests/references/platform-detection.md deleted file mode 100644 index 501113b638..0000000000 --- a/plugins/dotnet-test/skills/run-tests/references/platform-detection.md +++ /dev/null @@ -1,53 +0,0 @@ -# Test Platform and Framework Detection - -Determine **which test platform** (VSTest or Microsoft.Testing.Platform) and **which test framework** (MSTest, xUnit, NUnit, TUnit) a project uses. - -**Detection files to always check** (in order): `global.json` → `.csproj` → `Directory.Build.props` → `Directory.Packages.props` - -## Detecting the test framework - -Read the `.csproj` file **and** `Directory.Build.props` / `Directory.Packages.props` (for centrally managed dependencies) and look for: - -| Package or SDK reference | Framework | -|--------------------------|-----------| -| `MSTest` (metapackage, recommended) or `` | MSTest | -| `MSTest.TestFramework` + `MSTest.TestAdapter` | MSTest (also valid for v3/v4) | -| `xunit`, `xunit.v3`, `xunit.v3.mtp-v1`, `xunit.v3.mtp-v2`, `xunit.v3.core.mtp-v1`, `xunit.v3.core.mtp-v2` | xUnit | -| `NUnit` + `NUnit3TestAdapter` | NUnit | -| `TUnit` | TUnit (MTP only) | - -## Detecting the test platform - -The detection logic depends on the .NET SDK version. Run `dotnet --version` to determine it. - -### .NET SDK 10+ - -On .NET 10+, the `global.json` `test.runner` setting is the **authoritative source**: - -- If `global.json` contains `"test": { "runner": "Microsoft.Testing.Platform" }` → **MTP** -- If `global.json` has `"runner": "VSTest"`, or no `test` section exists → **VSTest** - -> **Important**: On .NET 10+, `` alone does **not** switch to MTP. The `global.json` runner setting takes precedence. If the runner is VSTest (or unset), the project uses VSTest regardless of `TestingPlatformDotnetTestSupport`. - -### .NET SDK 8 or 9 - -On older SDKs, check these signals in priority order: - -**1. Check the `` MSBuild property.** Look in the `.csproj`, `Directory.Build.props`, **and** `Directory.Packages.props`. If set to `true` in **any** of these files, the project uses **MTP**. - -> **Critical**: Always read `Directory.Build.props` and `Directory.Packages.props` if they exist. MTP properties are frequently set there instead of in the `.csproj`, so checking only the project file will miss them. - -**2. Check project-level signals:** - -| Signal | Platform | -|--------|----------| -| `` as project SDK | **MTP** by default | -| `true` | **MTP** runner (xUnit) | -| `true` | **MTP** runner (MSTest) | -| `true` | **MTP** runner (NUnit) | -| `Microsoft.Testing.Platform` package referenced directly | **MTP** | -| `TUnit` package referenced | **MTP** (TUnit is MTP-only) | - -> **Note**: The presence of `Microsoft.NET.Test.Sdk` does **not** necessarily mean VSTest. Some frameworks (e.g., MSTest) pull it in transitively for compatibility, even when MTP is enabled. Do not use this package as a signal on its own — always check the MTP signals above first. - -> **Key distinction**: VSTest is the classic platform that uses `vstest.console` under the hood. Microsoft.Testing.Platform (MTP) is the newer, faster platform. Both can be invoked via `dotnet test`, but their filter syntax and CLI options differ. diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index 8c4e9d9fa9..b273c89517 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -1,11 +1,11 @@ --- name: test-anti-patterns -description: "Detects anti-patterns and code smells in .NET test suites. Use when the user asks to review test quality, find test smells, identify flaky test indicators, or audit tests for common mistakes. Covers assertion quality, test isolation, naming, flakiness indicators, over-mocking, and structural problems. Works with MSTest, xUnit, NUnit, and TUnit." +description: "Quick pragmatic review of .NET test code for anti-patterns that undermine reliability and diagnostic value. Catches the most impactful issues — assertion gaps, flakiness indicators, over-mocking, naming, and structural problems — with actionable fixes. Use for periodic test code reviews and PR feedback. For a deep formal audit based on academic test smell taxonomy, use exp-test-smell-detection instead. Works with MSTest, xUnit, NUnit, and TUnit." --- # Test Anti-Pattern Detection -Analyze .NET test code for anti-patterns, code smells, and quality issues that undermine test reliability, maintainability, and diagnostic value. +Quick, pragmatic analysis of .NET test code for anti-patterns and quality issues that undermine test reliability, maintainability, and diagnostic value. ## When to Use @@ -21,6 +21,7 @@ Analyze .NET test code for anti-patterns, code smells, and quality issues that u - User wants to run or execute tests (use `run-tests`) - User wants to migrate between test frameworks or versions (use migration skills) - User wants to measure code coverage (out of scope) +- User wants a deep formal test smell audit with academic taxonomy and extended catalog (use `exp-test-smell-detection`) ## Inputs @@ -34,7 +35,7 @@ Analyze .NET test code for anti-patterns, code smells, and quality issues that u ### Step 1: Gather the test code -Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files (files containing `[TestClass]`, `[TestMethod]`, `[Fact]`, `[Test]`, or `[Theory]` attributes). +Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers (e.g., `[TestClass]`, `[Fact]`, `[Test]`). If production code is available, read it too -- this is critical for detecting tests that are coupled to implementation details rather than behavior. From 9c3e74999d7bb4b97b59ca5b330087f25e098489 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 14:16:06 +0200 Subject: [PATCH 2/7] Switch from shared/ directories to hidden reference skills MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the plugin-level shared/ directories with non-invocable reference skills (user-invocable: false) that other skills reference by name. - Create platform-detection, filter-syntax, and dotnet-test-frameworks as hidden skills under plugins/dotnet-test/skills/. These contain the detection tables and syntax references previously duplicated across run-tests, mtp-hot-reload, and migrate-vstest-to-mtp. - Create exp-dotnet-test-frameworks as a hidden skill under plugins/dotnet-experimental/skills/ for the experimental test analysis skills (exp-test-smell-detection, exp-assertion-quality, etc.). - Update all consuming skills to reference these by skill name in backtick notation instead of file links. - Revert the skill-validator ../../shared/ exception — no longer needed since all references now use the standard skill name mechanism. --- .../src/Check/SkillProfiler.cs | 24 ++----------------- .../tests/Check/SkillProfileTests.cs | 24 ------------------- .../skills/exp-assertion-quality/SKILL.md | 2 +- .../exp-dotnet-test-frameworks/SKILL.md} | 8 ++++++- .../exp-test-boilerplate-detection/SKILL.md | 2 +- .../skills/exp-test-smell-detection/SKILL.md | 8 +++---- .../skills/exp-test-tagging/SKILL.md | 2 +- .../dotnet-test-frameworks/SKILL.md} | 8 ++++++- .../filter-syntax/SKILL.md} | 6 +++++ .../skills/migrate-vstest-to-mtp/SKILL.md | 4 ++-- .../skills/mtp-hot-reload/SKILL.md | 4 ++-- .../platform-detection/SKILL.md} | 6 +++++ plugins/dotnet-test/skills/run-tests/SKILL.md | 4 ++-- .../skills/test-anti-patterns/SKILL.md | 2 +- 14 files changed, 42 insertions(+), 62 deletions(-) rename plugins/dotnet-experimental/{shared/dotnet-test-frameworks.md => skills/exp-dotnet-test-frameworks/SKILL.md} (90%) rename plugins/dotnet-test/{shared/dotnet-test-frameworks.md => skills/dotnet-test-frameworks/SKILL.md} (90%) rename plugins/dotnet-test/{shared/filter-syntax.md => skills/filter-syntax/SKILL.md} (93%) rename plugins/dotnet-test/{shared/platform-detection.md => skills/platform-detection/SKILL.md} (90%) diff --git a/eng/skill-validator/src/Check/SkillProfiler.cs b/eng/skill-validator/src/Check/SkillProfiler.cs index 564c198c58..6fa05010a7 100644 --- a/eng/skill-validator/src/Check/SkillProfiler.cs +++ b/eng/skill-validator/src/Check/SkillProfiler.cs @@ -127,30 +127,10 @@ public static SkillProfile AnalyzeSkill(SkillInfo skill) var segments = refPath.Split('/'); - // Allow parent-directory traversals only when resolving to a plugin-level shared/ directory. - // Skills live at plugins//skills//, so ../../shared/ resolves to - // the plugin's shared/ directory. This enables deduplication of reference files that - // multiple skills within the same plugin need (e.g., platform-detection.md). + // Reject parent-directory traversals if (segments.Any(s => s == "..")) { - bool isAllowedSharedRef = - segments.Length >= 4 && - segments[0] == ".." && segments[1] == ".." && segments[2] == "shared" && - segments.Take(2).All(s => s == "..") && - !segments.Skip(2).Any(s => s == ".."); - - if (!isAllowedSharedRef) - { - errors.Add($"File reference '{refMatch.Groups[1].Value}' uses parent-directory traversal — references must stay within the skill directory or use ../../shared/."); - continue; - } - - // Depth inside shared/ (exclude the "../../shared" prefix and the filename) - int sharedDirDepth = segments.Length - 4; // segments: [.., .., shared, , filename] - if (sharedDirDepth > 0) - { - errors.Add($"File reference '{refMatch.Groups[1].Value}' is {sharedDirDepth + 1} directories deep inside shared/ — files must be directly inside shared/."); - } + errors.Add($"File reference '{refMatch.Groups[1].Value}' uses parent-directory traversal — references must stay within the skill directory."); continue; } diff --git a/eng/skill-validator/tests/Check/SkillProfileTests.cs b/eng/skill-validator/tests/Check/SkillProfileTests.cs index 09109a4093..eb73d562c9 100644 --- a/eng/skill-validator/tests/Check/SkillProfileTests.cs +++ b/eng/skill-validator/tests/Check/SkillProfileTests.cs @@ -322,30 +322,6 @@ public void ParentDirectoryTraversalErrors() Assert.Contains(profile.Errors, e => e.Contains("parent-directory traversal")); } - [Fact] - public void SharedDirectoryTraversalAllowed() - { - var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../shared/platform-detection.md)\n" + new string('x', 4000); - var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); - Assert.DoesNotContain(profile.Errors, e => e.Contains("parent-directory traversal") || e.Contains("shared/")); - } - - [Fact] - public void SharedDirectoryDeepTraversalErrors() - { - var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../shared/sub/file.md)\n" + new string('x', 4000); - var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); - Assert.Contains(profile.Errors, e => e.Contains("inside shared/")); - } - - [Fact] - public void NonSharedParentTraversalStillErrors() - { - var content = "---\nname: test-skill\n---\n# Title\n1. Step\n```bash\necho\n```\nSee [ref](../../other/file.md)\n" + new string('x', 4000); - var profile = SkillProfiler.AnalyzeSkill(MakeSkill(content)); - Assert.Contains(profile.Errors, e => e.Contains("parent-directory traversal")); - } - [Fact] public void AnchorFragmentStrippedFromDepthCheck() { diff --git a/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md b/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md index 5bb05cd2d3..c9ee48f598 100644 --- a/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-assertion-quality/SKILL.md @@ -46,7 +46,7 @@ Low assertion diversity signals shallow testing. Tests may pass while bugs hide ### Step 1: Gather the test code -Read all test files the user provides. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers. +Read all test files the user provides. If the user points to a directory or project, scan for all test files — see the `exp-dotnet-test-frameworks` skill for framework-specific markers. ### Step 2: Classify every assertion diff --git a/plugins/dotnet-experimental/shared/dotnet-test-frameworks.md b/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md similarity index 90% rename from plugins/dotnet-experimental/shared/dotnet-test-frameworks.md rename to plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md index 7ebaa9c839..83a72859e8 100644 --- a/plugins/dotnet-experimental/shared/dotnet-test-frameworks.md +++ b/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md @@ -1,4 +1,10 @@ -# .NET Extension +--- +name: exp-dotnet-test-frameworks +description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (exp-test-smell-detection, exp-assertion-quality, exp-test-boilerplate-detection, exp-test-tagging) when they need framework-specific lookup tables." +user-invocable: false +--- + +# .NET Test Framework Reference Language-specific detection patterns for .NET test frameworks (MSTest, xUnit, NUnit, TUnit). diff --git a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md index b7cfec04bb..2574b50eca 100644 --- a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md @@ -34,7 +34,7 @@ Analyze .NET test code to find duplicated boilerplate patterns across test metho ### Step 1: Gather the test code -Read all test files the user provides or references. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers. +Read all test files the user provides or references. If the user points to a directory or project, scan for all test files — see the `exp-dotnet-test-frameworks` skill for framework-specific markers. ### Step 2: Identify boilerplate categories diff --git a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md index b18499ae60..9e4164e1da 100644 --- a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md @@ -49,7 +49,7 @@ Test smells erode confidence in a test suite and inflate maintenance costs: ### Step 1: Gather the test code -Read all test files the user provides. If the user points to a directory or project, scan for all test files by looking for test framework markers — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET-specific markers. +Read all test files the user provides. If the user points to a directory or project, scan for all test files by looking for test framework markers — see the `exp-dotnet-test-frameworks` skill for .NET-specific markers. For a thorough audit, also consult the [extended smell catalog](references/test-smell-catalog.md) which covers 9 additional smell types beyond the core 10 below. @@ -78,7 +78,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 -**Detection:** Calls to sleep/delay functions inside test methods. See [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET-specific patterns. +**Detection:** Calls to sleep/delay functions inside test methods. See the `exp-dotnet-test-frameworks` skill for .NET-specific patterns. #### Smell 4: Assertion-Free Test (Unknown Test) @@ -131,7 +131,7 @@ The test setup method or constructor initializes fields that are not used by eve Tests marked as skipped or disabled. These add overhead and clutter, and the underlying issue they were disabled for may never be addressed. **Severity:** Low -**Detection:** Skip/ignore annotations or conditional compilation that disables a test. See [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific skip attributes. +**Detection:** Skip/ignore annotations or conditional compilation that disables a test. See the `exp-dotnet-test-frameworks` skill for framework-specific skip attributes. ### Step 3: Apply calibration rules @@ -191,7 +191,7 @@ Present the analysis in this structure: | Flagging integration tests for using real resources | Check for integration test markers and adjust severity accordingly | | Flagging loop-over-collection-assert as conditional logic | Only flag loops with branching or complex logic, not assertion iterations | | Flagging obvious count assertions after adding N items | Consider the immediate context — self-documenting numbers are fine | -| Missing framework-specific assertion syntax | Consult [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for .NET framework assertion and skip APIs | +| Missing framework-specific assertion syntax | Consult the `exp-dotnet-test-frameworks` skill for .NET framework assertion and skip APIs | | Over-flagging try/catch that captures for assertion | Distinguish swallowed exceptions from capture-and-assert patterns | | Treating skip annotations with reasons same as bare skips | Note that reasoned skips are less concerning than unexplained ones | | Flagging `DoesNotThrow`-style tests as assertion-free | These implicitly assert no exception — note but acknowledge the intent | diff --git a/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md index 4a1cfa20db..47047ba0d6 100644 --- a/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-tagging/SKILL.md @@ -56,7 +56,7 @@ A single test may have **multiple traits** (e.g., both `negative` and `boundary` ### Step 1: Detect the test framework -Examine project files and source code to determine the framework — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for the complete detection table (package references, test markers, assertion APIs, and skip annotations). +Examine project files and source code to determine the framework — see the `exp-dotnet-test-frameworks` skill for the complete detection table (package references, test markers, assertion APIs, and skip annotations). ### Step 2: Scan existing traits diff --git a/plugins/dotnet-test/shared/dotnet-test-frameworks.md b/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md similarity index 90% rename from plugins/dotnet-test/shared/dotnet-test-frameworks.md rename to plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md index 7ebaa9c839..f0749e727b 100644 --- a/plugins/dotnet-test/shared/dotnet-test-frameworks.md +++ b/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md @@ -1,4 +1,10 @@ -# .NET Extension +--- +name: dotnet-test-frameworks +description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (test-anti-patterns, exp-test-smell-detection, exp-assertion-quality, exp-test-boilerplate-detection, exp-test-tagging) when they need framework-specific lookup tables." +user-invocable: false +--- + +# .NET Test Framework Reference Language-specific detection patterns for .NET test frameworks (MSTest, xUnit, NUnit, TUnit). diff --git a/plugins/dotnet-test/shared/filter-syntax.md b/plugins/dotnet-test/skills/filter-syntax/SKILL.md similarity index 93% rename from plugins/dotnet-test/shared/filter-syntax.md rename to plugins/dotnet-test/skills/filter-syntax/SKILL.md index 03d23ec57d..4de20defbd 100644 --- a/plugins/dotnet-test/shared/filter-syntax.md +++ b/plugins/dotnet-test/skills/filter-syntax/SKILL.md @@ -1,3 +1,9 @@ +--- +name: filter-syntax +description: "Reference data for test filter syntax across all platform and framework combinations: VSTest --filter expressions, MTP filters for MSTest/NUnit/xUnit v3/TUnit, and VSTest-to-MTP filter translation. DO NOT USE directly — loaded by run-tests, mtp-hot-reload, and migrate-vstest-to-mtp when they need filter syntax." +user-invocable: false +--- + # Test Filter Syntax Reference Filter syntax depends on the **platform** and **test framework**. diff --git a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md b/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md index 48eab919cc..e700207841 100644 --- a/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md +++ b/plugins/dotnet-test/skills/migrate-vstest-to-mtp/SKILL.md @@ -52,7 +52,7 @@ Migrate a .NET test solution from VSTest to Microsoft.Testing.Platform (MTP). Th ### Step 1: Assess the solution -1. Identify the test framework for each test project -- see [platform-detection.md](../../shared/platform-detection.md) for the package-to-framework mapping. Key indicators: +1. Identify the test framework for each test project -- see the `platform-detection` skill for the package-to-framework mapping. Key indicators: - **MSTest**: References `MSTest` or `MSTest.TestAdapter`, or uses `MSTest.Sdk` (with `` not set to `false`). Note: `MSTest.TestFramework` alone is a library dependency, not a test project. - **NUnit**: References `NUnit3TestAdapter` - **xUnit.net**: References `xunit` and `xunit.runner.visualstudio` @@ -197,7 +197,7 @@ VSTest-specific arguments must be translated to MTP equivalents. Build-related a **MSTest, NUnit, and xUnit.net v2 (with `YTest.MTP.XUnit2`)**: The VSTest `--filter` syntax is identical on both VSTest and MTP. No changes needed. -**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. See the **VSTest -> MTP filter translation** section in [filter-syntax.md](../../shared/filter-syntax.md) for the complete translation table. Key translation example: +**xUnit.net v3 (native MTP)**: xUnit.net v3 does NOT support the VSTest `--filter` syntax on MTP. See the **VSTest → MTP filter translation** section in the `filter-syntax` skill for the complete translation table. Key translation example: ```shell # VSTest diff --git a/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md b/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md index 38c8f6e417..3964535484 100644 --- a/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md +++ b/plugins/dotnet-test/skills/mtp-hot-reload/SKILL.md @@ -44,7 +44,7 @@ Set up and use Microsoft Testing Platform hot reload to rapidly iterate fixes on Hot reload requires MTP. It does **not** work with VSTest. -Follow the detection procedure in [platform-detection.md](../../shared/platform-detection.md) to determine the test platform. +Follow the detection procedure in the `platform-detection` skill to determine the test platform. If the project uses VSTest, inform the user that MTP hot reload is not available and suggest migrating to MTP first (see `migrate-vstest-to-mtp`), or using Visual Studio's built-in Test Explorer hot reload feature instead. @@ -97,7 +97,7 @@ Run the test project directly (not through `dotnet test`) to use hot reload in c dotnet run --project ``` -To filter to specific failing tests, pass the filter after `--`. The syntax depends on the test framework -- see [filter-syntax.md](../../shared/filter-syntax.md) for full details. Quick examples: +To filter to specific failing tests, pass the filter after `--`. The syntax depends on the test framework -- see the `filter-syntax` skill for full details. Quick examples: | Framework | Filter syntax | |-----------|--------------| diff --git a/plugins/dotnet-test/shared/platform-detection.md b/plugins/dotnet-test/skills/platform-detection/SKILL.md similarity index 90% rename from plugins/dotnet-test/shared/platform-detection.md rename to plugins/dotnet-test/skills/platform-detection/SKILL.md index 501113b638..10f1a24fab 100644 --- a/plugins/dotnet-test/shared/platform-detection.md +++ b/plugins/dotnet-test/skills/platform-detection/SKILL.md @@ -1,3 +1,9 @@ +--- +name: platform-detection +description: "Reference data for detecting the test platform (VSTest vs Microsoft.Testing.Platform) and test framework (MSTest, xUnit, NUnit, TUnit) from project files. DO NOT USE directly — loaded by run-tests, mtp-hot-reload, and migrate-vstest-to-mtp when they need detection logic." +user-invocable: false +--- + # Test Platform and Framework Detection Determine **which test platform** (VSTest or Microsoft.Testing.Platform) and **which test framework** (MSTest, xUnit, NUnit, TUnit) a project uses. diff --git a/plugins/dotnet-test/skills/run-tests/SKILL.md b/plugins/dotnet-test/skills/run-tests/SKILL.md index e2cfa2dd7e..91c2914bf8 100644 --- a/plugins/dotnet-test/skills/run-tests/SKILL.md +++ b/plugins/dotnet-test/skills/run-tests/SKILL.md @@ -56,7 +56,7 @@ Detect the test platform and framework, run tests, and apply filters using `dotn 1. Run `dotnet --version` to determine the .NET SDK version 2. Read `global.json`, `.csproj`, `Directory.Build.props`, and `Directory.Packages.props` -3. Follow the detection procedure in [platform-detection.md](../../shared/platform-detection.md) to determine: +3. Follow the detection procedure in the `platform-detection` skill to determine: - **Test framework**: MSTest, xUnit, NUnit, or TUnit - **Test platform**: VSTest or Microsoft.Testing.Platform (MTP) @@ -169,7 +169,7 @@ These alternative invocations accept MTP command line arguments directly (no `-- ### Step 3: Run filtered tests -See [filter-syntax.md](../../shared/filter-syntax.md) for the complete filter syntax for each platform and framework combination. Key points: +See the `filter-syntax` skill for the complete filter syntax for each platform and framework combination. Key points: - **VSTest** (MSTest, xUnit v2, NUnit): `dotnet test --filter ` with `=`, `!=`, `~`, `!~` operators - **MTP -- MSTest and NUnit**: Same `--filter` syntax as VSTest; pass after `--` on SDK 8/9, directly on SDK 10+ diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index b273c89517..ec44a27d29 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -35,7 +35,7 @@ Quick, pragmatic analysis of .NET test code for anti-patterns and quality issues ### Step 1: Gather the test code -Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files — see [dotnet-test-frameworks.md](../../shared/dotnet-test-frameworks.md) for framework-specific markers (e.g., `[TestClass]`, `[Fact]`, `[Test]`). +Read the test files the user wants reviewed. If the user points to a directory or project, scan for all test files using the framework-specific markers in the `dotnet-test-frameworks` skill (e.g., `[TestClass]`, `[Fact]`, `[Test]`). If production code is available, read it too -- this is critical for detecting tests that are coupled to implementation details rather than behavior. From 63741d2a85238107ba196f0bf094015ec568fef1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 14:44:28 +0200 Subject: [PATCH 3/7] Merge exp-test-boilerplate-detection into exp-test-maintainability exp-test-maintainability was only 6 calibration rules with no workflow. exp-test-boilerplate-detection had the full 5-category detection workflow, examples, calibration, and validation. Merge the boilerplate content into exp-test-maintainability (the broader, more user-facing name) and add the two unique maintainability rules (DisplayName guidance, DataRow vs DynamicData preference) to Category 3. - Replace exp-test-maintainability SKILL.md with the merged content - Move test fixtures from exp-test-boilerplate-detection to exp-test-maintainability - Merge eval.yaml scenarios (4 total: 2 from each original skill) - Delete exp-test-boilerplate-detection skill and tests - Update all cross-references in exp-test-smell-detection, dotnet-test-frameworks, exp-dotnet-test-frameworks, and CODEOWNERS --- .github/CODEOWNERS | 3 - .../exp-dotnet-test-frameworks/SKILL.md | 2 +- .../exp-test-boilerplate-detection/SKILL.md | 193 ----------------- .../skills/exp-test-maintainability/SKILL.md | 197 +++++++++++++++++- .../skills/exp-test-smell-detection/SKILL.md | 4 +- .../skills/dotnet-test-frameworks/SKILL.md | 2 +- .../exp-test-boilerplate-detection/eval.yaml | 90 -------- .../exp-test-maintainability/eval.yaml | 56 +++++ .../OrderService.Tests/OrderProcessorTests.cs | 0 .../OrderService.Tests.csproj | 0 .../Calculator.Tests/Calculator.Tests.csproj | 0 .../Calculator.Tests/CalculatorTests.cs | 0 12 files changed, 247 insertions(+), 300 deletions(-) delete mode 100644 plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md delete mode 100644 tests/dotnet-experimental/exp-test-boilerplate-detection/eval.yaml rename tests/dotnet-experimental/{exp-test-boilerplate-detection => exp-test-maintainability}/fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs (100%) rename tests/dotnet-experimental/{exp-test-boilerplate-detection => exp-test-maintainability}/fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj (100%) rename tests/dotnet-experimental/{exp-test-boilerplate-detection => exp-test-maintainability}/fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj (100%) rename tests/dotnet-experimental/{exp-test-boilerplate-detection => exp-test-maintainability}/fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs (100%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a9a17c4ef5..20d9ade9a9 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -101,9 +101,6 @@ /plugins/dotnet-experimental/skills/exp-test-maintainability/ @dotnet/dotnet-testing /tests/dotnet-experimental/exp-test-maintainability/ @dotnet/dotnet-testing -/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/ @dotnet/dotnet-testing -/tests/dotnet-experimental/exp-test-boilerplate-detection/ @dotnet/dotnet-testing - /plugins/dotnet-experimental/skills/exp-assertion-quality/ @dotnet/dotnet-testing /tests/dotnet-experimental/exp-assertion-quality/ @dotnet/dotnet-testing diff --git a/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md b/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md index 83a72859e8..3c5802922a 100644 --- a/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/SKILL.md @@ -1,6 +1,6 @@ --- name: exp-dotnet-test-frameworks -description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (exp-test-smell-detection, exp-assertion-quality, exp-test-boilerplate-detection, exp-test-tagging) when they need framework-specific lookup tables." +description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (exp-test-smell-detection, exp-assertion-quality, exp-test-maintainability, exp-test-tagging) when they need framework-specific lookup tables." user-invocable: false --- diff --git a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md deleted file mode 100644 index 2574b50eca..0000000000 --- a/plugins/dotnet-experimental/skills/exp-test-boilerplate-detection/SKILL.md +++ /dev/null @@ -1,193 +0,0 @@ ---- -name: exp-test-boilerplate-detection -description: "Detects duplicate boilerplate patterns across .NET test suites and identifies refactoring opportunities. Use when the user asks to find repeated code in tests, reduce test boilerplate, identify shared setup patterns, or discover refactoring opportunities across test classes. Does NOT modify code — produces an analysis report with actionable suggestions. Works with MSTest, xUnit, NUnit, and TUnit. DO NOT USE FOR: writing new tests (use writing-mstest-tests), reviewing test quality or anti-patterns (use test-anti-patterns), or assessing overall test maintainability (use exp-test-maintainability)." ---- - -# Test Boilerplate Detection - -Analyze .NET test code to find duplicated boilerplate patterns across test methods and classes. Produce a report of refactoring opportunities that would reduce repetition and improve maintainability. The goal is analysis only — do not modify any files. - -## When to Use - -- User asks to find duplicated code or boilerplate in tests -- User wants to know where test code can be DRY-ed up -- User asks for refactoring opportunities in a test suite -- User wants to identify shared setup or teardown candidates -- User asks "what patterns repeat across my tests?" - -## When Not to Use - -- User wants to write new tests from scratch (use `writing-mstest-tests`) -- User wants to detect anti-patterns or code smells (use `test-anti-patterns`) -- User wants to assess overall test maintainability (use `exp-test-maintainability`) -- User wants to actually perform the refactoring (help them directly, this skill only analyzes) - -## Inputs - -| Input | Required | Description | -|-------|----------|-------------| -| Test code | Yes | One or more test files or a test project directory to analyze | -| Production code | No | The code under test, for context on what abstractions might help | -| Scope | No | Whether to analyze within a single class or across multiple classes | - -## Workflow - -### Step 1: Gather the test code - -Read all test files the user provides or references. If the user points to a directory or project, scan for all test files — see the `exp-dotnet-test-frameworks` skill for framework-specific markers. - -### Step 2: Identify boilerplate categories - -Scan for these categories of duplication: - -#### Category 1: Repeated object construction - -Look for the same object being constructed in 3+ test methods with identical or near-identical parameters. - -**Indicators:** -- `new ClassName(...)` appearing with identical arguments in multiple tests -- Multiple tests creating the same "system under test" with similar configuration -- Repeated mock/fake/stub creation with the same setup - -**Potential refactorings:** -- Extract a factory method or test helper (e.g., `CreateSut()`, `CreateDefaultOrder()`) -- Use `[TestInitialize]`/constructor/`[SetUp]` for shared construction -- Introduce a builder pattern for complex objects with many variations - -**Example — before:** -```csharp -[TestMethod] -public void Process_ValidOrder_Succeeds() -{ - var logger = new FakeLogger(); - var email = new FakeEmailService(); - var inventory = new FakeInventory(stock: 100); - var processor = new OrderProcessor(logger, email, inventory); - // ... -} - -[TestMethod] -public void Process_EmptyItems_Fails() -{ - var logger = new FakeLogger(); - var email = new FakeEmailService(); - var inventory = new FakeInventory(stock: 100); - var processor = new OrderProcessor(logger, email, inventory); - // ... -} -``` - -**After — extract factory:** -```csharp -private static OrderProcessor CreateProcessor(int stock = 100) -{ - return new OrderProcessor(new FakeLogger(), new FakeEmailService(), new FakeInventory(stock)); -} -``` - -#### Category 2: Repeated assertion patterns - -Look for the same sequence of assertions appearing in 3+ test methods. - -**Indicators:** -- Multiple tests asserting the same set of properties on a result object -- Repeated null-check-then-value-check sequences -- Same collection of `Assert.AreEqual` calls across methods - -**Potential refactorings:** -- Extract a custom assertion helper (e.g., `AssertValidOrder(order, expectedTotal, expectedStatus)`) -- Use framework-specific assertion extensions -- Introduce a `Verify` method that checks a standard set of properties - -#### Category 3: Copy-paste test methods - -Look for test methods with near-identical bodies differing only in input values or a single parameter. - -**Indicators:** -- 3+ methods with the same structure but different literal values -- Methods that could be collapsed into `[DataRow]`/`[Theory]`/`[TestCase]` -- Test names that follow a pattern like `Method_Input1_Result`, `Method_Input2_Result` - -**Potential refactorings:** -- Convert to parameterized tests with `[DataRow]`/`[InlineData]`/`[TestCase]` -- Use `[DynamicData]`/`[MemberData]`/`[TestCaseSource]` for complex inputs - -#### Category 4: Duplicated setup/teardown logic - -Look for initialization or cleanup code repeated across test classes. - -**Indicators:** -- Multiple `[TestInitialize]`/`[SetUp]` methods with similar bodies -- Repeated database seeding, file creation, or HTTP client configuration -- Same `using`/`IDisposable` cleanup pattern across classes - -**Potential refactorings:** -- Extract a shared test base class or fixture -- Use composition with a shared helper class -- Create a test context factory - -#### Category 5: Repeated test infrastructure - -Look for structural patterns shared across test classes. - -**Indicators:** -- Same mock interfaces configured identically in multiple classes -- Repeated `HttpClient` setup with similar `DelegatingHandler` patterns -- Same logging/configuration scaffolding across test classes - -**Potential refactorings:** -- Extract a shared test fixture or helper library -- Create reusable fake implementations -- Introduce a test harness class - -### Step 3: Apply calibration rules - -Before reporting, filter findings through these rules: - -- **Only report at 3+ occurrences.** Two similar setups are not boilerplate — they may be intentional clarity. -- **Don't flag simple constructors.** `new Calculator()` or `new List()` is not meaningful boilerplate. -- **Respect intentional verbosity.** If each test is self-contained and reads clearly on its own, explicit setup per test is a valid choice. Note it but don't flag it as a problem. -- **Distinguish structural similarity from true duplication.** Tests that follow AAA (Arrange-Act-Assert) will look similar by nature. Only flag when the actual code (not just the structure) is duplicated. -- **Consider the blast radius of refactoring.** A helper shared across 20 tests creates coupling. Note the trade-off. -- **If tests have minimal boilerplate, say so.** A report finding only minor opportunities is perfectly valid. - -### Step 4: Report findings - -Present findings in this structure: - -1. **Summary** — How many boilerplate patterns found, broken down by category. If the test suite is clean, lead with that. -2. **Findings by category** — For each pattern found: - - Category name and description - - Locations: list the specific test methods and files involved - - The duplicated code pattern (show a representative sample) - - Suggested refactoring with a concrete before/after example - - Estimated impact: how many lines/methods would be simplified -3. **Refactoring priority** — Rank findings by: - - Occurrence count (more occurrences = higher value) - - Complexity of the duplicated code (complex setup > simple construction) - - Risk (low-risk extractions first) -4. **Trade-offs** — For each suggestion, note: - - What readability is gained - - What locality/independence is lost - - Whether it's worth it given the occurrence count - -## Validation - -- [ ] Every finding includes specific file and method locations -- [ ] Every finding shows the actual duplicated code, not just a description -- [ ] Every suggestion includes a concrete before/after example -- [ ] Findings are filtered through the 3+ occurrence threshold -- [ ] Simple constructors are not flagged -- [ ] Trade-offs are acknowledged for each suggestion -- [ ] If tests are clean, the report says so upfront - -## Common Pitfalls - -| Pitfall | Solution | -|---------|----------| -| Flagging AAA structure as duplication | The Arrange-Act-Assert pattern is not boilerplate — flag only when the actual code repeats | -| Suggesting extraction for 2 occurrences | Wait for 3+ before recommending extraction | -| Recommending base classes for everything | Prefer composition (helpers, factories) over inheritance | -| Ignoring the readability cost | Every extraction adds indirection — note the trade-off | -| Flagging simple `new X()` as boilerplate | Only flag complex construction with multiple parameters or configuration | -| Recommending DRY at the expense of test isolation | Tests that share mutable state through helpers become coupled — warn about this | diff --git a/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md index 92e706014a..6bd5632df4 100644 --- a/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md @@ -1,19 +1,196 @@ --- name: exp-test-maintainability -description: "Assesses maintainability of .NET test suites and recommends structural improvements. Use when the user asks to reduce test duplication, improve test readability, centralize test data, introduce builders or helpers, or clean up test boilerplate. Covers test size, data-driven patterns, shared setup, helper extraction, and display name quality. Works with MSTest, xUnit, NUnit, and TUnit." +description: "Detects duplicate boilerplate, copy-paste tests, and structural maintainability issues across .NET test suites. Identifies refactoring opportunities — repeated construction, assertion patterns, copy-paste methods convertible to data-driven tests, redundant setup/teardown, and shared infrastructure. Produces an analysis report with concrete before/after suggestions. Works with MSTest, xUnit, NUnit, and TUnit. DO NOT USE FOR: writing new tests (use writing-mstest-tests), reviewing test quality or anti-patterns (use test-anti-patterns), or deep mock auditing (use exp-mock-usage-analysis)." --- # Test Maintainability Assessment -Analyze .NET test code for maintainability issues and recommend targeted refactorings. Read the test files (and production code if available), then assess and report findings. +Analyze .NET test code for maintainability issues: duplicated boilerplate, copy-paste test methods, and structural repetition across test methods and classes. Produce a report of refactoring opportunities with concrete before/after suggestions. The goal is analysis only — do not modify any files. -## Calibration Rules +## When to Use -These judgment rules override default instincts. Apply before reporting: +- User asks to find duplicated code or boilerplate in tests +- User wants to know where test code can be DRY-ed up +- User asks to reduce test duplication, improve test readability, or clean up test boilerplate +- User asks for refactoring opportunities in a test suite +- User wants to identify shared setup or teardown candidates +- User asks "what patterns repeat across my tests?" +- User wants to centralize test data, introduce builders or helpers -- **Only recommend extraction at 3+ occurrences.** Two similar setups aren't worth extracting. -- **Don't recommend builders for simple objects.** `new Calculator()` or `new User(1, "Alice")` doesn't need a factory. -- **Respect intentional verbosity.** Explicit per-test setup is valid if each test reads clearly on its own. -- **Display names matter most for non-obvious values.** `[DataRow("Gold", 100.0, 90.0)]` is self-explanatory. `[DataRow(3, 7, 42)]` is not — add `DisplayName`. -- **Prefer `[DataRow]` with `DisplayName` over `[DynamicData]`** when all values are compile-time constants. `[DataRow]` is simpler. Reserve `[DynamicData]` for computed or complex values. -- **If tests are already well-maintained, say so.** A review finding only minor polish is perfectly valid. Acknowledge what's already good. +## When Not to Use + +- User wants to write new tests from scratch (use `writing-mstest-tests`) +- User wants to detect anti-patterns or code smells (use `test-anti-patterns`) +- User wants to actually perform the refactoring (help them directly, this skill only analyzes) + +## Inputs + +| Input | Required | Description | +|-------|----------|-------------| +| Test code | Yes | One or more test files or a test project directory to analyze | +| Production code | No | The code under test, for context on what abstractions might help | +| Scope | No | Whether to analyze within a single class or across multiple classes | + +## Workflow + +### Step 1: Gather the test code + +Read all test files the user provides or references. If the user points to a directory or project, scan for all test files — see the `exp-dotnet-test-frameworks` skill for framework-specific markers. + +### Step 2: Identify maintainability issues + +Scan for these categories: + +#### Category 1: Repeated object construction + +Look for the same object being constructed in 3+ test methods with identical or near-identical parameters. + +**Indicators:** +- `new ClassName(...)` appearing with identical arguments in multiple tests +- Multiple tests creating the same "system under test" with similar configuration +- Repeated mock/fake/stub creation with the same setup + +**Potential refactorings:** +- Extract a factory method or test helper (e.g., `CreateSut()`, `CreateDefaultOrder()`) +- Use `[TestInitialize]`/constructor/`[SetUp]` for shared construction +- Introduce a builder pattern for complex objects with many variations + +**Example — before:** +```csharp +[TestMethod] +public void Process_ValidOrder_Succeeds() +{ + var logger = new FakeLogger(); + var email = new FakeEmailService(); + var inventory = new FakeInventory(stock: 100); + var processor = new OrderProcessor(logger, email, inventory); + // ... +} + +[TestMethod] +public void Process_EmptyItems_Fails() +{ + var logger = new FakeLogger(); + var email = new FakeEmailService(); + var inventory = new FakeInventory(stock: 100); + var processor = new OrderProcessor(logger, email, inventory); + // ... +} +``` + +**After — extract factory:** +```csharp +private static OrderProcessor CreateProcessor(int stock = 100) +{ + return new OrderProcessor(new FakeLogger(), new FakeEmailService(), new FakeInventory(stock)); +} +``` + +#### Category 2: Repeated assertion patterns + +Look for the same sequence of assertions appearing in 3+ test methods. + +**Indicators:** +- Multiple tests asserting the same set of properties on a result object +- Repeated null-check-then-value-check sequences +- Same collection of `Assert.AreEqual` calls across methods + +**Potential refactorings:** +- Extract a custom assertion helper (e.g., `AssertValidOrder(order, expectedTotal, expectedStatus)`) +- Use framework-specific assertion extensions +- Introduce a `Verify` method that checks a standard set of properties + +#### Category 3: Copy-paste test methods + +Look for test methods with near-identical bodies differing only in input values or a single parameter. + +**Indicators:** +- 3+ methods with the same structure but different literal values +- Methods that could be collapsed into `[DataRow]`/`[Theory]`/`[TestCase]` +- Test names that follow a pattern like `Method_Input1_Result`, `Method_Input2_Result` + +**Potential refactorings:** +- Convert to parameterized tests with `[DataRow]`/`[InlineData]`/`[TestCase]` +- Use `[DynamicData]`/`[MemberData]`/`[TestCaseSource]` for complex inputs +- Prefer `[DataRow]` with `DisplayName` over `[DynamicData]` when all values are compile-time constants. Reserve `[DynamicData]` for computed or complex values. +- Add `DisplayName` for non-obvious parameter values. `[DataRow("Gold", 100.0, 90.0)]` is self-explanatory; `[DataRow(3, 7, 42)]` is not. + +#### Category 4: Duplicated setup/teardown logic + +Look for initialization or cleanup code repeated across test classes. + +**Indicators:** +- Multiple `[TestInitialize]`/`[SetUp]` methods with similar bodies +- Repeated database seeding, file creation, or HTTP client configuration +- Same `using`/`IDisposable` cleanup pattern across classes + +**Potential refactorings:** +- Extract a shared test base class or fixture +- Use composition with a shared helper class +- Create a test context factory + +#### Category 5: Repeated test infrastructure + +Look for structural patterns shared across test classes. + +**Indicators:** +- Same mock interfaces configured identically in multiple classes +- Repeated `HttpClient` setup with similar `DelegatingHandler` patterns +- Same logging/configuration scaffolding across test classes + +**Potential refactorings:** +- Extract a shared test fixture or helper library +- Create reusable fake implementations +- Introduce a test harness class + +### Step 3: Apply calibration rules + +Before reporting, filter findings through these rules: + +- **Only report at 3+ occurrences.** Two similar setups are not boilerplate — they may be intentional clarity. +- **Don't flag simple constructors.** `new Calculator()` or `new List()` is not meaningful boilerplate. Don't recommend builders for `new User(1, "Alice")` either. +- **Respect intentional verbosity.** If each test is self-contained and reads clearly on its own, explicit setup per test is a valid choice. Note it but don't flag it as a problem. +- **Distinguish structural similarity from true duplication.** Tests that follow AAA (Arrange-Act-Assert) will look similar by nature. Only flag when the actual code (not just the structure) is duplicated. +- **Consider the blast radius of refactoring.** A helper shared across 20 tests creates coupling. Note the trade-off. +- **If tests are already well-maintained, say so.** A report finding only minor opportunities is perfectly valid. Acknowledge what's already good. + +### Step 4: Report findings + +Present findings in this structure: + +1. **Summary** — How many patterns found, broken down by category. If the test suite is clean, lead with that. +2. **Findings by category** — For each pattern found: + - Category name and description + - Locations: list the specific test methods and files involved + - The duplicated code pattern (show a representative sample) + - Suggested refactoring with a concrete before/after example + - Estimated impact: how many lines/methods would be simplified +3. **Refactoring priority** — Rank findings by: + - Occurrence count (more occurrences = higher value) + - Complexity of the duplicated code (complex setup > simple construction) + - Risk (low-risk extractions first) +4. **Trade-offs** — For each suggestion, note: + - What readability is gained + - What locality/independence is lost + - Whether it's worth it given the occurrence count + +## Validation + +- [ ] Every finding includes specific file and method locations +- [ ] Every finding shows the actual duplicated code, not just a description +- [ ] Every suggestion includes a concrete before/after example +- [ ] Findings are filtered through the 3+ occurrence threshold +- [ ] Simple constructors are not flagged +- [ ] Trade-offs are acknowledged for each suggestion +- [ ] If tests are clean, the report says so upfront + +## Common Pitfalls + +| Pitfall | Solution | +|---------|----------| +| Flagging AAA structure as duplication | The Arrange-Act-Assert pattern is not boilerplate — flag only when the actual code repeats | +| Suggesting extraction for 2 occurrences | Wait for 3+ before recommending extraction | +| Recommending base classes for everything | Prefer composition (helpers, factories) over inheritance | +| Ignoring the readability cost | Every extraction adds indirection — note the trade-off | +| Flagging simple `new X()` as boilerplate | Only flag complex construction with multiple parameters or configuration | +| Recommending DRY at the expense of test isolation | Tests that share mutable state through helpers become coupled — warn about this | diff --git a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md index 9e4164e1da..77004f8b89 100644 --- a/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-smell-detection/SKILL.md @@ -1,6 +1,6 @@ --- name: exp-test-smell-detection -description: "Deep formal test smell audit based on academic research taxonomy (testsmells.org). Detects 19 categorized smell types — conditional logic, mystery guests, sensitive equality, eager tests, and more — with calibrated severity and research-backed remediation. Use for comprehensive test suite health assessments. For a quick pragmatic review, use test-anti-patterns instead. DO NOT USE FOR: writing new tests (use writing-mstest-tests), evaluating assertion quality specifically (use exp-assertion-quality), or detecting boilerplate duplication (use exp-test-boilerplate-detection)." +description: "Deep formal test smell audit based on academic research taxonomy (testsmells.org). Detects 19 categorized smell types — conditional logic, mystery guests, sensitive equality, eager tests, and more — with calibrated severity and research-backed remediation. Use for comprehensive test suite health assessments. For a quick pragmatic review, use test-anti-patterns instead. DO NOT USE FOR: writing new tests (use writing-mstest-tests), evaluating assertion quality specifically (use exp-assertion-quality), or finding test duplication and boilerplate (use exp-test-maintainability)." --- # Test Smell Detection @@ -34,7 +34,7 @@ Test smells erode confidence in a test suite and inflate maintenance costs: - User wants a quick pragmatic test review (use `test-anti-patterns` — faster, covers the most common issues) - User wants to evaluate assertion diversity specifically (use `exp-assertion-quality`) -- User wants to find duplicated boilerplate across tests (use `exp-test-boilerplate-detection`) +- User wants to find duplicated boilerplate across tests (use `exp-test-maintainability`) - User wants to write new tests from scratch (help them directly) - User wants to fix a specific failing test (diagnose and fix directly) diff --git a/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md b/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md index f0749e727b..eee9bfe20e 100644 --- a/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md +++ b/plugins/dotnet-test/skills/dotnet-test-frameworks/SKILL.md @@ -1,6 +1,6 @@ --- name: dotnet-test-frameworks -description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (test-anti-patterns, exp-test-smell-detection, exp-assertion-quality, exp-test-boilerplate-detection, exp-test-tagging) when they need framework-specific lookup tables." +description: "Reference data for .NET test framework detection patterns, assertion APIs, skip annotations, setup/teardown methods, and common test smell indicators across MSTest, xUnit, NUnit, and TUnit. DO NOT USE directly — loaded by test analysis skills (test-anti-patterns, exp-test-smell-detection, exp-assertion-quality, exp-test-maintainability, exp-test-tagging) when they need framework-specific lookup tables." user-invocable: false --- diff --git a/tests/dotnet-experimental/exp-test-boilerplate-detection/eval.yaml b/tests/dotnet-experimental/exp-test-boilerplate-detection/eval.yaml deleted file mode 100644 index 0d014e3a51..0000000000 --- a/tests/dotnet-experimental/exp-test-boilerplate-detection/eval.yaml +++ /dev/null @@ -1,90 +0,0 @@ -scenarios: - # ========================================================================== - # Scenario 1: Heavy boilerplate — repeated construction and assertion patterns - # ========================================================================== - - - name: "Detect repeated object construction and setup across test methods" - prompt: | - I feel like my test code has a lot of copy-paste. Can you analyze - my OrderService.Tests project and tell me what patterns are repeated - and where I could refactor? - setup: - files: - - path: "OrderService.Tests/OrderService.Tests.csproj" - source: "fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj" - - path: "OrderService.Tests/OrderProcessorTests.cs" - source: "fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs" - assertions: - - type: "output_matches" - pattern: "(factory|helper|extract|CreateProcessor|CreateOrder|shared|reuse|common)" - - type: "output_matches" - pattern: "(3|4|5|6|7|8|multiple|every|all|each|repeated)" - - type: "exit_success" - rubric: - - "Identified that the three-line FakeLogger/FakeEmailService/FakeInventoryService construction block is repeated in every OrderProcessorTests test method" - - "Identified that Order object construction with Items list is repeated across both test classes with very similar structure" - - "Suggested extracting a helper method or factory for creating the OrderProcessor with its dependencies" - - "Suggested extracting a helper for creating test Order objects with customizable properties" - - "Noted that OrderValidatorTests also repeats the same Order construction pattern as OrderProcessorTests, suggesting a cross-class shared helper" - - "Provided concrete before/after code showing how at least one extraction would look" - - "Did not recommend removing all duplication indiscriminately — acknowledged trade-offs between DRY and test readability" - timeout: 120 - - # ========================================================================== - # Scenario 2: Minimal boilerplate — tests are already well-structured - # ========================================================================== - - - name: "Recognize tests with minimal boilerplate that need no refactoring" - prompt: | - Can you check my Calculator.Tests project for any repeated code patterns - that I should clean up? I have two test classes in there. - setup: - files: - - path: "Calculator.Tests/Calculator.Tests.csproj" - source: "fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj" - - path: "Calculator.Tests/CalculatorTests.cs" - source: "fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs" - assertions: - - type: "output_matches" - pattern: "(clean|minimal|good|well.structured|no.*(major|significant)|already|little.*(duplicat|boilerplate)|minor)" - - type: "exit_success" - rubric: - - "Recognized that CalculatorTests already uses parameterized data-driven tests and a shared field instance effectively" - - "Identified that ScientificCalculatorTests has individual test methods that could be consolidated into parameterized tests (e.g., SquareRoot positive/zero cases, Power squared/zero/negative exponent cases)" - - "Noted that the overall code is well-structured and any suggestions are minor improvements, not critical refactorings" - - "Did not recommend over-engineered patterns like base classes or shared fixtures for these simple test classes" - timeout: 120 - - # ========================================================================== - # Scenario 3: Non-activation — user wants to write new tests - # ========================================================================== - - - name: "Decline request to write new tests" - prompt: | - I need to write unit tests for my ShoppingCart class. It handles - adding items, removing items, and calculating totals with discounts. - Can you help me write comprehensive MSTest tests? - expect_activation: false - setup: - files: - - path: "ShoppingCart.cs" - content: | - namespace Store; - - public sealed class ShoppingCart - { - private readonly List _items = new(); - public void Add(CartItem item) => _items.Add(item); - public void Remove(string productId) => _items.RemoveAll(i => i.ProductId == productId); - public decimal GetTotal(decimal discountPercent = 0) - => _items.Sum(i => i.Price * i.Quantity) * (1 - discountPercent / 100); - } - - public record CartItem(string ProductId, string Name, decimal Price, int Quantity); - assertions: - - type: "output_matches" - pattern: "(TestMethod|TestClass|\\[Fact\\]|test)" - rubric: - - "Wrote test methods for the ShoppingCart class" - - "Covered both adding and removing items" - timeout: 300 diff --git a/tests/dotnet-experimental/exp-test-maintainability/eval.yaml b/tests/dotnet-experimental/exp-test-maintainability/eval.yaml index 8a0f1b31a2..3b7a9ae635 100644 --- a/tests/dotnet-experimental/exp-test-maintainability/eval.yaml +++ b/tests/dotnet-experimental/exp-test-maintainability/eval.yaml @@ -188,3 +188,59 @@ scenarios: - "Did not recommend over-engineered patterns disproportionate to the test complexity" - "If any suggestions were made, they were minor and clearly labeled as optional polish" timeout: 120 + + # ========================================================================== + # Scenario 3: Heavy boilerplate — repeated construction and assertion patterns + # ========================================================================== + + - name: "Detect repeated object construction and setup across test methods" + prompt: | + I feel like my test code has a lot of copy-paste. Can you analyze + my OrderService.Tests project and tell me what patterns are repeated + and where I could refactor? + setup: + files: + - path: "OrderService.Tests/OrderService.Tests.csproj" + source: "fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj" + - path: "OrderService.Tests/OrderProcessorTests.cs" + source: "fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs" + assertions: + - type: "output_matches" + pattern: "(factory|helper|extract|CreateProcessor|CreateOrder|shared|reuse|common)" + - type: "output_matches" + pattern: "(3|4|5|6|7|8|multiple|every|all|each|repeated)" + - type: "exit_success" + rubric: + - "Identified that the three-line FakeLogger/FakeEmailService/FakeInventoryService construction block is repeated in every OrderProcessorTests test method" + - "Identified that Order object construction with Items list is repeated across both test classes with very similar structure" + - "Suggested extracting a helper method or factory for creating the OrderProcessor with its dependencies" + - "Suggested extracting a helper for creating test Order objects with customizable properties" + - "Noted that OrderValidatorTests also repeats the same Order construction pattern as OrderProcessorTests, suggesting a cross-class shared helper" + - "Provided concrete before/after code showing how at least one extraction would look" + - "Did not recommend removing all duplication indiscriminately — acknowledged trade-offs between DRY and test readability" + timeout: 120 + + # ========================================================================== + # Scenario 4: Minimal boilerplate — tests are already well-structured + # ========================================================================== + + - name: "Recognize tests with minimal boilerplate that need no refactoring" + prompt: | + Can you check my Calculator.Tests project for any repeated code patterns + that I should clean up? I have two test classes in there. + setup: + files: + - path: "Calculator.Tests/Calculator.Tests.csproj" + source: "fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj" + - path: "Calculator.Tests/CalculatorTests.cs" + source: "fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs" + assertions: + - type: "output_matches" + pattern: "(clean|minimal|good|well.structured|no.*(major|significant)|already|little.*(duplicat|boilerplate)|minor)" + - type: "exit_success" + rubric: + - "Recognized that CalculatorTests already uses parameterized data-driven tests and a shared field instance effectively" + - "Identified that ScientificCalculatorTests has individual test methods that could be consolidated into parameterized tests (e.g., SquareRoot positive/zero cases, Power squared/zero/negative exponent cases)" + - "Noted that the overall code is well-structured and any suggestions are minor improvements, not critical refactorings" + - "Did not recommend over-engineered patterns like base classes or shared fixtures for these simple test classes" + timeout: 120 diff --git a/tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs b/tests/dotnet-experimental/exp-test-maintainability/fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs similarity index 100% rename from tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs rename to tests/dotnet-experimental/exp-test-maintainability/fixtures/heavy-boilerplate/OrderService.Tests/OrderProcessorTests.cs diff --git a/tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj b/tests/dotnet-experimental/exp-test-maintainability/fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj similarity index 100% rename from tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj rename to tests/dotnet-experimental/exp-test-maintainability/fixtures/heavy-boilerplate/OrderService.Tests/OrderService.Tests.csproj diff --git a/tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj b/tests/dotnet-experimental/exp-test-maintainability/fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj similarity index 100% rename from tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj rename to tests/dotnet-experimental/exp-test-maintainability/fixtures/minimal-boilerplate/Calculator.Tests/Calculator.Tests.csproj diff --git a/tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs b/tests/dotnet-experimental/exp-test-maintainability/fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs similarity index 100% rename from tests/dotnet-experimental/exp-test-boilerplate-detection/fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs rename to tests/dotnet-experimental/exp-test-maintainability/fixtures/minimal-boilerplate/Calculator.Tests/CalculatorTests.cs From 83c67dd928ff53f00f66b8cbac97e9b0bfd93bea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 14:47:52 +0200 Subject: [PATCH 4/7] Add cross-references to test-anti-patterns for deep mock and duplication analysis Point users to exp-mock-usage-analysis from the Over-mocking entry and to exp-test-maintainability from the Duplicate tests entry. --- plugins/dotnet-test/skills/test-anti-patterns/SKILL.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index ec44a27d29..87f9dcf6da 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -59,7 +59,7 @@ Check each test file against the anti-pattern catalog below. Report findings gro |---|---| | **Flakiness indicators** | `Thread.Sleep(...)`, `Task.Delay(...)` for synchronization, `DateTime.Now`/`DateTime.UtcNow` without abstraction, `Random` without a seed, environment-dependent paths. | | **Test ordering dependency** | Static mutable fields modified across tests, `[TestInitialize]` that doesn't fully reset state, tests that fail when run individually but pass in suite (or vice versa). | -| **Over-mocking** | More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. | +| **Over-mocking** | More mock setup lines than actual test logic. Verifying exact call sequences on mocks rather than outcomes. Mocking types the test owns. For a deep mock audit, use `exp-mock-usage-analysis`. | | **Implementation coupling** | Testing private methods via reflection, asserting on internal state, verifying exact method call counts on collaborators instead of observable behavior. | | **Broad exception assertions** | `Assert.ThrowsException(...)` instead of the specific exception type. Also: `[ExpectedException(typeof(Exception))]`. | @@ -69,7 +69,7 @@ Check each test file against the anti-pattern catalog below. Report findings gro |---|---| | **Poor naming** | Test names like `Test1`, `TestMethod`, names that don't describe the scenario or expected outcome. Good: `Add_NegativeNumber_ThrowsArgumentException`. | | **Magic values** | Unexplained numbers or strings in arrange/assert: `Assert.AreEqual(42, result)` -- what does 42 mean? | -| **Duplicate tests** | Three or more test methods with near-identical bodies that differ only in a single input value. Should be data-driven (`[DataRow]`, `[Theory]`, `[TestCase]`). Note: Two tests covering distinct boundary conditions (e.g., zero vs. negative) are NOT duplicates -- separate tests for different edge cases provide clearer failure diagnostics and are a valid practice. | +| **Duplicate tests** | Three or more test methods with near-identical bodies that differ only in a single input value. Should be data-driven (`[DataRow]`, `[Theory]`, `[TestCase]`). For a detailed duplication analysis, use `exp-test-maintainability`. Note: Two tests covering distinct boundary conditions (e.g., zero vs. negative) are NOT duplicates -- separate tests for different edge cases provide clearer failure diagnostics and are a valid practice. | | **Giant tests** | Test methods exceeding ~30 lines or testing multiple behaviors at once. Hard to diagnose when they fail. | | **Assertion messages that repeat the assertion** | `Assert.AreEqual(expected, actual, "Expected and actual are not equal")` adds no information. Messages should describe the business meaning. | | **Missing AAA separation** | Arrange, Act, Assert phases are interleaved or indistinguishable. | From b357bf1b72f7691cae15e68688e69cfb70624763 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 14:57:24 +0200 Subject: [PATCH 5/7] Add exp-dotnet-test-frameworks to CODEOWNERS --- .github/CODEOWNERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 20d9ade9a9..f9b8cfb6ad 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -95,6 +95,8 @@ /plugins/dotnet-experimental/plugin.json @JanKrivanek @ViktorHofer @Evangelink @ManishJayaswal @AbhitejJohn # dotnet-experimental-tests (everything experimental test related) +/plugins/dotnet-experimental/skills/exp-dotnet-test-frameworks/ @dotnet/dotnet-testing + /plugins/dotnet-experimental/skills/exp-test-tagging/ @dotnet/dotnet-testing /tests/dotnet-experimental/exp-test-tagging/ @dotnet/dotnet-testing From 393a2fb30752ede3dff93735cb16777ea2487498 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 15:34:04 +0200 Subject: [PATCH 6/7] Improve run-tests SDK 10 MTP detection for blame-hang scenario Inline the critical SDK 10 detection signal (global.json test.runner) directly in Step 1 instead of deferring entirely to the platform-detection skill. This makes the distinction between SDK 10 (no -- separator) and SDK 8/9 (requires -- separator) more prominent. Add a quick detection summary table, strengthen the Common Pitfalls entry for SDK 10 with a blame-hang-timeout example, and keep the platform-detection skill reference for the full detection logic. --- plugins/dotnet-test/skills/run-tests/SKILL.md | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/plugins/dotnet-test/skills/run-tests/SKILL.md b/plugins/dotnet-test/skills/run-tests/SKILL.md index 91c2914bf8..843b75df4a 100644 --- a/plugins/dotnet-test/skills/run-tests/SKILL.md +++ b/plugins/dotnet-test/skills/run-tests/SKILL.md @@ -54,11 +54,17 @@ Detect the test platform and framework, run tests, and apply filters using `dotn ### Step 1: Detect the test platform and framework -1. Run `dotnet --version` to determine the .NET SDK version -2. Read `global.json`, `.csproj`, `Directory.Build.props`, and `Directory.Packages.props` -3. Follow the detection procedure in the `platform-detection` skill to determine: - - **Test framework**: MSTest, xUnit, NUnit, or TUnit - - **Test platform**: VSTest or Microsoft.Testing.Platform (MTP) +1. Read `global.json` first — on .NET SDK 10+, `"test": { "runner": "Microsoft.Testing.Platform" }` is the **authoritative MTP signal**. If present, the project uses MTP and SDK 10+ syntax (no `--` separator). +2. Read `.csproj`, `Directory.Build.props`, and `Directory.Packages.props` for framework packages and MTP properties. +3. For full detection logic (SDK 8/9 signals, framework identification), see the `platform-detection` skill. + +**Quick detection summary:** + +| Signal | Means | +|--------|-------| +| `global.json` has `"test": { "runner": "Microsoft.Testing.Platform" }` | **MTP on SDK 10+** — pass args directly, no `--` | +| `true` in csproj or Directory.Build.props | **MTP on SDK 8/9** — pass args after `--` | +| Neither signal present | **VSTest** | ### Step 2: Run tests @@ -191,7 +197,7 @@ See the `filter-syntax` skill for the complete filter syntax for each platform a | Missing `Microsoft.NET.Test.Sdk` in a VSTest project | Tests won't be discovered. Add `` | | Using VSTest `--filter` syntax with xUnit v3 on MTP | xUnit v3 on MTP uses `--filter-class`, `--filter-method`, etc. -- not the VSTest expression syntax | | Passing MTP args without `--` on .NET SDK 8/9 | Before .NET 10, MTP args must go after `--`: `dotnet test -- --report-trx` | -| Using `--` for MTP args on .NET SDK 10+ | On .NET 10+, MTP args are passed directly: `dotnet test --report-trx` (using `--` still works but is unnecessary) | +| Using `--` for MTP args on .NET SDK 10+ | On .NET 10+, MTP args are passed directly: `dotnet test --project . --blame-hang-timeout 5min` — do NOT use `-- --blame-hang-timeout` | | Multi-TFM project runs tests for all frameworks | Use `--framework ` to target a specific framework | | `global.json` runner setting ignored | Requires .NET 10+ SDK. On older SDKs, use `` MSBuild property instead | | TUnit `--treenode-filter` not recognized | TUnit is MTP-only. On .NET SDK 10+ use `dotnet test`; on older SDKs use `dotnet run` since VSTest-mode `dotnet test` does not support TUnit | From ea03642cd1f94091ce1ee9ec92bc8e636c36e48d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 7 Apr 2026 16:41:03 +0200 Subject: [PATCH 7/7] Improve skill activation keywords in descriptions - exp-test-maintainability: Add 'suggest a better test structure', 'consolidate similar test methods', 'convert copy-paste tests to data-driven parameterized tests' to match prompts like 'each new case needs a whole new method, suggest a better structure'. - test-anti-patterns: Add 'review tests', 'find test problems', 'check test quality', 'audit tests for common mistakes' to match review-style prompts that don't use the word 'anti-pattern'. - run-tests: Add 'hang timeout', 'blame-hang', 'blame-crash', 'TUnit' to match SDK 10 blame scenarios and TUnit filter scenarios that were intermittently not activating. --- .../skills/exp-test-maintainability/SKILL.md | 2 +- plugins/dotnet-test/skills/run-tests/SKILL.md | 5 +++-- plugins/dotnet-test/skills/test-anti-patterns/SKILL.md | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md b/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md index 6bd5632df4..1a9c6fdff4 100644 --- a/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md +++ b/plugins/dotnet-experimental/skills/exp-test-maintainability/SKILL.md @@ -1,6 +1,6 @@ --- name: exp-test-maintainability -description: "Detects duplicate boilerplate, copy-paste tests, and structural maintainability issues across .NET test suites. Identifies refactoring opportunities — repeated construction, assertion patterns, copy-paste methods convertible to data-driven tests, redundant setup/teardown, and shared infrastructure. Produces an analysis report with concrete before/after suggestions. Works with MSTest, xUnit, NUnit, and TUnit. DO NOT USE FOR: writing new tests (use writing-mstest-tests), reviewing test quality or anti-patterns (use test-anti-patterns), or deep mock auditing (use exp-mock-usage-analysis)." +description: "Detects duplicate boilerplate, copy-paste tests, and structural maintainability issues across .NET test suites. Use when the user asks to reduce repetition, consolidate similar test methods, convert copy-paste tests to data-driven parameterized tests, suggest a better test structure, or identify refactoring opportunities. Identifies repeated construction, assertion patterns, copy-paste methods convertible to DataRow/Theory/TestCase, redundant setup/teardown, and shared infrastructure. Produces an analysis report with concrete before/after suggestions. Works with MSTest, xUnit, NUnit, and TUnit. DO NOT USE FOR: writing new tests (use writing-mstest-tests), reviewing test quality or anti-patterns (use test-anti-patterns), or deep mock auditing (use exp-mock-usage-analysis)." --- # Test Maintainability Assessment diff --git a/plugins/dotnet-test/skills/run-tests/SKILL.md b/plugins/dotnet-test/skills/run-tests/SKILL.md index 843b75df4a..1a269cbafe 100644 --- a/plugins/dotnet-test/skills/run-tests/SKILL.md +++ b/plugins/dotnet-test/skills/run-tests/SKILL.md @@ -3,11 +3,12 @@ name: run-tests description: > Runs .NET tests with dotnet test. Use when user says "run tests", "execute tests", "dotnet test", "test filter", "filter by category", "filter by - class", "run only specific tests", "tests not running", or needs to + class", "run only specific tests", "tests not running", "hang timeout", + "blame-hang", "blame-crash", "TUnit", "treenode-filter", or needs to detect the test platform (VSTest or Microsoft.Testing.Platform), identify the test framework, apply test filters, or troubleshoot test execution failures. Covers MSTest, xUnit, NUnit, and TUnit across both VSTest and MTP platforms. - Also use for treenode-filter, --filter-class, --filter-trait, and other + Also use for --filter-class, --filter-trait, and other framework-specific filter syntax. DO NOT USE FOR: writing or generating test code, CI/CD pipeline configuration, or debugging failing test logic. diff --git a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md index 87f9dcf6da..ce5111116b 100644 --- a/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md +++ b/plugins/dotnet-test/skills/test-anti-patterns/SKILL.md @@ -1,6 +1,6 @@ --- name: test-anti-patterns -description: "Quick pragmatic review of .NET test code for anti-patterns that undermine reliability and diagnostic value. Catches the most impactful issues — assertion gaps, flakiness indicators, over-mocking, naming, and structural problems — with actionable fixes. Use for periodic test code reviews and PR feedback. For a deep formal audit based on academic test smell taxonomy, use exp-test-smell-detection instead. Works with MSTest, xUnit, NUnit, and TUnit." +description: "Quick pragmatic review of .NET test code for anti-patterns that undermine reliability and diagnostic value. Use when asked to review tests, find test problems, check test quality, or audit tests for common mistakes. Catches assertion gaps, flakiness indicators, over-mocking, naming issues, and structural problems with actionable fixes. Use for periodic test code reviews and PR feedback. For a deep formal audit based on academic test smell taxonomy, use exp-test-smell-detection instead. Works with MSTest, xUnit, NUnit, and TUnit." --- # Test Anti-Pattern Detection