Skip to content

chore(agents): make test skills language-neutral - #2136

Merged
Benoît Cortier (CBenoit) merged 19 commits into
masterfrom
cbenoit-language-neutral-test-skills
Oct 21, 2026
Merged

Benoît Cortier (CBenoit) merged 19 commits into
masterfrom
cbenoit-language-neutral-test-skills

Conversation

@CBenoit

@CBenoit Benoît Cortier (CBenoit) commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

The test skills stay one skill per job rather than gaining a copy per language, because the method is identical in Rust and C# and only the mechanics differ.

Each test, case, or assertion carries an // oracle: marker naming the source of its expected value. Tests added to catch a mutant are marked weak, and any intentional change may delete them without justification, because coverage driven by mutants rewards tests that pin how the code works today; tests test-pruner protects stay protected whatever their marker. Tests written only for feedback while implementing never merge: they gain a source or are deleted first.

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

The test skills stay one skill per job rather than gaining a copy per
language, because the method is identical in Rust and C# and only the
mechanics differ; each language's tool mechanics and test forms sit in a
reference loaded only for that language.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Point consolidation forms at `test-writer` instead of restating them in
each mechanics reference, and state the failed-strengthening rule once
on top of `test-oracle-reviewer`. Exempting protected tests from the
challenge leaves no protected test to hand to CODEOWNERS, so that branch
goes, and "protected" is now defined where the tests are listed.

xUnit v2 turns a whole theory into one test case when any row holds a
value it cannot serialize, so `[MemberData]` rows hold only serializable
values. Stryker reruns compare `--mutate` runs, because a test edit
changes which mutants `--since` selects. Domain mutant patches are taken
with `git diff --output`, since a Windows PowerShell redirect writes
UTF-16 with CRLF that `git apply` rejects, and a patch that does not
build proves nothing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A failing gap test is a finding like a failing strengthened test, and
the rule keeps its reason because it is the one most tempting to break.
Protected tests are no longer offered for merging, since issue
regression tests stay standalone under `test-writer`. The evidence
tables are defined where editing mode first names them, Stryker reruns
for a diff are measured with `--mutate` from the start, and a
challenger's hand-written patch is rebuilt with `git diff --output`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 20, 2026 23:16
@CBenoit

Copy link
Copy Markdown
Member Author

Evidence from running test-pruner with these skills on the .NET authority library in #2125:

Before After
Test methods 346 287
Stryker.NET mutation score 75.25% 88.04%

154 mutants are caught that were not caught before, and no previously caught mutant was lost.

Note

Human-tuned, LLM-assisted content.

@CBenoit

Benoît Cortier (CBenoit) commented Oct 20, 2026 •

Copy link
Copy Markdown
Member Author

Lines changed against master:

Area Files Added Removed
Production code 0 0 0
Tests 0 0 0
Agent skills under .agents/skills/ and .github/skills/ 10 231 85

Note

LLM-assisted content (no human feedback).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

The mutation guidance loses test identities and can conflate mutation sites, undermining safe pruning decisions.

3 open issues
What changed in this PR

Extends the shared testing skills to C# while keeping language-specific mechanics in reference files.

Changes:

  • Adds xUnit, CsCheck, and Stryker.NET guidance.
  • Separates shared domain mutants from Rust tooling instructions.
  • Clarifies test-pruning modes and evidence requirements.
File Description
.agents/​skills/​test-writer/​SKILL.md Extends test-writing guidance to C#.
.agents/​skills/​test-writer/​references/​csharp.md Adds C# testing forms.
.agents/​skills/​test-reviewer/​SKILL.md Generalizes review criteria across languages.
.agents/​skills/​test-pruner/​SKILL.md Updates pruning workflows and safeguards.
.agents/​skills/​test-pruner/​references/​stryker.md Documents C# mutation testing and comparison.
.agents/​skills/​test-pruner/​references/​domain-mutants.md Extracts shared domain mutation catalogs.
.agents/​skills/​test-pruner/​references/​cargo-mutants.md Keeps Rust-specific mutation mechanics.
.agents/​skills/​test-oracle-reviewer/​SKILL.md Aligns the review-mode terminology.

Review effort: Balanced

The Stryker kill matrix now prints each killing test's id beside its
display name, since truncated display names collide. Mutants that share
file, mutator, replacement, and line text are matched by their order in
the file, which test and comment edits leave unchanged. A CsCheck
counterexample is pinned in a regression test rather than an
`[InlineData]` row, so regression tests stay standalone.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The documented Stryker command omits a solution-path prerequisite needed to run mutation testing on the repository’s .NET Framework installer tests.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Add solution path for .NET Framework mutation testing

.agents/​skills/​test-pruner/​references/​stryker.md:18

The command lacks the solution path required for .NET Framework projects. package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj:3 targets net48, so this workflow cannot start mutation testing there unless a solution path is already configured. The Stryker.NET 5.0.0 configuration reference requires it. Document adding --solution <path-to-solution.sln> for these projects.

[!NOTE]
LLM-assisted content (no human feedback).

3 resolved since last review

Review effort: Balanced

Stryker.NET cannot run on a .NET Framework test project without a
solution path, and the Agent installer tests target net48.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation changes are coherent, with no substantiated blocking issues remaining.

No open issues 🎉

Review effort: Balanced

`test-writer` now states each rule once without naming a library, and
the Rust forms sit in `references/rust.md` beside the C# ones, so neither
language reads the other's forms and each reference stands alone.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation-only changes are coherent, with one non-blocking compatibility clarification remaining.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Specify a CsCheck version compatible with net48 installer tests

.agents/​skills/​test-writer/​references/​csharp.md:6

The installer tests target net48 (package/AgentWindowsManaged.Tests/DevolutionsAgent.Installer.Tests.csproj:3), while the upstream CsCheck project targets modern .NET. This guidance does not identify a compatible version for those tests, so following it with an incompatible release would fail package restore. Document a compatible CsCheck version or qualify the recommendation with an alternative for unsupported frameworks.

[!NOTE]
LLM-assisted content (no human feedback).

Review effort: Balanced

CsCheck releases after 2.14.1 need .NET 6 or later, and the Agent
installer tests target net48.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation-only changes have no blocking issues; the remaining feedback is a minor safeguard clarification.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Apply viable-mutant safeguard to shared measurement step

.agents/​skills/​test-pruner/​SKILL.md:21

The safeguard against an empty or near-empty viable-mutant sample is only in references/cargo-mutants.md:11, which C# runs do not load. With no viable mutants, every self-derived test has zero unique kills, so deleting unprotected tests still preserves the empty caught set. Add the safeguard to this shared measurement step so it also applies to C# pruning.

[!NOTE]
LLM-assisted content (no human feedback).

Review effort: Balanced

The rule against deciding deletions on an empty or near-empty viable
sample applies to every mutation tool, so it moves from the
cargo-mutants reference into the shared measurement step.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The version-specific mutation workflows need human confirmation because their execution behavior remains unverified.

No open issues 🎉

Review effort: Balanced

Review mode changes nothing in the reviewed branch but still applies
mutants in its own clone, and a failing strengthened or gap test goes to
whoever assigned the run, or to the author in review mode. Deletions
wait until some mutant of the exercised code is caught, because zero
unique kills over nothing caught measures nothing however many mutants
survived. Survivor classes are defined by what an independent
requirement says about the change, after scoping errors and tool
artifacts are ruled out, and challenger mutants are proposed for the
catalog separately because neither mode edits skills.

`code-review` routes C# test changes to `test-reviewer`, the fuzz rule
defers to the language reference where no harness exists, and the
Stryker notes confirm the clean run discovered tests. Restated
definitions and discoverable lines are dropped from the references.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@CBenoit
Benoît Cortier (CBenoit) requested a review from a team as a code owner October 21, 2026 04:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

These guidance-only changes leave one non-blocking documentation consistency nit.

1 open issue

Review effort: Balanced

Comment thread .agents/skills/test-pruner/SKILL.md Resolved
A protected test the pruner cannot resolve would otherwise lose its
only marker and turn into an unmarked test every review flags, so it
keeps the marker and is listed as unresolved in the oracle table.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The skill-only changes align with the stated purpose, with no unresolved findings.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

…utral-test-skills

# Conflicts:
#	.github/skills/code-review/SKILL.md
Contracts may now be named `CONTRACT.<name>.md`, so a `contract`
marker accepts any `CONTRACT*.md` rather than only `CONTRACT.md`.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A scaffold test now gains a strong source, becomes `mutant` when it
alone catches a fault, or is deleted before its pull request merges.
One that checks a property or intention no document states prompts a
question about where to specify it, and is marked with that source once
it exists, because a confirmation is not a source. Protected tests take
that path instead of keeping their scaffold marker, which a merge check
could not tell apart from a forgotten one.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation-only changes have no identified unresolved blocking issues.

No open issues 🎉

Review effort: Balanced

Markers sit in test files at every depth, so a path relative to the
test would read differently from each and could not be resolved without
knowing where the marker lives. Marker paths are now repository-root
relative with forward slashes and exact case, a `spec` outside `docs/`
is named rather than given as a path, a crash file sits under `fuzz/`,
and a section names a heading or the number it starts with.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
A `regression` marker names a Jira key, a GitHub issue as `#123` or
`owner/repo#123`, or a crash file under `fuzz/`, the forms a marker
check can resolve without guessing.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The mutation-driven deletion workflow and version-specific C# tooling procedures need human validation.

No open issues 🎉

Review effort: Balanced

A `property` marker now names one of six well-known laws, because a
named law is its own definition while any other claimed property needs a
written source, and stays `scaffold` until it has one. An assertion
whose value comes from running the code is never a property, even over
generated inputs, and a round-trip never replaces a codec's vectors.

A section reference also matches an `<a id>` anchor placed before a
heading, and markers cite the anchor when the file has one, since it
survives a renamed heading or a renumbered section.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The guidance-only changes have no unresolved blocking findings.

No open issues 🎉

Review effort: Balanced

A marker can now cite a file in another repository as
`<owner>/<repo>@<sha>:<path>`, pinned to a commit of at least 12 hex
digits rather than a branch or tag, because a pinned commit never
changes and resolves the same every time. When the pinned section
changes upstream, the test is re-checked against the new text and its
pin or the test changes, never the expected value alone, since a value
changed without its pin cites text that decides the old one.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The documentation-only changes support the stated testing policy, with no unresolved findings.

No open issues 🎉

Review effort: Balanced

@CBenoit
Benoît Cortier (CBenoit) merged commit f25a014 into master Oct 21, 2026
47 checks passed
@CBenoit
Benoît Cortier (CBenoit) deleted the cbenoit-language-neutral-test-skills branch October 21, 2026 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants