Skip to content

tests(ci): always build httpjail once per run to avoid stale binaries#39

Merged
ammario merged 13 commits intomainfrom
blink/build-test-binary-once
Sep 12, 2025
Merged

tests(ci): always build httpjail once per run to avoid stale binaries#39
ammario merged 13 commits intomainfrom
blink/build-test-binary-once

Conversation

@blink-so
Copy link
Copy Markdown
Contributor

@blink-so blink-so Bot commented Sep 12, 2025

Summary:

  • Tests now always run cargo build --bin httpjail once per test process (guarded by OnceLock) to avoid relying on a potentially stale binary on disk.
  • Removed HTTPJAIL_BIN and prebuilt-binary shortcuts from tests/common.
  • CI no longer prebuilds and exports 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 build is typically a no-op due to caching, but guarantees freshness.

Notes:

  • Linux sudo tests remain unchanged aside from not needing HTTPJAIL_BIN.
  • macOS relies on the same behavior; no README/docs changes included.

Tested:

  • Local compile of test helpers. CI should validate end-to-end.

blink-so Bot added 8 commits September 12, 2025 19:19
…- 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>
@ammario ammario marked this pull request as ready for review September 12, 2025 19:51
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

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".

Comment thread tests/common/mod.rs Outdated
Comment on lines +12 to +15
let output = Command::new("cargo")
.args(["build", "--bin", "httpjail"])
.output()
.map_err(|e| format!("Failed to execute 'cargo build --bin httpjail': {}", e))?;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[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 👍 / 👎.

blink-so Bot added 5 commits September 12, 2025 19:58
…\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>
@ammario ammario merged commit a48833a into main Sep 12, 2025
6 checks passed
@ammario ammario deleted the blink/build-test-binary-once branch September 12, 2025 20:40
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.

1 participant