Skip to content

test_main_materializes_external_source_file_symlinks exceeds the 30 s global pytest timeout on a local host #452

Closed

Description

@wafflecat-df12

What happens

scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks
fails under make test with:

Failed: Timeout (>30.0s) from pytest-timeout.

It is deterministic here, not load-dependent

Reproduced in isolation, single-threaded, with no other gate running:

command wall result
pytest -n 0 <nodeid> (repo config, timeout = 30) 30.13 s fail — timeout at the cap
pytest -n 0 --timeout=180 <nodeid> 49.09 s pass

Only the bound changed between the two runs, and load average was 2.92 during
the passing run. The test's real cost (~49 s) therefore sits well above the
global bound: it does not merely race the cap, it exceeds it by ~19 s.

There is no per-test override. pyproject.toml:366 sets timeout = 30 for the
whole suite, and neither conftest.py nor scripts/tests/conftest.py raises it
for this module.

Why CI does not see it

The test's expensive work is a genuine cold Rust compile — contract.main()
copies the safe workspace and _compile() builds it, then the test compiles the
copied crate again. _compile() sets CARGO_TARGET_DIR to a path under the
test's own tmp_path (on /tmp), so there is no warm shared cache to amortise
it against.

CI buys that build more parallelism than a local run does:

  • Makefile:99 — PYTEST_CARGO_BUILD_JOBS ?= 1, deliberately (a developer
    laptop sharing cores with an editor).
  • .github/workflows/ci.yml (typecheck-test job) —
    make test-python PYTEST_CARGO_BUILD_JOBS="${LINUX_RUNNER_VCPUS}", i.e. 2,
    on a dedicated runner the job is billed for regardless.

So the bound is asked to cover a cold Rust build whose wall clock depends on
-j, and it fits on a 2-vCPU runner while it does not fit at -j1 locally.
Any PYTEST_CARGO_BUILD_JOBS value other than the one CI uses is then
misleading: a passing local make test does not imply the bound is sound, and
make test PYTEST_CARGO_BUILD_JOBS=1 fails for a reason unrelated to the code
under test.

Provenance

Introduced by #448 (df0b4f6c, "Enforce the Rust lint baseline"). On that
commit git log -1 -- scripts/tests/test_boundary_workspace.py is #448 itself,
so the file has no earlier history to compare against.

Ask

Make this test's completion deterministic rather than -j-dependent, e.g. by
giving this module an explicit per-test timeout that reflects a cold Rust
compile, or by arranging for the compile it performs to be bounded
independently of the suite-wide timeout = 30. The global bound should stay
tight for the tests that hang, which is what conftest.py:32 says it is for.

Activity

  1. added
    bugSomething isn't working
    testingTest coverage, test infrastructure, and verification tooling work.
    on Sep 20, 2026
  2. leynos commented on Oct 11, 2026

    @leynos
    Owner
  3. coderabbitai commented on Oct 11, 2026

    @coderabbitai

    Coding Plan

    Summary

    • Treat the ticket as partly stale. Confirm by reproduction that the existing 900 s marker overrides the 30 s default under CARGO_BUILD_JOBS=1.
    • Keep the established pattern: a local @pytest.mark.timeout on the single real-compile test, and a tight global bound for every other test.
    • Replace the timeout literals with named constants. Bound the cargo metadata helper. Correct the stale comment. Give each subprocess its own bound and keep the result independent of -j.
    • Pin the marker and the tier ordering with a fast contract test. Document the tier in docs/coverage-timeout-tiers.md, as the repository already does for the Rust tiers.
    Design Choices
    Design Choice 1: Determine whether the existing `@pytest.mark.timeout(900)` is sufficient

    Options Considered:

    1. Keep the 900 s value. Replace the literals with named constants. Correct the stale comment. Pin the ordering with a contract test.
    2. Introduce a shared fixture or a collection-time hook that raises timeouts for compile tests in scripts/tests/.
    3. Raise the global timeout in pyproject.toml.

    Chosen Option: 1

    Rationale: The ticket says that no per-test override exists. The tree shows a 900 s marker above the 600 s Cuprum compile timeout. The measured cold -j1 cost is 181 s, so 900 s covers the slowest supported configuration.

    Option 1: the repository already uses a local marker for each slow test, and only one test in scripts/tests/ reaches a real compile. A shared hook adds abstraction without need, and the documented policy forbids a raise of the global bound.

    Design Choice 2: Give the `cargo metadata` helper its own deadline

    Options Considered:

    1. Add an explicit, named timeout to the subprocess.run call in boundary_harness_support.py.
    2. Leave the helper unbounded and rely on the pytest marker.

    Chosen Option: 1

    Rationale: The 900 s marker covers the whole test. An unbounded metadata call can therefore consume the budget before the Cuprum 600 s timeout fires. Then the outer bound fires first and gives a less specific failure.

    Option 1: this change is small and local. It makes each subprocess in the named test bounded independently of the suite-wide timeout = 30, which the ticket asks for.

    💡 User Tips

    Regenerate the plan with different choices with @coderabbitai <feedback>.

    Implementation Steps

    Phase 1: Confirm the current state

    Show that the reported failure does not occur on the current tree. Do this before you change code, so that later changes do not mask a different cause.

    Reproduce under serial and parallel Cargo builds

    Confirm that the marker already overrides the 30 s default and covers a -j1 cold build.

    • Run uv run pytest -n 0 scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks with the repository configuration and no --timeout argument.
    • Run the same command with CARGO_BUILD_JOBS=1 in the environment. This is the slowest supported configuration.
    • Record the wall-clock time of each run. Confirm that pytest reports no Timeout (>30.0s) failure.
    • If either run fails at 30 s, stop. Check that pytest-timeout honours the marker. Check for a stale checkout or a --timeout override in addopts or in the Makefile test-python recipe. Fix that cause before you continue.
    🤖 Prompt for AI agents
    Confirm the existing timeout behaviour before changing code. Verify that the
    local marker overrides the 30 s default under the repository configuration and
    the slowest supported Cargo configuration.
    
    - Run `uv run pytest -n 0
    scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks`
    without a `--timeout` argument.
    - Repeat the command with `CARGO_BUILD_JOBS=1` in the environment to check the
    serial cold-build case.
    - Record each wall-clock time and confirm that neither run reports `Timeout
    (>30.0s)`.
    - If either run fails at 30 s, stop implementation. Check marker handling, a
    stale checkout, and `--timeout` overrides in `addopts` or the Makefile
    `test-python` recipe. Fix the cause before continuing.
    

    Phase 2: Make the timeout tier explicit and pinned

    Replace the magic numbers with named constants. Bound the remaining unbounded subprocess. Add a regression test so that a later edit cannot remove the marker or invert the ordering.

    Task 1: Name the real compile budget

    Expose the Cuprum compile timeout as a constant that tests can import.

    • Update scripts/boundary_compile.py. Add a module-level constant, for example COMPILE_TIMEOUT_SECONDS = 600.
    • Use this constant in the sh.ExecutionContext(..., timeout=...) call in place of the literal 600.
    • Keep the value unchanged. Keep production make boundary-contract behaviour unchanged.
    Task 2: Bound the `cargo metadata` helper

    Stop untimed preparation work from consuming the per-test budget.

    • Update scripts/tests/boundary_harness_support.py. Add a named constant, for example CARGO_METADATA_TIMEOUT_SECONDS. Set a modest value that covers a wait for the package-cache lock, for example 120.
    • Pass this constant as timeout= to the subprocess.run([cargo, "metadata", ...]) call.
    • Let subprocess.TimeoutExpired propagate. Make a timeout fail the test with a clear cause. Do not make a timeout look like an empty target list.
    Task 3: Replace the marker literal and correct the comment

    Keep the 900 s bound. Express it as a named constant and remove the stale historical claim.

    • Update scripts/tests/test_boundary_workspace.py. Add a module-level constant, for example SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS = 900. Use it in @pytest.mark.timeout(...).
    • Rewrite the comment above the marker to state:
      • This is the only test in the file that reaches the real _compile. Exactly one cargo check runs.
      • Cargo build parallelism belongs to the caller. The bound is sized for the slowest supported case, a cold CARGO_BUILD_JOBS=1 build measured at 181 s. The result does not depend on -j.
      • COMPILE_TIMEOUT_SECONDS owns the real compile budget. CARGO_METADATA_TIMEOUT_SECONDS bounds the metadata calls. The marker sits above both and catches only a wedged process.
      • The suite-wide timeout = 30 stays unchanged on purpose.
    • Remove the sentence that says the Makefile forces serial Cargo builds.
    Task 4: Add a timeout-tier contract test

    Pin the marker and its ordering against the inner budget. Follow the pattern of cuprum/unittests/test_compile_test_timeout_tier.py.

    • Add a test to scripts/tests/test_boundary_workspace.py near the existing argument-coverage tests. Name it, for example, test_real_compile_test_timeout_exceeds_compile_budget.
    • Read the timeout marker from test_main_materializes_external_source_file_symlinks.pytestmark. Assert that the marker exists and that its value equals SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS.
    • Assert that SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS is greater than COMPILE_TIMEOUT_SECONDS + CARGO_METADATA_TIMEOUT_SECONDS.
    • Assert that the marker value is greater than the global timeout. Read the global value from pyproject.toml with tomllib. Do not hard-code it.
    • Keep this test fast. Keep it within the default 30 s bound. Do not invoke Cargo.
    🤖 Prompt for AI agents
    Make the subprocess budgets and the existing 900 s test tier explicit. Add a
    fast contract test that protects the marker and timeout ordering.
    
    - In `scripts/boundary_compile.py`, expose `COMPILE_TIMEOUT_SECONDS = 600` at
    module level. Replace the literal timeout in `sh.ExecutionContext` with the
    constant. Preserve production `make boundary-contract` behaviour.
    - In `scripts/tests/boundary_harness_support.py`, add
    `CARGO_METADATA_TIMEOUT_SECONDS` with the 120 s value specified in the follow-up
    prompt. Allow for a package-cache lock wait. Pass the constant as `timeout=` to
    the `cargo metadata` `subprocess.run` call. Let `subprocess.TimeoutExpired`
    propagate; do not return an empty target list on timeout.
    - In `scripts/tests/test_boundary_workspace.py`, add
    `SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS = 900` and use it in the existing timeout
    marker.
    - Rewrite the marker comment. State that this is the only test in the file that
    reaches real `_compile`, and that exactly one `cargo check` runs. State that
    parallelism belongs to the caller and that the bound covers a cold
    `CARGO_BUILD_JOBS=1` build measured at 181 s, independently of `-j`. Explain the
    compile budget, metadata budget, and outer marker for a wedged process. State
    that the global 30 s timeout remains unchanged. Remove the claim that the
    Makefile forces serial builds.
    - Follow `cuprum/unittests/test_compile_test_timeout_tier.py`. Add
    `test_real_compile_test_timeout_exceeds_compile_budget` near the
    argument-coverage tests. Read the named compile test's `pytestmark`; assert that
    the timeout marker exists and equals the test constant. Assert that the test
    constant exceeds the sum of the compile and metadata budgets. Read the global
    timeout from `pyproject.toml` with `tomllib` and assert that the marker exceeds
    it. Do not invoke Cargo; keep the contract test within 30 s.
    - Leave `pyproject.toml`, `conftest.py`, and `Makefile` unchanged. Do not
    reintroduce `PYTEST_CARGO_BUILD_JOBS` or add a `CARGO_BUILD_JOBS` assignment to
    the repository.
    

    Phase 3: Record the tier in the documentation

    Make the Python-side tier visible beside the Rust tiers. Document the policy so that a reader does not need to recover it from a code comment.

    Task 1: Document the boundary-harness compile tier

    Extend the existing tier documentation. Do not create a new document.

    • Update docs/coverage-timeout-tiers.md. Add a short entry for the boundary-harness compile test. State the ordering: Cuprum compile COMPILE_TIMEOUT_SECONDS (600 s) < test marker (900 s).
    • State that the global timeout = 30 stays tight, and that only this test raises its own bound.
    • State that the bound is sized for CARGO_BUILD_JOBS=1, so the result does not depend on the caller's -j value.
    • Name the contract test from Task 2.4 as the enforcement point.
    Task 2: Run validation from the follow-up agent prompt

    Run make boundary-test and the repository lint and type-check targets. Fix any findings.

    🤖 Prompt for AI agents
    Document the boundary-harness compile tier beside the existing Rust tiers.
    Validate the changes with the checks specified in the follow-up agent prompt.
    
    - Extend `docs/coverage-timeout-tiers.md`; do not create a new document.
    - State the ordering: Cuprum compile `COMPILE_TIMEOUT_SECONDS` (600 s) < test
    marker (900 s).
    - State that the global `timeout = 30` remains tight and only this test raises
    its own bound.
    - Explain that the bound covers `CARGO_BUILD_JOBS=1` and does not depend on the
    caller's `-j` value.
    - Name `test_real_compile_test_timeout_exceeds_compile_budget` as the
    enforcement point.
    - Run `make boundary-test` and the repository lint and type-check targets. Fix
    any findings.
    
    Research

    Ticket Summary

    Make scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks complete within a deterministic bound that does not depend on the Cargo -j value. Keep the suite-wide timeout = 30 tight for tests that hang. The current tree already contains @pytest.mark.timeout(900) for this test. Treat the ticket as partly stale. Make the remaining bound explicit, correct and regression-tested.

    Codebase Summary

    The boundary harness lives in scripts/:

    • check_boundary_contract.py holds the orchestration. main() copies the workspace and runs the baseline and unsafe probes.
    • boundary_workspace.py holds the workspace copy logic.
    • boundary_compile.py holds the real cargo check. It uses a Cuprum ExecutionContext with a literal timeout=600 and a CARGO_TARGET_DIR under the copied root.

    The tests are in scripts/tests/. boundary_harness_support.py copies the repository and runs an unbounded cargo metadata helper. In the named test, contract.main() resolves _compile through a recording fake. The test then makes exactly one real cargo check, on the two materialized integration-test targets. It does not make two real builds, as the ticket states.

    Timeout policy has these rules:

    • pyproject.toml sets timeout = 30 globally.
    • A slow test raises only its own bound with @pytest.mark.timeout(N).
    • Rust tiers are documented in docs/coverage-timeout-tiers.md and pinned by contract tests such as cuprum/unittests/test_compile_test_timeout_tier.py.

    PYTEST_CARGO_BUILD_JOBS is retired, and tests/test_ci_worker_bounds.py rejects it. Cargo build parallelism belongs to the caller. CI passes CARGO_BUILD_JOBS="${LINUX_RUNNER_VCPUS}". The comment above the marker still says that the Makefile forces CARGO_BUILD_JOBS=1. That claim is no longer true.

    File-level change summary

    File Nature of change
    scripts/boundary_compile.py Add COMPILE_TIMEOUT_SECONDS. Use it in the Cuprum ExecutionContext. Keep the value at 600.
    scripts/tests/boundary_harness_support.py Add CARGO_METADATA_TIMEOUT_SECONDS. Pass it as timeout= to the cargo metadata subprocess.
    scripts/tests/test_boundary_workspace.py Add SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS = 900. Use it in the marker. Correct the stale comment. Add a fast contract test that pins the marker and the ordering.
    docs/coverage-timeout-tiers.md Add the boundary-harness compile tier and its rationale.
    pyproject.toml, conftest.py, Makefile Make no change. Keep the global timeout = 30. Do not reintroduce PYTEST_CARGO_BUILD_JOBS.

    Follow-up agent prompt

    Work in the cuprum repository. The ticket says that
    scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks
    has no per-test timeout. The current tree already has @pytest.mark.timeout(900). Do not raise
    the global `timeout = 30` in pyproject.toml. Do not reintroduce PYTEST_CARGO_BUILD_JOBS or any
    CARGO_BUILD_JOBS assignment, because tests/test_ci_worker_bounds.py rejects them.
    
    1. Run the named test with `uv run pytest -n 0 <nodeid>`. Run it again with CARGO_BUILD_JOBS=1.
       Confirm that neither run fails at 30 s. If a run fails at 30 s, find out why the marker is
       ignored and stop.
    2. In scripts/boundary_compile.py, add COMPILE_TIMEOUT_SECONDS = 600. Use it in the
       sh.ExecutionContext timeout.
    3. In scripts/tests/boundary_harness_support.py, add CARGO_METADATA_TIMEOUT_SECONDS (120).
       Pass it as timeout= to the cargo metadata subprocess.run call. Let TimeoutExpired propagate.
    4. In scripts/tests/test_boundary_workspace.py:
       - Add SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS = 900 and use it in the marker.
       - Rewrite the comment above the marker. Remove the claim that the Makefile forces
         CARGO_BUILD_JOBS=1. State that Cargo parallelism belongs to the caller, and that the bound
         is sized for a cold -j1 build (181 s measured).
       - Add a fast test. It reads the timeout marker from the named test's pytestmark. It asserts
         that the marker value equals the constant, exceeds COMPILE_TIMEOUT_SECONDS +
         CARGO_METADATA_TIMEOUT_SECONDS, and exceeds the global timeout read from pyproject.toml
         with tomllib.
    5. In docs/coverage-timeout-tiers.md, add the boundary-harness compile tier, its ordering and
       its enforcement test.
    6. Run `make boundary-test` and the repository lint and type-check targets. Fix any findings.
    

    🚀 Next Steps

    🤖 All AI agent prompts combined
    Task: 1
    
    Confirm the existing timeout behaviour before changing code. Verify that the
    local marker overrides the 30 s default under the repository configuration and
    the slowest supported Cargo configuration.
    
    - Run `uv run pytest -n 0
    scripts/tests/test_boundary_workspace.py::test_main_materializes_external_source_file_symlinks`
    without a `--timeout` argument.
    - Repeat the command with `CARGO_BUILD_JOBS=1` in the environment to check the
    serial cold-build case.
    - Record each wall-clock time and confirm that neither run reports `Timeout
    (>30.0s)`.
    - If either run fails at 30 s, stop implementation. Check marker handling, a
    stale checkout, and `--timeout` overrides in `addopts` or the Makefile
    `test-python` recipe. Fix the cause before continuing.
    ===============================================================================
    
    Task: 2
    
    Make the subprocess budgets and the existing 900 s test tier explicit. Add a
    fast contract test that protects the marker and timeout ordering.
    
    - In `scripts/boundary_compile.py`, expose `COMPILE_TIMEOUT_SECONDS = 600` at
    module level. Replace the literal timeout in `sh.ExecutionContext` with the
    constant. Preserve production `make boundary-contract` behaviour.
    - In `scripts/tests/boundary_harness_support.py`, add
    `CARGO_METADATA_TIMEOUT_SECONDS` with the 120 s value specified in the follow-up
    prompt. Allow for a package-cache lock wait. Pass the constant as `timeout=` to
    the `cargo metadata` `subprocess.run` call. Let `subprocess.TimeoutExpired`
    propagate; do not return an empty target list on timeout.
    - In `scripts/tests/test_boundary_workspace.py`, add
    `SYMLINK_COMPILE_TEST_TIMEOUT_SECONDS = 900` and use it in the existing timeout
    marker.
    - Rewrite the marker comment. State that this is the only test in the file that
    reaches real `_compile`, and that exactly one `cargo check` runs. State that
    parallelism belongs to the caller and that the bound covers a cold
    `CARGO_BUILD_JOBS=1` build measured at 181 s, independently of `-j`. Explain the
    compile budget, metadata budget, and outer marker for a wedged process. State
    that the global 30 s timeout remains unchanged. Remove the claim that the
    Makefile forces serial builds.
    - Follow `cuprum/unittests/test_compile_test_timeout_tier.py`. Add
    `test_real_compile_test_timeout_exceeds_compile_budget` near the
    argument-coverage tests. Read the named compile test's `pytestmark`; assert that
    the timeout marker exists and equals the test constant. Assert that the test
    constant exceeds the sum of the compile and metadata budgets. Read the global
    timeout from `pyproject.toml` with `tomllib` and assert that the marker exceeds
    it. Do not invoke Cargo; keep the contract test within 30 s.
    - Leave `pyproject.toml`, `conftest.py`, and `Makefile` unchanged. Do not
    reintroduce `PYTEST_CARGO_BUILD_JOBS` or add a `CARGO_BUILD_JOBS` assignment to
    the repository.
    ===============================================================================
    
    Task: 3
    
    Document the boundary-harness compile tier beside the existing Rust tiers.
    Validate the changes with the checks specified in the follow-up agent prompt.
    
    - Extend `docs/coverage-timeout-tiers.md`; do not create a new document.
    - State the ordering: Cuprum compile `COMPILE_TIMEOUT_SECONDS` (600 s) < test
    marker (900 s).
    - State that the global `timeout = 30` remains tight and only this test raises
    its own bound.
    - Explain that the bound covers `CARGO_BUILD_JOBS=1` and does not depend on the
    caller's `-j` value.
    - Name `test_real_compile_test_timeout_exceeds_compile_budget` as the
    enforcement point.
    - Run `make boundary-test` and the repository lint and type-check targets. Fix
    any findings.
    
    💡 Iterate on the plan with: @coderabbitai <feedback>
    Example Feedback
    - @coderabbitai You can skip phase 3. Add a simple unit test case for phase 2.
    - @coderabbitai For design choice 1 go ahead with option 3 and replan.
    

    Start Coding →

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingperformancetestingTest coverage, test infrastructure, and verification tooling work.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions