Skip to content

test: give each test its own temporary directory - #154

Merged
ferdingler merged 1 commit into
strands-agents:mainfrom
ferdingler:test/unique-temp-dirs
Oct 16, 2026
Merged

ferdingler merged 1 commit into
strands-agents:mainfrom
ferdingler:test/unique-temp-dirs

Conversation

@ferdingler

@ferdingler ferdingler commented Oct 16, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Twenty-three tests in tests/shell_integration.rs and tests/curl_integration.rs use fixed host paths such as $TMPDIR/lsh_config_test. The grep binary fixture, which 47 tests use, also deletes its directory before it starts. When two test runs share a host, they write, read, and delete the same directories, and random tests fail. On main (ef2fbca), 4 of 6 runs failed when two full suites ran at the same time. A different set of tests failed each time, for example config_file_with_binds_and_creds, bind_direct_write_back, curl_allowed_url_via_toml_config, and several grep_* tests.

This PR adds a small TestDir helper to each of the two files. It creates a new directory named with the test name, the process ID, and a counter, and it removes the directory on drop. A failure to create or remove the directory now fails the test. Before, the tests ignored these errors. The dangling-symlink test now binds a subdirectory, so its escape target stays inside the test directory and outside the mount.

No production code changes. No dependency is added.

Related Issues

None.

Documentation PR

Not applicable.

Type of Change

Other: test reliability.

Testing

  • cargo test --workspace --all-targets: 1805 passed, 0 failed. This is the same count as main.

  • Collision: three rounds of two full suites at the same time, each with its own CARGO_TARGET_DIR.

    • main: 4 of 6 runs failed (7, 3, 1, and 2 failing tests).
    • This branch: no directory failures in 12 runs. In 2 of the 12 runs, bind_direct_write_back failed because the host file was still empty after its fixed 50 ms sleep. Its directory is unique, so this is a separate timing problem. With the sleep set to 2 s for the experiment only, 6 of 6 runs passed. This PR does not change that test's timing.
  • cargo clippy --workspace --all-targets: 14 warnings, the same as main. cargo fmt --check is clean.

  • I ran the relevant test suites for the bindings I touched (cargo test --workspace --all-targets, pytest tests/python, npm test)

  • If I touched Rust, I ran cargo fmt and cargo clippy

Checklist

  • I have read the CONTRIBUTING document
  • I have reviewed and understand every line of code in this PR, including any generated by AI tools, and I can explain why it works
  • My change is focused and reasonably small; I have split unrelated work into separate PRs
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly (not applicable)
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests in shell_integration.rs and curl_integration.rs used fixed host
paths such as $TMPDIR/lsh_config_test. When two test runs share a host,
they write, read, and delete the same directories, so tests fail at
random. The grep binary fixture also deleted its directory before it
started. Setup and cleanup errors were ignored.

Add a small TestDir helper to each file. It creates a new directory
named with the test name, the process ID, and a counter, and it removes
the directory on drop. A failure to create or remove the directory now
fails the test. The dangling-symlink test now binds a subdirectory, so
its escape target stays inside the test directory and outside the mount.
@ferdingler
ferdingler requested a review from a team as a code owner October 16, 2026 17:37
@ferdingler
ferdingler requested a review from liramon2 October 16, 2026 17:37
@ferdingler
ferdingler merged commit bba8d08 into strands-agents:main Oct 16, 2026
31 checks passed
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