Skip to content

fix: set cargo target dir explicitly - #683

Merged
m4tx merged 1 commit into
masterfrom
set-cargo-target-dir
Oct 2, 2026
Merged

m4tx merged 1 commit into
masterfrom
set-cargo-target-dir

Conversation

@m4tx

@m4tx m4tx commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

When a user has a global target set in .cargo/config.toml, e.g.:

[build]
target-dir = "./.local/share/cargo/target"

The recently introduced test harness (#665) might cause snapshot tests to run unpredictably, as the exact behavior and stderr changes depending on the exact order the tests are run in.

This sets the target directory explicitly for the test harness, making the snapshot tests reproducible.

@github-actions github-actions Bot added the C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI) label Oct 1, 2026
@github-actions

github-actions Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

🐰 Bencher Report

Projectcot
Branchset-cargo-target-dir
Testbedgithub-ubuntu-latest
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
milliseconds (ms)
(Result Δ%)
Upper Boundary
milliseconds (ms)
(Limit %)
empty_router/empty_router📈 view plot
🚷 view threshold
13.12 ms
(+19.27%)Baseline: 11.00 ms
18.79 ms
(69.81%)
json_api/json_api📈 view plot
🚷 view threshold
1.13 ms
(+11.29%)Baseline: 1.01 ms
1.35 ms
(83.71%)
nested_routers/nested_routers📈 view plot
🚷 view threshold
1.05 ms
(+9.89%)Baseline: 0.95 ms
1.25 ms
(83.83%)
single_root_route/single_root_route📈 view plot
🚷 view threshold
1.05 ms
(+14.39%)Baseline: 0.92 ms
1.21 ms
(86.64%)
single_root_route_burst/single_root_route_burst📈 view plot
🚷 view threshold
18.33 ms
(+8.85%)Baseline: 16.84 ms
22.20 ms
(82.59%)
🐰 View full continuous benchmarking report in Bencher

@m4tx
m4tx force-pushed the set-cargo-target-dir branch from afb306a to 40473c4 Compare October 2, 2026 17:59
@m4tx
m4tx marked this pull request as ready for review October 2, 2026 18:05
@m4tx
m4tx requested review from ElijahAhianyo and a balanced review from Copilot October 2, 2026 18:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The target-directory overrides consistently address the reported nondeterminism without affecting raw configuration-discovery tests.

Review effort: Balanced
Findings: None

What changed in this PR

Pins Cargo target directories to make CLI snapshot and project tests reproducible regardless of global Cargo configuration.

Changes:

  • Sets CARGO_TARGET_DIR for compiled test-project commands.
  • Adds a local Cargo target configuration to temporary unit-test projects.
File Description
cot-cli/​src/​test_harness.rs Pins proxy commands to the bridged project target directory.
cot-cli/​src/​project.rs Isolates temporary project tests from global Cargo target settings.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag Coverage Δ
rust 90.08% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
cot-cli/src/project.rs 97.01% <100.00%> (+0.04%) ⬆️
cot-cli/src/test_harness.rs 71.46% <100.00%> (+0.14%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@m4tx
m4tx merged commit ddaccb5 into master Oct 2, 2026
46 checks passed
@m4tx
m4tx deleted the set-cargo-target-dir branch October 2, 2026 19:47
@cotbot cotbot Bot mentioned this pull request Oct 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-cli Crate: cot-cli (issues and Pull Requests related to Cot CLI)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants