mirror of
https://github.com/dotnet/skills.git
synced 2026-09-20 09:49:54 +08:00
Amaury Levé
f532cf1d94
* Add coverage-touching and self-referential assertion heuristics
- test-anti-patterns: add 'Coverage touching' and 'Self-referential assertion'
as named Critical-severity anti-patterns in the detection catalog
- assertion-quality: add 'Tests with self-referential assertions' metric and
a calibration rule distinguishing tautological from valid round-trip assertions
- Add eval scenarios for both new heuristics in test-anti-patterns (coverage-
touching ProductCatalog facade, self-referential SerializerTests)
- Add eval scenario and fixture for self-referential assertions in
assertion-quality (ConfigTests with identity/round-trip patterns)
* Fix test-anti-patterns activation: rewrite description with USE FOR keyword list
The skill was not activating in any eval scenario because the description
used narrative prose instead of the explicit 'USE FOR:' / 'DO NOT USE FOR:'
keyword list pattern that well-activating sibling skills use (run-tests,
writing-mstest-tests). Restructured the description with trigger phrases
matching common user prompts.
* Fix eval activation, timeouts, and add missing vally scenarios
- test-anti-patterns/eval.vally.yaml: increase timeout 2m→5m, add
coverage-touching and self-referential scenarios with constraints
- assertion-quality/eval.vally.yaml: add self-referential scenario
with environment files, broaden assertion pattern
- test-anti-patterns/eval.yaml: add setup.files to all 6 scenarios
to create workspace files that trigger skill discovery/activation
- assertion-quality/eval.yaml: increase timeouts 120→300 for
self-referential and decline-request scenarios, broaden
self-referential assertion pattern
- Create fixture files for mixed-severity, well-written,
coverage-touching, and self-referential test-anti-patterns scenarios
* Address review comments: fix test names and use source references
- Rename ThrowsArgumentException to ThrowsArgumentOutOfRangeException in
PaymentProcessorTests.cs fixture and inlined code in eval.yaml and
eval.vally.yaml to match the actual exception type being asserted.
- Replace inline csproj content blocks with source references to existing
fixture files for UserRepo, Payments, and CatalogService scenarios in
eval.yaml to avoid drift between fixtures and eval setup.
* Address review comments: pin MSTest versions, use source refs, fix naming
- Pin MSTest to 3.8.2 in all fixture .csproj files, add standard
properties (ImplicitUsings, Nullable, IsPackable)
- Replace inline csproj content blocks with source references to
fixture files to avoid content drift
- Create missing .csproj fixtures for flaky-coupled, duplicate-magic,
and assertion-quality self-referential scenarios
- Fix test method names: ThrowsArgumentException ->
ThrowsArgumentOutOfRangeException to match the actual assertion
* Fix activation, eval timeouts, and per-skill description limits
Address remaining issues from PR #635 eval scoreboard
(run 25677669933) and skill-validator failures:
1. test-anti-patterns activation (was NOT ACTIVATED in BOTH plugin and
isolated mode for ALL 6 scenarios — model answered everything from
inline code without ever needing the skill):
- eval.yaml + eval.vally.yaml: drop the giant inline `csharp` blocks
from every prompt. Each prompt now names the workspace files
explicitly (`Audit `Foo.Tests/FooTests.cs` for .NET test
anti-patterns ...`) and asks for the skill's signature deliverable
(severity-ranked list with concrete code-level fixes). The model
must read the workspace, which gives it a chance to consult the
skill — and the skill's structured-findings format is the actual
value-add over inline analysis.
- All 6 scenarios consistently get reject_tools (read-only review)
and timeout 180s.
- eval.vally.yaml: also added missing environment.files for each
scenario (was prompt-only before, no fixture on disk).
2. assertion-quality `Flag assertion-free tests` plugin-mode activation
(was bouncing to other skills because the prompt was generic `check
whether these tests verify anything useful`):
- eval.yaml + eval.vally.yaml: rewrite the prompt to name the file
(`SmokeTests/ApiEndpointSmokeTests.cs`) and explicitly request
`assertion quality and depth`, `assertion-free tests`, and
`trivial assertions like Assert.IsNotNull` — direct trigger
phrases for assertion-quality.
- assertion-quality SKILL.md: extend description with explicit USE
FOR keywords for `assertion-free tests`, `self-referential`, and
`tautological` so plugin-mode activation isn't won by sibling
skills like test-anti-patterns.
3. test-anti-patterns description size — was 1,301 chars (over the
1,024 per-skill limit) AND pushed dotnet-test plugin aggregate to
15,312 (over the 15,000 limit), failing skill-check on PR #635.
Trimmed to 779 chars; aggregate now at 14,983.
4. Vally job-level timeout (60m → 180m) was already raised on main
in commit 0f31228b5 (#641); inheriting that via merge of upstream/main.
Verified: skill-validator check --plugin passes (22 skills, 11 agents,
1 plugin); markdownlint clean; all 4 eval YAMLs parse OK.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address round-2 review comments: add missing production stubs, reject_tools
Address the 3 new Copilot review comments on PR #635
(IDs 3240547649, 3240547701, 3240547725):
1. coverage-touching fixture: `ProductCatalogTests.cs` references
`new InMemoryStore()` but the fixture only declared the
`IProductStore` interface. Audit agents could surface `this won't
compile` as the top finding instead of the intended coverage-touching
pattern. Added a minimal `InMemoryStore : IProductStore`
implementation in the same file as `ProductCatalog.cs`.
2. self-referential fixture: `SerializerTests.cs` referenced 8 production
types (Serializer, UserDto, Money, Config, NameNormalizer, Mapper,
UserEntity, Validator) with no production code on disk. Added a new
`Serialization/Serialization.cs` file with minimal stubs for all 8
types so the audit can stay focused on assertion semantics. Updated
the `Detect self-referential...` scenario in eval.yaml +
eval.vally.yaml to copy the new file into the workspace.
3. assertion-quality `Identify self-referential assertions...` scenario:
added `reject_tools: [bash, edit, create]` and explicit `Do not
edit the files` wording to match the read-only audit pattern used by
other scenarios in the file. Applied to both eval.yaml and
eval.vally.yaml (where it lives under `constraints.reject_tools`).
Verified: skill-validator check passes, markdownlint clean, all 4
eval YAMLs parse OK.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Strengthen activation: name anti-patterns explicitly, dedupe self-referential boundary
PR #635 eval scoreboard run [25859929883](https://github.com/dotnet/skills/actions/runs/25859929883):
- 3 test-anti-patterns scenarios still NOT ACTIVATED in plugin mode (mixed
severity, flakiness, well-written) — variance was high (CV 0.90-1.59),
but only 1-3 of 9 plugin runs actually loaded the skill, vs 3/3 for
scenarios that named specific patterns (coverage-touching, self-referential).
- assertion-quality 'Identify self-referential' NOT ACTIVATED in plugin —
detectedSkills was empty (model picked neither AQ nor TAP despite both
matching the prompt's 'self-referential or tautological' phrasing).
Two-pronged fix:
1. Reduce overlap between assertion-quality and test-anti-patterns —
remove 'self-referential or tautological assertions' from
test-anti-patterns USE FOR. Both skills could plausibly own this, but
assertion-quality is the more focused owner; in plugin mode neither
was winning, so giving assertion-quality the unambiguous trigger
improves activation. test-anti-patterns still owns the broader 'tests
pass but don't verify anything', and the test-anti-patterns 'Detect
self-referential' eval scenario activates fine on the explicit
'audit ... for .NET test anti-patterns' phrasing.
2. Strengthen the 5 problem prompts with skill-specific terminology —
the working pattern from coverage-touching/self-referential prompts
(3/3 and 2/3 plugin activation) was naming the specific anti-pattern
category. Apply the same pattern to the failing scenarios:
- test-anti-patterns/Detect mixed severity: enumerate the catalog
(missing/no assertions, swallowed exceptions, always-true /
self-comparing, weak negative, broad exception types, unused
fields) and ask for findings tied to named anti-patterns.
- test-anti-patterns/Detect flakiness: name the pattern categories
explicitly (Thread.Sleep / DateTime.Now flakiness, ordering
dependency, shared static state, reflection coupling, god tests).
- test-anti-patterns/Recognize well-written: enumerate the catalog
and ask for severity-ranked output 'Critical / Warning / Info /
no issues found' tied to named anti-patterns.
- assertion-quality/Identify self-referential: ask for an
'assertion-quality analysis' / 'assertion-quality metrics' /
'assertion-quality review' (3 mentions of the skill name) and
name the metric (how many tests are self-referential).
Same prompt updates applied to both eval.yaml and eval.vally.yaml.
skill-validator check passes (22 skills, 11 agents, 1 plugin),
aggregate description 14,938 / 15,000.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
166 lines
8.3 KiB
YAML
166 lines
8.3 KiB
YAML
name: assertion-quality
|
|
description: Evaluates the dotnet-test/assertion-quality skill
|
|
type: capability
|
|
config:
|
|
timeout: 5m
|
|
stimuli:
|
|
- name: Identify low assertion diversity in equality-dominated test suite
|
|
prompt: |
|
|
I want to understand whether my PaymentService tests are actually
|
|
thorough. They all pass, but I'm not confident they'd catch real bugs.
|
|
Can you analyze the assertion quality and variety?
|
|
environment:
|
|
files:
|
|
- src: fixtures/low-diversity/PaymentService.Tests/PaymentService.Tests.csproj
|
|
dest: PaymentService.Tests/PaymentService.Tests.csproj
|
|
- src: fixtures/low-diversity/PaymentService.Tests/PaymentProcessorTests.cs
|
|
dest: PaymentService.Tests/PaymentProcessorTests.cs
|
|
graders:
|
|
- type: output-matches
|
|
config:
|
|
pattern: (diversity|variety|spread|shallow|narrow|limited|homogeneous|repetitive)
|
|
- type: output-matches
|
|
config:
|
|
pattern: (AreEqual|equality|equal)
|
|
- type: output-matches
|
|
config:
|
|
pattern: (exception|throw|error|negative|missing)
|
|
- type: exit-success
|
|
- type: prompt
|
|
- type: pairwise
|
|
rubric:
|
|
- Identified that the vast majority of assertions are Assert.AreEqual, showing low assertion type diversity
|
|
- Noted the absence of exception assertions — no tests verify error handling behavior (e.g., invalid card,
|
|
negative amounts)
|
|
- Identified GetReceipt test as having only a trivial Assert.IsNotNull without checking the receipt's content
|
|
- Noted that no tests verify collection contents in GetTransactionHistory — only the count is checked
|
|
- Recommended specific additional assertion types that would improve coverage (exception, collection, structural)
|
|
- Provided concrete examples of assertions that could be added to specific test methods
|
|
- name: Flag assertion-free tests and trivial-only assertions
|
|
prompt: |
|
|
Our QA team says these API smoke tests give us good coverage but I'm
|
|
suspicious. Analyze the assertion quality and depth in
|
|
`SmokeTests/ApiEndpointSmokeTests.cs` — specifically identify
|
|
assertion-free tests (tests that call methods without verifying
|
|
results) and tests with only trivial assertions like
|
|
`Assert.IsNotNull`. Report which tests are assertion-free, the overall
|
|
assertion quality, and concrete recommendations. Do not edit the
|
|
files.
|
|
environment:
|
|
files:
|
|
- src: fixtures/assertion-free/SmokeTests/SmokeTests.csproj
|
|
dest: SmokeTests/SmokeTests.csproj
|
|
- src: fixtures/assertion-free/SmokeTests/ApiEndpointSmokeTests.cs
|
|
dest: SmokeTests/ApiEndpointSmokeTests.cs
|
|
graders:
|
|
- type: output-matches
|
|
config:
|
|
pattern: (no assertion|zero assertion|assertion.free|without.*assert|missing.*assert|don.*assert|lack.*assert)
|
|
- type: output-matches
|
|
config:
|
|
pattern: (5|five|62%|most|majority|several)
|
|
- type: exit-success
|
|
- type: prompt
|
|
- type: pairwise
|
|
rubric:
|
|
- Identified that 5 out of 8 tests have zero assertions — they only call methods without verifying results
|
|
- Identified that the remaining 3 tests only use Assert.IsNotNull, which is a trivial assertion that doesn't
|
|
verify content
|
|
- Explained that these tests give false security — code coverage may look good but correctness is not verified
|
|
- Recommended specific meaningful assertions for at least some of the assertion-free tests
|
|
- Distinguished between intentional smoke tests (acceptable if labeled as such) and tests masquerading as real
|
|
verification
|
|
- name: Recognize well-diversified assertion usage
|
|
prompt: |
|
|
Can you evaluate the assertion quality in my UserService.Tests project?
|
|
I want to make sure my tests are thorough enough before we ship.
|
|
environment:
|
|
files:
|
|
- src: fixtures/good-diversity/UserService.Tests/UserService.Tests.csproj
|
|
dest: UserService.Tests/UserService.Tests.csproj
|
|
- src: fixtures/good-diversity/UserService.Tests/UserManagerTests.cs
|
|
dest: UserService.Tests/UserManagerTests.cs
|
|
graders:
|
|
- type: output-matches
|
|
config:
|
|
pattern: (good|strong|solid|diverse|well|thorough|comprehensive|varied)
|
|
- type: exit-success
|
|
- type: prompt
|
|
- type: pairwise
|
|
rubric:
|
|
- Recognized that the test suite uses a variety of assertion types including equality, boolean, null, exception,
|
|
and collection assertions
|
|
- "Noted positive patterns: tests check multiple properties on returned objects, not just one field"
|
|
- Acknowledged the negative assertions (Assert.IsNull for non-existent user, Assert.IsFalse for failed delete,
|
|
Assert.AreNotEqual for role change)
|
|
- Acknowledged the exception assertions with specific exception types and parameter name verification
|
|
- If any gaps were identified, they were presented as minor enhancement opportunities rather than problems
|
|
- name: Identify self-referential assertions in identity and round-trip tests
|
|
prompt: |
|
|
I want an assertion-quality analysis of my Config tests. They all pass
|
|
but I'm not sure they'd catch real bugs. Analyze the assertion quality
|
|
and depth in `ConfigService.Tests/ConfigTests.cs` — specifically
|
|
identify self-referential or tautological assertions where the test
|
|
asserts the output equals the input on identity / round-trip
|
|
operations. Report assertion-quality metrics (how many tests are
|
|
self-referential, which ones), explain why each flagged test fails to
|
|
verify real behavior, and recommend concrete improvements. Do not
|
|
edit the files — this is a read-only assertion-quality review.
|
|
environment:
|
|
files:
|
|
- src: fixtures/self-referential/ConfigService.Tests/ConfigTests.cs
|
|
dest: ConfigService.Tests/ConfigTests.cs
|
|
- src: fixtures/self-referential/ConfigService.Tests/ConfigService.Tests.csproj
|
|
dest: ConfigService.Tests/ConfigService.Tests.csproj
|
|
graders:
|
|
- type: output-matches
|
|
config:
|
|
pattern: (self.referent|tautolog|identity|input.*equals.*output|circular|same.*value|trivial.*round.?trip|round.?trip|set.*get|write.*read|proves
|
|
nothing|not.*(testing|verif)|meaningless)
|
|
- type: output-matches
|
|
config:
|
|
pattern: (GetTimeout|GetRetries|FormatConnectionString|ValidateKey)
|
|
- type: exit-success
|
|
- type: prompt
|
|
- type: pairwise
|
|
rubric:
|
|
- Identified that multiple tests assert the output equals the input — a self-referential pattern that cannot catch
|
|
transformation bugs
|
|
- Flagged GetTimeout_Default_ReturnsDefault and GetRetries_Default_ReturnsDefault as tautological — they assert
|
|
that reading back a just-written value returns the same value without testing the default/fallback behavior the
|
|
names suggest
|
|
- Flagged FormatConnectionString_RoundTrip as only proving round-trip identity without verifying the formatted
|
|
string's actual structure or content
|
|
- Flagged ValidateKey_Valid_ReturnsSameKey as not exercising the validation logic — it only proves valid input
|
|
passes through unchanged
|
|
- Noted that Serialize_Deserialize round-trip is potentially valid but should be accompanied by tests with
|
|
edge-case inputs (special characters, nulls, empty strings)
|
|
- "Recommended concrete improvements: test actual default values, test invalid keys, test formatted string
|
|
structure, test edge-case serialization inputs"
|
|
constraints:
|
|
reject_tools:
|
|
- bash
|
|
- edit
|
|
- create
|
|
- name: Decline request to write new tests from scratch
|
|
prompt: |
|
|
I need to write unit tests for my InventoryTracker class.
|
|
It handles stock levels, reorder alerts, and warehouse transfers.
|
|
Can you help me write a full MSTest test suite from scratch?
|
|
environment:
|
|
files:
|
|
- src: ./fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs
|
|
dest: InventoryTracker.cs
|
|
graders:
|
|
- type: output-matches
|
|
config:
|
|
pattern: (TestMethod|TestClass|\[Fact\]|test)
|
|
- type: prompt
|
|
- type: pairwise
|
|
rubric:
|
|
- Wrote test methods for the InventoryTracker class
|
|
- Covered multiple methods of the class
|
|
constraints:
|
|
reject_skills:
|
|
- "*"
|