tests(ci): always build httpjail once per run to avoid stale binaries#39
tests(ci): always build httpjail once per run to avoid stale binaries#39
Conversation
…- Remove HTTPJAIL_BIN and prebuilt binary checks in tests/common\n- Build via cargo in OnceLock to ensure single build per test process\n- Simplify CI to rely on tests building the binary (no prebuild step)\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…GO_HOME, HOME/.cargo, and SUDO_USER before PATH fallback.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…allback; fix clippy lint\n\nThis resolves flaky path detection on CI and collapses nested if for clippy.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…build --bin httpjail\n- Return CARGO_TARGET_DIR/debug/httpjail (or target/debug/httpjail)\n- Drop serde_json and helper path resolvers\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…Helps debug CI by surfacing cargo output in failure strings.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…el under target/ for debug/httpjail when standard path missing.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…R)\n\nAvoids CWD issues in CI; still checks target/<triple>/debug as fallback.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| let output = Command::new("cargo") | ||
| .args(["build", "--bin", "httpjail"]) | ||
| .output() | ||
| .map_err(|e| format!("Failed to execute 'cargo build --bin httpjail': {}", e))?; |
There was a problem hiding this comment.
[P1] Resolve cargo path for sudoed tests
build_httpjail now invokes Command::new("cargo") for every test run. The Linux workflow executes linux_integration under sudo using an absolute cargo path (sudo -E $(which cargo) …) because the root environment’s PATH is sanitized and omits $HOME/.cargo/bin. Once inside the tests, spawning plain cargo via PATH will fail to find the tool, so the binary is never built and the root integration tests crash before running. Use the absolute cargo path (e.g. env!("CARGO") or an env var) instead of relying on PATH in this context.
Useful? React with 👍 / 👎.
…\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
… build from tests\n\nThis ensures tests always execute the freshly built binary matching the test profile, and removes CI flakiness around stale or mismatched binaries.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…age-format json to get the executable path reliably; fallback to probing.\nUse if provided.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…XE_httpjail; fix clippy borrows\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Summary:
cargo build --bin httpjailonce per test process (guarded by OnceLock) to avoid relying on a potentially stale binary on disk.HTTPJAIL_BINand prebuilt-binary shortcuts from tests/common.HTTPJAIL_BIN; tests build the binary as needed, leveraging cargo cache.Rationale:
This ensures integration tests do not accidentally execute with an old binary. The extra
cargo buildis typically a no-op due to caching, but guarantees freshness.Notes:
HTTPJAIL_BIN.Tested: