Migrate crates/pycc_codegen test wrapper onto ScratchDir (Batch A of #782) - #792
Merged
rotnov merged 2 commits intoAug 26, 2026
Merged
Conversation
…782) Retire the crate-local tempfile_dir()/TempTestDir wrapper in favor of pycc_scratch::ScratchDir across crates/pycc_codegen/src/tests.rs (273 call sites) and bigint_rc.rs (2 call sites), including the file's one raw std::env::temp_dir().join(...) occurrence. Deletes tests_support.rs and its #[path]/pub(crate) re-export wiring, adds pycc_scratch as a dev-dependency to root Cargo.toml and crates/pycc_codegen/Cargo.toml, and shrinks check_scratch_dir_usage.py's ALLOWLIST to match. Part of #782 (Batch A per the published implementation plan); the remaining batches (src/main.rs + project_config.rs, and the tests/*.rs integration files) are separate follow-up PRs, so #782 stays open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
crates/pycc_codegen/Cargo.toml already declares pycc_scratch as its own dev-dependency (path = "../pycc_scratch"), which is sufficient for the crate's tests. The duplicate entry in root Cargo.toml's [dev-dependencies] was unused, and it tripped the D-091 bench-manifest fingerprint check in frontend-perf-measure: that check hard-aborts on any change to root Cargo.toml's [dev-dependencies]-onward tail by design, since anything there could affect the pinned benchmark's measurement rather than the compiled artifact it measures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Batch A of the sequential migration plan published on #782: retires
crates/pycc_codegen/src/tests_support.rs'stempfile_dir/TempTestDirwrapper entirely, replacing every call site withpycc_scratch::ScratchDir.pycc_scratchas a[dev-dependencies]entry to rootCargo.tomland tocrates/pycc_codegen/Cargo.toml(the latter had no[dev-dependencies]section before).tempfile_dir("label")call sites incrates/pycc_codegen/src/tests.rs(269 literal-argument calls plus 5&format!(...)-argument calls migrated one at a time in thestr_truthy/float_truthy/str_cmp_runtime/nested_finally/try_pep758_multitests — one more site than initially matched by the bulk-substitution regex, brought the total to 273 as the plan estimated) topycc_scratch::ScratchDir::new("label").expect("failed to create scratch dir").std::env::temp_dir().join(...)occurrence (write_to_file_failure_is_reported_as_an_error) to derive its path from a newScratchDirbinding instead — the test deliberately builds a nonexistent nested path to exercise a real failure mode, so nocreate_dir/create_dir_allwas added; only the root of the path changed.crates/pycc_codegen/src/bigint_rc.rs's 2 call sites and drops itsuse crate::tests::tempfile_dir;import.crates/pycc_codegen/src/tests_support.rsand the#[path = "tests_support.rs"] mod support; pub(crate) use support::tempfile_dir;wiring intests.rs.scripts/check_scratch_dir_usage.py'sALLOWLIST: removes the now-zerocrates/pycc_codegen/src/tests.rsentry and the entry for the now-deletedtests_support.rs. No other files' entries were touched —bigint_rc.rswas never inALLOWLIST(0 raw occurrences to begin with, only wrapper calls, confirmed before editing).Every
ScratchDir::new(...)call in this diff uses.expect(...)only, never?ormatch/if let Err, per the plan's D-014 coverage-gate guidance (§6.1) — this is a near-pure constructor swap with no new fallible branch requiring a dedicated failure-path test.Part of #782 (Batch A). The remaining batches —
src/main.rs's test module +src/project_config.rs(Batch B), and the remainingtests/*.rsintegration files (Batches C/D) — are separate follow-up pull requests; #782 stays open until all batches land.Baseline note
Branched from
feat/issue-781-scratch-dir(PR #786's head), re-confirmed still open and unmerged both before starting and immediately before opening this PR, per the plan's own re-resolution instruction (§1).Overlap note
Cross-referenced open PRs against
crates/pycc_codegen/src/tests.rsandbigint_rc.rsbefore starting: PR #780 (feat/issue-769-optional-narrowing) appends new tests to the end oftests.rsusingtempfile_dir(...)calls of its own. This is a same-file textual collision (not a logical one — different, non-overlapping regions of the file) consistent with the plan's own risk table (§7); whichever of this PR / #780 merges second will need to migrate #780's newly-added call sites ontoScratchDiras part of its own rebase, or as quick follow-up. No other open PR touches either target file.Test plan
cargo build --workspace --testsafter the setup step (Cargo dev-dependency additions) alone, before any call-site editscargo build --workspace --testsafter the full batchcargo test --workspace— 0 failures across the full suitecargo clippy --workspace --all-targets -- -D warnings— cleanpython3 -B scripts/check_scratch_dir_usage.py— no violationspython3 -B -m unittest discover -s scripts -p 'test_*.py'— 951 tests, OK (6 skipped)cargo llvm-cov --workspace --fail-under-lines 100 --fail-under-regions 100— 100.00% lines, 100.00% regionsievo:deep-reviewer) — 0 findings, independently re-verified the 273/273ScratchDir::new/.expectpairing, theALLOWLISTcounts against the live tree, and thewrite_to_file_failure_is_reported_as_an_errornonexistent-path invariant