From 195f2abe042696ab546ef60e29c4e40be957ef10 Mon Sep 17 00:00:00 2001 From: Dave Wilding Date: Tue, 25 Aug 2026 13:07:59 +0800 Subject: [PATCH] Add test strategy guidance: stay grounded, don't shy from integration tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agent's third run succeeded but wrote a generic pytest logging test instead of engaging with the issue's Jubilant context. It used a generic Python logger instead of Jubilant's jubilant.wait logger, and wrote a unit test instead of an integration test, because that's what it could validate with run_tox. Add a 'Test strategy' section to both the composed prompt (probe_issue.py) and the agent definition (probe-issue.md): 1. Frame the agent's purpose as preparation for CI — CI is the ultimate test, run_tox is for increasing confidence in preparation, not for defining the test strategy. 2. 'Stay grounded in the issue's context' — don't abstract away to generic tests. If the issue is about Jubilant's logging, use Jubilant's logger. 3. 'Do not be shy about integration tests' — write them when the claim is about integration test behaviour, even though run_tox can't run them. Use run_tox to validate imports/types; let CI validate behaviour. 4. 'Choose test type based on the issue, not on what you can run' — the test type should match what the issue is about. --- .github/agent/probe-issue.md | 29 ++++++++++++++++++++++++++++- .github/scripts/probe_issue.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 56 insertions(+), 2 deletions(-) diff --git a/.github/agent/probe-issue.md b/.github/agent/probe-issue.md index c4d0ef5..2a073c0 100644 --- a/.github/agent/probe-issue.md +++ b/.github/agent/probe-issue.md @@ -17,7 +17,11 @@ permission: # Doc-validation agent -You write deterministic, reviewable tests that run via CI on the PR. +Your purpose is to prepare a PR whose CI result validates or refutes a doc +claim. You are preparing for CI — CI is the ultimate test. `run_tox` is a +tool for increasing confidence that your preparation is sound (imports +resolve, types check, formatting passes). It is not the arbiter of whether +your test is correct — CI is. ## Core principle @@ -50,6 +54,29 @@ are identical modulo the marker — so do not vary anything else between them. `strict=True` matters: if the xfailed test unexpectedly passes, CI fails, surfacing that the behavioural difference you expected does not actually exist. +### Test strategy + +Choose your test type based on what the issue is about, not based on what +`run_tox` can run. If the claim is about integration test behaviour (e.g., +what Jubilant logs during `juju.wait()`), write an integration test — even +though `run_tox` can only check that it imports and type-checks. CI will run +the integration test and determine the outcome. If the claim is about unit +test behaviour, write a unit test. + +**Stay grounded in the issue's context.** The issue describes a claim in a +specific context — a particular library, tool, or test type. Your test should +engage with that context, not abstract it away. If the issue is about +Jubilant's logging, use Jubilant's logger (`jubilant.wait`), not a generic +Python logger. If the issue is about a specific library version, pin that +version and test against it. Before writing your test, verify that it +exercises the thing the issue is actually about. + +**Do not be shy about integration tests.** `run_tox` runs `format,lint,unit` +only — not integration tests. But integration tests are first-class: they run +in CI after the reviewer marks the PR ready. Write them when the claim is +about integration test behaviour. Use `run_tox` to validate that they import +and type-check; let CI validate the behaviour. + ## What you receive The calling prompt supplies an issue number, a pre-created branch, and the diff --git a/.github/scripts/probe_issue.py b/.github/scripts/probe_issue.py index dabda57..9a59641 100644 --- a/.github/scripts/probe_issue.py +++ b/.github/scripts/probe_issue.py @@ -286,7 +286,11 @@ def test_deploy(charm, juju: jubilant.Juju): TASK_INSTRUCTIONS = """\ ## Task instructions -You write deterministic, reviewable tests that run via CI on the PR. +Your purpose is to prepare a PR whose CI result validates or refutes a doc \ +claim. You are preparing for CI — CI is the ultimate test. `run_tox` is a \ +tool for increasing confidence that your preparation is sound (imports \ +resolve, types check, formatting passes). It is not the arbiter of whether \ +your test is correct — CI is. ### Core principle @@ -308,6 +312,29 @@ def test_deploy(charm, juju: jubilant.Juju): states what you believed, what you tested, and what the CI result means for the \ doc. +### Test strategy + +Choose your test type based on what the issue is about, not based on what \ +`run_tox` can run. If the claim is about integration test behaviour (e.g., \ +what Jubilant logs during `juju.wait()`), write an integration test — even \ +though `run_tox` can only check that it imports and type-checks. CI will run \ +the integration test and determine the outcome. If the claim is about unit \ +test behaviour, write a unit test. + +**Stay grounded in the issue's context.** The issue describes a claim in a \ +specific context — a particular library, tool, or test type. Your test should \ +engage with that context, not abstract it away. If the issue is about \ +Jubilant's logging, use Jubilant's logger (`jubilant.wait`), not a generic \ +Python logger. If the issue is about a specific library version, pin that \ +version and test against it. Before writing your test, verify that it \ +exercises the thing the issue is actually about. + +**Do not be shy about integration tests.** `run_tox` runs `format,lint,unit` \ +only — not integration tests. But integration tests are first-class: they run \ +in CI after the reviewer marks the PR ready. Write them when the claim is \ +about integration test behaviour. Use `run_tox` to validate that they import \ +and type-check; let CI validate the behaviour. + ### Differential testing with xfail Sometimes a claim is best tested by showing that the SAME test behaves \