From 69fe56a74f51071cab41a83f14e435a53d8f9c30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 5 Aug 2026 17:06:52 +0200 Subject: [PATCH] Improve dotnet-test eval power and resilience (#989) * Improve dotnet-test eval coverage and efficiency Address remaining high-confidence items from #899 by bounding the code-testing pipeline and adding eval coverage for grade-tests and find-untested-sources. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e430fee9-d3df-4ef5-85a4-745ae4b17046 * Fix dotnet-test eval activation and quality Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5c1a52-4f99-49d6-b503-1bec713a6e98 * Strengthen dotnet-test skill activation Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9c5c1a52-4f99-49d6-b503-1bec713a6e98 * Improve dotnet-test eval signal and resilience Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Address eval review findings Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Remove non-measuring smell eval trials Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Fail evaluation on unexpected adapter errors Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Stabilize assertion quality evidence Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Strengthen assertion quality routing Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a * Treat adapter SIGKILL as failure Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a --------- Copilot-Session: e430fee9-d3df-4ef5-85a4-745ae4b17046 Copilot-Session: 9c5c1a52-4f99-49d6-b503-1bec713a6e98 Copilot-Session: a492694c-6e0f-4af4-8b5e-18331568499a --- .github/workflows/evaluation-run.yml | 33 +++++-- .../skills/assertion-quality/SKILL.md | 2 +- tests/dotnet-test/assertion-quality/eval.yaml | 87 ++++++++++--------- .../InventoryTracker.cs | 8 -- .../fixtures/pytest-mixed/src/inventory.py | 17 ++++ .../pytest-mixed/tests/test_inventory.py | 47 ++++++++++ .../test-smell-detection/eval.yaml | 87 +++---------------- .../ShoppingCart.cs | 9 -- 8 files changed, 148 insertions(+), 142 deletions(-) delete mode 100644 tests/dotnet-test/assertion-quality/fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs create mode 100644 tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/src/inventory.py create mode 100644 tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/tests/test_inventory.py delete mode 100644 tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs diff --git a/.github/workflows/evaluation-run.yml b/.github/workflows/evaluation-run.yml index 55c6e9af..248343dd 100644 --- a/.github/workflows/evaluation-run.yml +++ b/.github/workflows/evaluation-run.yml @@ -328,6 +328,10 @@ jobs: - name: Run vally evaluations if: steps.find-evals.outputs.has_evals == 'true' + # Keep the always-run artifact upload outside this budget. If any + # pre-upload phase stalls, GitHub cancels this step while the job still + # has time to preserve trajectories and partial verdicts. + timeout-minutes: 155 env: GITHUB_TOKEN: ${{ steps.select-token.outputs.token }} RESULTS_DIR: artifacts/TestResults/vally/${{ matrix.entry.name }} @@ -471,14 +475,27 @@ jobs: # compare` per skill (in the adapter) to produce the per-skill # results.json downstream jobs consume. The adapter auto-detects the # `plugin` variant when present. `vally` is on PATH here. - node eng/vally-adapter/adapt.mjs \ - --experiment-dir "$RUN_DIR" \ - --output-root "$RESULTS_DIR" \ - --vally "vally" \ - --repo-root "$GITHUB_WORKSPACE" \ - --model "$MODEL" \ - --judge-model "$JUDGE_MODEL" \ - "${OVERFIT_ARGS[@]}" + # Comparisons are serial and depend on a remote judge. Bound this + # phase more tightly than the whole evaluation step so a stalled + # comparison returns control before the step-level watchdog. + set +e + timeout --signal=TERM --kill-after=30s 45m \ + node eng/vally-adapter/adapt.mjs \ + --experiment-dir "$RUN_DIR" \ + --output-root "$RESULTS_DIR" \ + --vally "vally" \ + --repo-root "$GITHUB_WORKSPACE" \ + --model "$MODEL" \ + --judge-model "$JUDGE_MODEL" \ + "${OVERFIT_ARGS[@]}" + ADAPT_STATUS=$? + set -e + if [ "$ADAPT_STATUS" -eq 124 ]; then + echo "::warning::Vally comparison watchdog expired after 45 minutes; uploading partial results" + elif [ "$ADAPT_STATUS" -ne 0 ]; then + echo "::error::Vally adapter exited unexpectedly with status $ADAPT_STATUS" + exit "$ADAPT_STATUS" + fi # Surface how many verdicts were produced. PRODUCED=$(find "$RESULTS_DIR" -name results.json -not -path "$EXPERIMENT_OUT/*" | wc -l | tr -d ' ') diff --git a/plugins/dotnet-test/skills/assertion-quality/SKILL.md b/plugins/dotnet-test/skills/assertion-quality/SKILL.md index fa8b6303..8b4975ec 100644 --- a/plugins/dotnet-test/skills/assertion-quality/SKILL.md +++ b/plugins/dotnet-test/skills/assertion-quality/SKILL.md @@ -1,6 +1,6 @@ --- name: assertion-quality -description: "Analyzes the variety and depth of assertions across test suites in any language. Use when the user asks to evaluate assertion quality, find shallow tests, identify assertion-free tests (no assertions or only trivial ones like Assert.IsNotNull / toBeTruthy()), flag self-referential or tautological assertions, measure assertion diversity, or audit whether tests verify different facets of behavior. Polyglot: .NET, Python, TS/JS, Java, Go, Ruby, Rust, Swift, Kotlin, PowerShell, C++. DO NOT USE FOR: writing new tests (use code-testing-agent / writing-mstest-tests), mutation reasoning about whether tests would catch a bug (use test-gap-analysis), or a general severity-ranked anti-pattern audit (use test-anti-patterns), fixing or rewriting assertions, or writing, fixing, or modernizing MSTest tests, assertions, or attributes (use writing-mstest-tests)." +description: "MANDATORY for reviewing assertion strength, depth, and variety in existing tests. Invoke when the user asks whether individual assertions are weak, shallow, trivial, always true, self-referential, or diverse; asks which tests are assertion-free or rely only on presence/truthiness checks; or requests assertion quality/depth/variety metrics. Polyglot: .NET, Python/pytest, TS/JS/Jest, Java, Go, Ruby, Rust, Swift, Kotlin, PowerShell, C++. DO NOT USE FOR: writing or fixing tests/assertions (use code-testing-agent or writing-mstest-tests), mutation reasoning (use test-gap-analysis), or a general severity-ranked anti-pattern audit (use test-anti-patterns)." license: MIT --- diff --git a/tests/dotnet-test/assertion-quality/eval.yaml b/tests/dotnet-test/assertion-quality/eval.yaml index bbe80d04..616fbb78 100644 --- a/tests/dotnet-test/assertion-quality/eval.yaml +++ b/tests/dotnet-test/assertion-quality/eval.yaml @@ -1,8 +1,9 @@ name: assertion-quality description: Evaluates the dotnet-test/assertion-quality skill type: capability -config: +defaults: timeout: 5m + runs: 2 stimuli: - name: Identify low assertion diversity in equality-dominated test suite prompt: | @@ -40,12 +41,10 @@ stimuli: 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. + `SmokeTests/ApiEndpointSmokeTests.cs`. Identify tests with no meaningful + verification or only trivial assertions, explain the false confidence + they create, and recommend what each weak test should verify instead. + Include an overall quality assessment. Do not edit the files. environment: files: - src: fixtures/assertion-free/SmokeTests/SmokeTests.csproj @@ -100,14 +99,11 @@ stimuli: - 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. + but regressions still escape. Analyze the depth of + `ConfigService.Tests/ConfigTests.cs`, identify the tests whose assertions + do not meaningfully verify the behavior implied by their names, and + explain what concrete checks would make them useful. Include counts and + method names. Do not edit the files. environment: files: - src: fixtures/self-referential/ConfigService.Tests/ConfigTests.cs @@ -143,34 +139,6 @@ stimuli: - 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? - # Off-target request: this skill reviews existing assertions, it does not - # author suites. The skill IS loaded here on purpose — the property under - # test is that it stays dormant and does not hijack the request. Do not add - # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it - # identical to the baseline arm, so the head-to-head score becomes pure - # judge noise instead of a measurement (it swung to -40% in run - # 30217274869). `expect_activation: false` alone records the expected - # dormancy without polluting the verdict. - expect_activation: false - 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 - rubric: - - Wrote test methods for the InventoryTracker class - - Covered multiple methods of the class - - Did not derail into an assertion-quality audit of code the user never asked about - - name: Judge assertion strength in a shallow Jest suite prompt: | These Jest tests for our OrderService all pass and the file looks busy, @@ -202,3 +170,36 @@ stimuli: - Recognized that the two tests pinning the returned customer and asserting the thrown error message do verify real behavior, instead of condemning the whole file - Recommended concrete replacement assertions pinning the actual expected values + + - name: Distinguish weak and meaningful assertions in a pytest suite + prompt: | + Analyze the assertion quality in `tests/test_inventory.py`. Some tests may + only execute code or make checks that cannot catch a realistic bug, while + others may already verify the behavior well. Identify both groups and + recommend focused improvements. This is a read-only review. + environment: + files: + - src: fixtures/pytest-mixed/src/inventory.py + dest: src/inventory.py + - src: fixtures/pytest-mixed/tests/test_inventory.py + dest: tests/test_inventory.py + graders: + - type: output-matches + config: + pattern: (no assertion|assertion.free|without.*assert|does not assert) + - type: output-matches + config: + pattern: (assert True|always.true|tautolog|cannot fail) + - type: output-matches + config: + pattern: (pytest\.raises|exception|mock|assert_called_once_with) + - type: exit-success + - type: prompt + rubric: + - Identified test_add_item_executes_without_verification as assertion-free + - Flagged test_find_item_exists as trivial because `is not None` does not verify which item or its values + - Flagged test_discount_is_valid as an always-true assertion that cannot catch a production bug + - Recognized the `pytest.raises(ValueError)` test as a meaningful exception assertion + - Recognized `assert_called_once_with("sku-1", 3)` as meaningful side-effect verification + - Recognized the exact total assertion as meaningful rather than condemning every equality assertion + - Recommended concrete value, state, or collection assertions for the weak tests diff --git a/tests/dotnet-test/assertion-quality/fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs b/tests/dotnet-test/assertion-quality/fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs deleted file mode 100644 index cc8edd00..00000000 --- a/tests/dotnet-test/assertion-quality/fixtures/decline-request-to-write-new-tests-from-scratch/InventoryTracker.cs +++ /dev/null @@ -1,8 +0,0 @@ -namespace Warehouse; - -public sealed class InventoryTracker -{ - public int GetStockLevel(string sku) => throw new NotImplementedException(); - public void AddStock(string sku, int quantity) => throw new NotImplementedException(); - public bool NeedsReorder(string sku) => throw new NotImplementedException(); -} diff --git a/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/src/inventory.py b/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/src/inventory.py new file mode 100644 index 00000000..d1d66f7c --- /dev/null +++ b/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/src/inventory.py @@ -0,0 +1,17 @@ +class Inventory: + def __init__(self, notifier=None): + self._items = {} + self._notifier = notifier + + def add(self, sku, quantity, price): + if quantity <= 0: + raise ValueError("quantity must be positive") + self._items[sku] = {"quantity": quantity, "price": price} + if self._notifier: + self._notifier.stock_added(sku, quantity) + + def find(self, sku): + return self._items.get(sku) + + def total(self): + return sum(item["quantity"] * item["price"] for item in self._items.values()) diff --git a/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/tests/test_inventory.py b/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/tests/test_inventory.py new file mode 100644 index 00000000..2b280549 --- /dev/null +++ b/tests/dotnet-test/assertion-quality/fixtures/pytest-mixed/tests/test_inventory.py @@ -0,0 +1,47 @@ +from unittest.mock import Mock + +import pytest + +from src.inventory import Inventory + + +def test_add_item_executes_without_verification(): + inventory = Inventory() + inventory.add("sku-1", 3, 4.00) + + +def test_find_item_exists(): + inventory = Inventory() + inventory.add("sku-1", 3, 4.00) + + assert inventory.find("sku-1") is not None + + +def test_discount_is_valid(): + discount = 0.15 + + assert True + + +def test_rejects_non_positive_quantity(): + inventory = Inventory() + + with pytest.raises(ValueError): + inventory.add("sku-1", 0, 4.00) + + +def test_notifies_when_stock_is_added(): + notifier = Mock() + inventory = Inventory(notifier) + + inventory.add("sku-1", 3, 4.00) + + notifier.stock_added.assert_called_once_with("sku-1", 3) + + +def test_total_uses_quantity_and_price(): + inventory = Inventory() + inventory.add("sku-1", 3, 4.00) + inventory.add("sku-2", 2, 5.00) + + assert inventory.total() == 22.00 diff --git a/tests/dotnet-test/test-smell-detection/eval.yaml b/tests/dotnet-test/test-smell-detection/eval.yaml index db38fdd2..6f4f75c5 100644 --- a/tests/dotnet-test/test-smell-detection/eval.yaml +++ b/tests/dotnet-test/test-smell-detection/eval.yaml @@ -1,17 +1,16 @@ name: test-smell-detection description: Evaluates the dotnet-test/test-smell-detection skill type: capability -config: +defaults: timeout: 5m + runs: 2 stimuli: - name: Detect multiple test smells in order processing test suite prompt: | - I'm worried about the quality of our OrderProcessor test suite. - Please run a full testsmells.org 19-smell catalog audit on this - suite and name each finding using the academic taxonomy - (Conditional Test Logic, Mystery Guest, Sleepy Test, - Assertion-Free Test, etc.) so I can cite the research catalog - when I share the results. + I'm worried that our OrderProcessor tests pass while still missing real + regressions. Audit the suite using the academic test-smell catalog. Name + each finding, identify the affected test, explain the practical risk, and + recommend a concrete fix. This is a read-only review. environment: files: - src: fixtures/smelly-tests/OrderService.Tests/OrderService.Tests.csproj @@ -44,11 +43,10 @@ stimuli: - Provided concrete fix suggestions showing how to rewrite at least some of the smelly tests - name: Recognize well-written tests with no significant smells prompt: | - Can you do a research-backed academic test-smell taxonomy review - of my Calculator tests using the full testsmells.org 19-smell - catalog? I want each finding (or "no smell detected") to map to - a named smell from the academic literature so we can use these - tests as a clean reference template. + Review my Calculator tests against the academic test-smell catalog. I want + to know whether they are a sound reference for the team or whether any + named smells should be fixed first. Do not invent issues just to fill out + the report. environment: files: - src: fixtures/clean-tests/Calculator.Tests/Calculator.Tests.csproj @@ -70,10 +68,10 @@ stimuli: - name: Recognize integration tests and avoid false positives for external resources prompt: | Our team has been arguing about whether our integration tests are - well-designed. Please use the academic testsmells.org 19-smell - catalog to give us an objective severity-ranked assessment, with - each finding tagged by its formal taxonomy name and integration- - test calibration rules applied. + well-designed. Apply the academic test-smell catalog and give us a + severity-ranked assessment that accounts for the fact that these tests + intentionally use a real database. Name genuine smells without treating + every integration-test pattern as a defect. environment: files: - src: fixtures/integration-tests/DataAccess.IntegrationTests/DataAccess.IntegrationTests.csproj @@ -144,34 +142,6 @@ stimuli: - Every finding includes a concrete fix example — the rewritten code — rather than a bare "fix this" instruction - Used xUnit terminology and APIs (Fact, Skip, Assert.Equal) rather than describing the suite in MSTest terms - - name: Decline request to write new tests from scratch - prompt: | - I need to write unit tests for my ShoppingCart class. It supports - adding items, removing items, calculating totals, and applying - coupon codes. Can you write a complete MSTest test suite for me? - # Off-target request: this skill audits existing tests, it does not author - # suites. The skill IS loaded here on purpose — the property under test is - # that it stays dormant and does not hijack the request. Do not add - # `reject_skills: ["*"]`: forcing the skilled arm skill-free makes it - # identical to the baseline arm, so the head-to-head score becomes pure - # judge noise (it cost this skill a pass at -40% in run 30247614857). - expect_activation: false - environment: - files: - - src: ./fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs - dest: ShoppingCart.cs - graders: - - type: output-matches - config: - pattern: (TestMethod|TestClass|\[Fact\]|test) - - type: prompt - rubric: - - Wrote test methods for the ShoppingCart class - - Covered multiple methods of the class - - Did not derail into a test-smell audit of code the user never asked about - - Judged on whether the requested behaviours are actually covered, not on which response happened to emit more - test methods - - name: Audit a JUnit suite using Java-specific smell markers prompt: > Review the tests in this Java project and tell me which test smells they @@ -207,32 +177,3 @@ stimuli: - Flagged the @Disabled test as an Ignored Test - Recognised addThrowsOnNegativeCount as a well-written test rather than condemning the whole file - Used JUnit terminology and Java APIs throughout rather than describing the suite in MSTest terms - - - name: Calibrate skips and contextually obvious numbers - prompt: > - Audit these xUnit tests for smells. I care about real problems — please - don't flag things that are actually fine. - environment: - files: - - src: fixtures/skip-and-magic/Inventory.Tests/Inventory.Tests.csproj - dest: Inventory.Tests/Inventory.Tests.csproj - - src: fixtures/skip-and-magic/Inventory.Tests/InventoryServiceTests.cs - dest: Inventory.Tests/InventoryServiceTests.cs - graders: - - type: output-matches - config: - pattern: (Skip|skipped|Ignored|disabled) - - type: output-matches - config: - pattern: (sleep|Sleepy|Audit_RunsWithoutError|assertion.free|no assertion) - - type: prompt - rubric: - - Flagged the Thread.Sleep(3000) in Replenish_AsyncJob_Completes as a real Sleepy Test - - Flagged Audit_RunsWithoutError as an assertion-free test - - Flagged the bare Skip = "skip" on Restock_FromSupplier_UpdatesQuantities as an Ignored Test with no - justification - - Distinguished the documented skip on Reserve_AcrossWarehouses_BalancesStock, which cites a tracking issue and - a reason, as materially less concerning than the bare one - - Did NOT flag the counts in AddItems_ThreeAdded_CountIsThree or RemoveItem_FromTwo_LeavesOne as magic numbers — - adding three items and asserting three is self-documenting - - Kept the report proportionate instead of inflating severity to justify the review diff --git a/tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs b/tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs deleted file mode 100644 index 69b81fe3..00000000 --- a/tests/dotnet-test/test-smell-detection/fixtures/decline-request-to-write-new-tests-from-scratch/ShoppingCart.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Store; - -public sealed class ShoppingCart -{ - public void AddItem(string sku, int quantity, decimal price) => throw new NotImplementedException(); - public void RemoveItem(string sku) => throw new NotImplementedException(); - public decimal CalculateTotal() => throw new NotImplementedException(); - public void ApplyCoupon(string code) => throw new NotImplementedException(); -}