Skip to content

task template: restore test files before tests, and run the changed package's tests folder - #68

Merged
ArjunS07 merged 6 commits into
release/template-v1from
fix/protect-test-files
Oct 20, 2026
Merged

ArjunS07 merged 6 commits into
release/template-v1from
fix/protect-test-files

Conversation

@ArjunS07

@ArjunS07 ArjunS07 commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

The grader kept agent edits to test files, and pytest ran only the test file that matches a changed module by name. A Qwen3.5-9B trial on rich#3546 used both gaps. Its rich/text.py change broke tests/test_card.py, the agent then rewrote the expected output in tests/_card_render.py, and the trial got a positive reward. A third gap showed up in the checks: one test module that does not import stopped pytest before any test ran, and the runner then reported no regressions. This PR closes all three gaps.

How it works

flowchart TD
  A["patch.diff captured"] --> B["restore step: protected paths and test files go back to the starting tree"]
  B --> C["timing, snapshots"]
  C --> D["pytest: tests mapped by name, base run"]
  D --> E["test_results.json written"]
  E --> F["wider check: tests folders of the changed packages, time limit"]
  F --> G["tests that fail twice with the agent code: run again at base"]
  G --> H["regression list: mapped plus wider; test_results.json written again"]
Loading
  1. The restore step in test.sh now also matches test files:
    (^|/)tests?/   (^|/)test_[^/]*\.py$   (^|/)[^/]*_test\.py$
    
    • Edited test files go back to the starting tree.
    • Test files that the agent added are deleted.
    • patch.diff keeps the full agent diff.
  2. After the mapped tests and their base run, pytest_runner.py runs the nearest tests folder above each changed Python or compiled source file, at most 4 folders.
    • This run is a separate process with a time limit:
    time limit = min(FC_WIDE_TESTS_SEC, FC_PYTEST_TIMEOUT - time used so far - 180 s)
    
    • The defaults are 600 s and 1800 s. With less than 60 s left, the wider check does not run.
    • If the limit stops the run, the tests that finished still count.
  3. A test is a regression when it fails in this run, fails again in a second run of its file, and passes at base.
    • Only the files with such failures (at most 40) run at base.
    • Tests that the mapped run already ran are not counted twice.
  4. These regressions go into regression.regressed and regression.n_regressed, so the grader needs no change. The details are in regression_wide.
  5. Collection errors no longer stop a run, because every run has --continue-on-collection-errors.
    • A test file that does not import with the patch, while one of its tests passes at base, is a regression with the file path as its id.
  6. The rule line in the prompt now says that test files are restored too.

Changes

  1. template/tests/test.sh: three test file patterns in the restore step.
    • Effect: pytest runs the original tests against the agent's code.
    • Before: agent edits to tests stayed in the tree.
  2. template/tests/pytest_runner.py: the wider check.
    • New: run_wide_check, _agent_run_limited, a stream option on run_pytest_and_collect (one line per result, plus pytest's rootdir), and write_results.
    • Effect: a regression in a test file that no name maps to is found.
    • Effect: test_results.json is also written before the wider check, so a stop at FC_PYTEST_TIMEOUT keeps the mapped results.
    • Effect: when the wider run gives no results, regression_wide.output_tail keeps the end of its output.
    • Before: only name-mapped tests (or the fallback when none matched) ran.
  3. template/tests/pytest_runner.py: collection errors.
    • Effect: a patch that breaks the import of a test module fails that file. The other files still run.
    • Before: pytest stopped at the first collection error. The run had 0 tests and no regressions (dascore#415 oracle: 0 of 5005 tests ran).
  4. template/tests/pytest_runner.py: pytest 9.
    • Effect: the runner uses config.rootpath, because pytest 9 has no config.rootdir.
  5. github/templates/final.md.j2.
    • Effect: the rule line names the test files.
  6. Tests.
    • tests/docker/test_test_sh_restore.py: the restore step on a temporary repo.
    • tests/docker/test_pytest_runner_wide.py: the wider check, including a changed compiled source.
    • tests/docker/test_pytest_runner_regression.py: a test file that no longer imports is a regression.

Verification

Replays in the rich#3546 task image, after setup.sh (the starting-tree commit), with the new test.sh restore step and runner:

Tree Restore discarded Mapped regressions Wider check
agent patch hDcEFgz tests/_card_render.py 0 849 tests, 14 failed twice, 6 regressions (test_card_render, test_progress::test_columns, 4 in test_syntax), 12 s
oracle patch none 0 849 tests, 8 failed, all 8 also fail at base, 0 regressions
no change none not run (no tests selected) not run (no changed source)

dascore#415 oracle patch, which changes 5 test files to match an API change:

  • Before the collection fix, 0 tests ran.
  • After it, the wider check ran 4955 tests and found 5 regressions. Two of them are whole files (tests/test_proc/test_filter.py, tests/test_utils/test_patch_utils.py).

Other checks:

  • In the same rich#3546 image, pytest tests/test_card.py with the agent's code and the original expected output: 1 failed.
  • On the current head (d3b18e6): pytest tests/docker/test_test_sh_restore.py tests/docker/test_pytest_runner_*.py tests/docker/test_harbor_adapter_render.py: 33 passed.
  • On the current head: PYTHONPATH=src python -m pytest -q -p no:cacheprovider tests -m "not slow": 1790 passed, 10 skipped.

Notes

  • This PR is based on task template v1: the Phase 1 template and its fixes in one branch, for one merge into main #65 (release/template-v1). Merge task template v1: the Phase 1 template and its fixes in one branch, for one merge into main #65 first.
  • The test file patterns are only in test.sh. They are not added to PROTECTED_PATCH_PATTERNS in formulacode-verified-rl on purpose.
    • For older trials without protected_restore.json, that list fails the trial on any edit.
    • Most agents add tests (194 of 254 Phase 2 trials).
  • Task copies need the new tests/ files. No image rebuild is needed.
  • A test that the oracle patch rewrote now fails with the oracle code too.
    • Oracle records measured with the old test.sh do not list it as an oracle failure, so it would count against an agent that makes the same change.
    • 17 of the 36 Phase 2 and held-out oracle patches change existing tests.
    • formula-code/formulacode-verified-rl#194 (open) adds verified_survey/oracle_tests_rerun.py. It runs the oracle patch through the new restore step and runner, and adds the tests it breaks to the record.

…ackage's tests folder

An agent could rewrite a test's expected output to hide a regression (rich#3546: a rich/text.py change broke
tests/test_card.py; the agent edited tests/_card_render.py). test.sh now discards agent edits to test files with the
measurement setup, and pytest_runner.py runs the tests folders of the changed packages within a time limit: a test that
fails twice with the agent's code and passes at base is a regression.
…ection errors no longer stop the run

dascore#415: the restored tests import a name the oracle patch removed, so pytest stopped at collection and the
runner reported 0 tests run with no regressions.

@atharvas atharvas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good.

@ArjunS07
ArjunS07 merged commit 4d1aa7d into release/template-v1 Oct 20, 2026
0 of 3 checks passed
ArjunS07 added a commit that referenced this pull request Oct 20, 2026
Bring #66, #67 and #68 to main (they were merged into their stacked bases)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants