Add exp-mock-usage-analysis skill and evaluation

.github/CODEOWNERS

@@ -110,6 +110,9 @@
 /plugins/dotnet-experimental/skills/exp-test-gap-analysis/ @dotnet/dotnet-testing
 /tests/dotnet-experimental/exp-test-gap-analysis/ @dotnet/dotnet-testing
 
+/plugins/dotnet-experimental/skills/exp-mock-usage-analysis/ @dotnet/dotnet-testing
+/tests/dotnet-experimental/exp-mock-usage-analysis/ @dotnet/dotnet-testing
+
 /plugins/dotnet-experimental/skills/exp-test-smell-detection/ @dotnet/dotnet-testing
 /tests/dotnet-experimental/exp-test-smell-detection/ @dotnet/dotnet-testing
 

docs/dotnet-experimental/exp-mock-usage-analysis-design-notes.md

@@ -0,0 +1,132 @@
+# Mock Usage Analysis Skill — Design Notes
+
+## Evaluation Results (March 2026)
+
+### Round 1 — Initial skill (11 scenarios)
+
+| Scenario | Baseline | Isolated | Plugin | Verdict |
+| --- | --- | --- | --- | --- |
+| Detect mocking of DTOs, records, and enums | 4.3/5 | 3.3/5 ⏰ | 4.3/5 | ❌ |
+| Detect unused and unreachable mock setups | 4.0/5 | 4.0/5 ⏰ | 4.3/5 ⏰ | ❌ ¹ |
+| Detect redundant mock configurations | 3.0/5 | 2.3/5 ⏰ | 3.3/5 | ❌ |
+| Detect mocking of stable framework types | 3.0/5 | 5.0/5 | 5.0/5 | ✅ |
+| Recognize well-placed mocks | 5.0/5 | 5.0/5 | 5.0/5 | ❌ ² |
+| Analyze mock usage in NSubstitute tests | 3.7/5 | 5.0/5 | 5.0/5 | ✅ |
+| Analyze mock usage in FakeItEasy tests | 5.0/5 | 4.7/5 | 4.7/5 | ❌ |
+| Detect excessive mock configuration sprawl | 3.3/5 | 4.0/5 | 3.3/5 | ✅ |
+| Decline request to write new tests | 2.0/5 | 2.0/5 | 2.3/5 | ❌ ³ |
+| Decline non-mock test anti-patterns | 5.0/5 | 5.0/5 | 5.0/5 | ❌ ² |
+| Decline mock framework migration | 4.0/5 | 4.0/5 | 4.0/5 | ❌ ⁴ |
+
+**3/11 passed.** Overfitting: 0.06 (excellent).
+
+¹ Quality improved in plugin but weighted score -25.2% from token/time overhead.
+² Baseline at ceiling — no headroom for skill to add value.
+³ Token overhead regression on a non-activation scenario with no quality gain.
+⁴ Weighted -1.9% from token/time overhead with no quality delta.
+
+**Issues identified:**
+
+- **Timeouts** on scenarios 1-3 (120s too short for fixture-based scenarios)
+- **Activation failures** — scenario 1 not activated in plugin, scenario 3 not activated in either mode (prompts lacked mock-specific keywords)
+- **Baseline at ceiling** — 4 scenarios where the model already scores 5.0/5
+
+### Round 2 — Fix timeouts, activation, and no-headroom scenarios
+
+**Changes:**
+
+- Increased timeouts: 120s → 180s for scenarios with fixture files
+- Rewrote prompts with explicit mock terminology for better activation
+- Added `reject_tools: ["bash", "edit"]` to FakeItEasy and well-placed mocks scenarios
+- Improved skill description with framework-specific keywords (Mock<T>, Substitute.For, A.Fake)
+- Removed "Decline write tests" scenario (token overhead, no value)
+
+| Scenario | Baseline | Isolated | Plugin | Verdict |
+| --- | --- | --- | --- | --- |
+| Detect mocking of DTOs, records, and enums | 5.0/5 | 5.0/5 | 5.0/5 | ❌ ⁵ |
+| Detect unused and unreachable mock setups | 3.3/5 | 5.0/5 | — | ✅ |
+| Detect redundant mock configurations | 3.0/5 | 4.0/5 | — | ✅ |
+| Detect mocking of stable framework types | 3.0/5 | 5.0/5 | — | ✅ |
+| Recognize well-placed mocks | 5.0/5 | 5.0/5 | 5.0/5 | ❌ ⁵ |
+| Analyze mock usage in NSubstitute tests | 3.0/5 ⏰ | 5.0/5 | — | ✅ |
+| Analyze mock usage in FakeItEasy tests | 4.3/5 | 4.7/5 | — | ❌ |
+| Detect excessive mock configuration sprawl | 3.0/5 | 4.0/5 | — | ✅ |
+| Decline non-mock test anti-patterns | 5.0/5 | 3.7/5 ⏰ | — | ❌ |
+| Decline mock framework migration | 5.0/5 | 5.0/5 | — | ❌ ⁵ |
+
+⁵ Baseline at ceiling — model handles these well without skill guidance.
+
+**Improvements from Round 1:**
+
+- DTOs scenario: now activates in both isolated and plugin (was plugin-only failure)
+- Redundant mocks: now activates in both modes (was NOT ACTIVATED in either)
+- No more timeouts on scenarios 1-3
+- NSubstitute baseline still hit timeout at 120s
+
+### Round 3 — Remove no-headroom scenarios, fix NSubstitute timeout
+
+**Changes:**
+
+- Removed 4 scenarios where baseline scores 5.0/5 (see "Decisions" below)
+- Increased NSubstitute timeout: 120s → 180s
+
+6 remaining scenarios all show positive skill impact.
+
+## Key Insight
+
+The baseline LLM already excels at two mock-related tasks:
+
+1. **Identifying trivial-type mocking** — the model recognizes when `Mock<CustomerDto>` should be `new CustomerDto(...)` without guidance.
+2. **Recognizing well-placed mocks** — when tests correctly mock external boundaries, the model concludes the approach is sound without inventing false positives.
+
+The skill's unique value is in **deep code-path analysis**: tracing mock setups through production code to determine whether they are actually invoked at runtime, identifying unreachable setups after early returns or exceptions, and detecting redundant configurations duplicated across tests.
+
+## Decisions
+
+### Removed: "Detect mocking of DTOs, records, and enums" scenario
+
+Baseline scores 5.0/5 — the model already identifies when DTOs, records, and enums are unnecessarily mocked and recommends real instance construction. No quality delta for the skill to contribute.
+
+### Removed: "Recognize well-placed mocks without inventing false positives" scenario
+
+Baseline scores 5.0/5 — the model already correctly concludes that mocking external boundaries (HTTP, DB, email) is appropriate without inflating severity.
+
+### Removed: "Decline when asked about non-mock test anti-patterns" scenario
+
+Baseline scores 5.0/5 — non-activation scenario where the model already handles Thread.Sleep/DateTime.Now reviews without the skill. The timeout regression (5.0→3.7 ⏰) in the skilled run was caused by the 60s timeout being too short, not a skill problem.
+
+### Removed: "Decline mock framework migration request" scenario
+
+Baseline scores 5.0/5 — the model already handles Moq→NSubstitute migration requests without the skill. Weighted score was -1.2% from time overhead alone.
+
+### Round 4 — Drastic skill simplification
+
+**Problem:** Results degraded significantly. The skill was actively hurting quality — 5/6 scenarios scored worse with the skill than without it. When activated, scores dropped from 2.3-3.3 baseline to 1.0-2.3. Two scenarios showed "NOT ACTIVATED" indicating the skill loaded but the model chose not to use it.
+
+| Scenario | Baseline | With Skill | Verdict |
+| --- | --- | --- | --- |
+| Detect unused and unreachable mock setups | 3.0/5 | 2.3/5 | ❌ |
+| Detect redundant mock configurations | 3.3/5 | 1.0/5 (NOT ACTIVATED) | ❌ |
+| Detect mocking of stable framework types | 3.0/5 | 2.3/5 (NOT ACTIVATED) | ❌ |
+| Analyze mock usage in NSubstitute tests | 2.3/5 | 1.0/5 | ❌ |
+| Analyze mock usage in FakeItEasy tests | 3.3/5 | 1.0/5 | ❌ |
+| Detect excessive mock configuration sprawl | 2.7/5 | 3.7/5 | ✅ |
+
+**Root cause analysis:**
+
+1. **Skill too verbose (~200 lines)** — Extensive anti-pattern catalog tables that the model already knows, consuming attention budget that should go to code analysis.
+2. **6-step workflow too rigid** — Model spent effort following the prescribed categorization workflow (classify dependencies as Trivial/Stable/Thin/External/Complex) instead of doing actual code-path tracing.
+3. **Anti-pattern encyclopedia redundant** — 4 severity levels × 3-4 patterns each = 15+ anti-patterns listed. The model already knows these; listing them added noise without value.
+4. **Reporting format instructions too prescriptive** — "Present findings in this structure: Summary → Critical/High → Medium/Low → Positive → Aggregate" forced a template that didn't match rubric expectations.
+
+**Changes:**
+
+- Cut skill from ~200 lines to ~90 lines
+- Reduced workflow from 6 steps to 4 (read → trace → check replaceable → report)
+- Removed the anti-pattern catalog entirely — model already knows common mock anti-patterns
+- Removed dependency categorization tables (Trivial/Stable/Thin/External/Complex)
+- Removed the runtime data incorporation step (never used in evals)
+- Focused Step 2 entirely on **code-path tracing** — the unique value-add identified in Round 2
+- Added explicit guidance on early returns, exceptions, and branch-specific logic as things to trace
+- Simplified reporting to: specific location + why unreachable + concrete fix
+- Enhanced description with more trigger phrases for better activation

plugins/dotnet-experimental/skills/exp-mock-usage-analysis/SKILL.md

@@ -0,0 +1,98 @@
+---
+name: exp-mock-usage-analysis
+description: "Audits .NET test mock usage by tracing each mock setup through the production code's execution path to find dead, unreachable, redundant, or replaceable mocks. Use when the user asks to audit mock usage, find unused or unnecessary mock setups, check if mocks are needed, reduce mock duplication or over-mocking, simplify test setup, or review whether mock configurations like ILogger/IOptions should use real implementations instead. Supports Moq, NSubstitute, and FakeItEasy."
+---
+
+# Mock Usage Analysis
+
+Trace each mock setup through the production code's execution path to determine which setups are actually exercised at runtime and which are dead, unreachable, redundant, or replaceable with real implementations.
+
+## When to Use
+
+- User asks to audit, review, or analyze mock usage in .NET tests
+- User wants to find unused, unnecessary, or redundant mock setups
+- User wants to simplify test setup or reduce over-mocking
+- User asks whether mocks of ILogger, IOptions, or similar types are needed
+
+## When Not to Use
+
+- User wants to write new mocks or tests (general testing guidance)
+- User wants to detect non-mock test anti-patterns (use `test-anti-patterns`)
+- User wants to migrate between mock frameworks (out of scope)
+
+## Inputs
+
+| Input | Required | Description |
+|-------|----------|-------------|
+| Test code | Yes | Test files to analyze |
+| Production code | Yes | Code under test — essential for tracing execution paths |
+
+## Workflow
+
+### Step 1: Read all provided code
+
+Read the test files and **always** read the production code. You cannot determine whether a mock setup is necessary without understanding the production method's control flow.
+
+Identify the mock framework by scanning for its patterns:
+- **Moq**: `new Mock<T>()`, `.Setup(...)`, `.Verify(...)`
+- **NSubstitute**: `Substitute.For<T>()`, `.Returns(...)`, `.Received(...)`
+- **FakeItEasy**: `A.Fake<T>()`, `A.CallTo(...)`, `.MustHaveHappened()`
+
+Use the correct framework's terminology throughout your analysis.
+
+### Step 2: Trace each mock setup through the production code
+
+For **each test method**, do the following:
+
+1. Identify every mock setup line (`.Setup`, `.Returns`, `A.CallTo`, etc.)
+2. Read the production method being tested and trace its execution path for the specific inputs used in that test
+3. Determine which mock setups are actually reached during execution
+4. Classify each setup:
+
+| Classification | Meaning | Example |
+|---------------|---------|---------|
+| **Used** | The production code calls this mock during the test's execution path | `GetStock` setup when `Reserve` is called and stock is sufficient |
+| **Unreachable** | The production code returns early, throws, or branches away before reaching this mock call | `UpdateStock` setup when the test expects the method to throw `ArgumentOutOfRangeException` on the first line |
+| **Unused** | The mock method is never called by the production method under test at all, regardless of inputs | `GetLowStockProducts` setup when testing `Reserve`, which never calls that method |
+| **Redundant** | Identical mock configurations are duplicated across multiple tests instead of being shared | Five tests each creating `new Mock<IPaymentGateway>()` with the same default setup |
+
+Pay special attention to:
+- **Early returns and guard clauses** — setups for mocks called after a guard clause are unreachable when the guard triggers
+- **Exception throws** — if the method throws before using dependencies, all setups for those dependencies are unnecessary
+- **Branch-specific logic** — if a method dispatches by channel/type, setups for other channels are unused
+- **Verify-only tests** — tests that only call `.Verify`/`.Received`/`.MustHaveHappened` without asserting on the method's return value
+
+### Step 3: Check for replaceable mocks
+
+Flag mocks of stable framework types that should use real implementations:
+- `Mock<ILogger<T>>` → `NullLogger<T>.Instance` (unless log output is asserted)
+- `Mock<IOptions<T>>` → `Options.Create(new T { ... })`
+- Mocks of DTOs, records, or value objects → use `new T { ... }` directly
+
+Explicitly confirm which mocks are **correctly placed** — external boundaries (databases, HTTP clients, message queues, third-party APIs) and security-sensitive types should remain mocked.
+
+### Step 4: Report findings
+
+For each finding, state:
+1. The specific test method and mock setup line
+2. Why the setup is unnecessary (trace the production code path to explain)
+3. A concrete fix — which lines to remove, what to replace them with, or how to extract shared setup
+
+When multiple tests duplicate mock configurations, provide a before/after example showing how to extract shared setup into a fixture or helper method.
+
+## Validation
+
+- [ ] Production code was read and execution paths were traced (not just test code reviewed)
+- [ ] Every finding references a specific test method and setup line
+- [ ] Unreachable setups include an explanation of which production code path makes them unreachable
+- [ ] Correctly-placed mocks (external boundaries) are explicitly noted as appropriate
+- [ ] Correct framework terminology is used throughout (not mixing Moq/NSubstitute/FakeItEasy terms)
+
+## Common Pitfalls
+
+| Pitfall | Solution |
+|---------|----------|
+| Analyzing test code without reading production code | Always read the production method to trace which mocks are actually called |
+| Flagging mocks for external boundaries (HTTP, DB) | These are valid isolation boundaries — keep them mocked |
+| Flagging `ILogger` mock when log output is asserted | Only flag when the mock is set up but log output is never verified |
+| Using wrong framework terminology | Match the framework in the code: Moq (`Setup`/`Verify`), NSubstitute (`Returns`/`Received`), FakeItEasy (`A.CallTo`/`MustHaveHappened`) |