CI: run all tests on macOS and Linux; drop separate weak job#37
Merged
CI: run all tests on macOS and Linux; drop separate weak job#37
Conversation
…do-only linux tests - macOS: single nextest run for entire suite\n- Linux: run all tests as user, then linux_integration + isolated cleanup under sudo\n- Removes Weak-mode job; coverage preserved by macOS + Linux runs\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Normalize to uppercase before parsing to avoid hyper::Method Extension capturing lowercase tokens\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Use nextest filter to skip the linux_integration binary in the non-root sweep\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
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".
… run Use 'binary == "linux_integration"' instead of 'test(binary == ...)'.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Parser requires ! prefix; earlier 'not (...)' failed to parse.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…or non-root sweep Fix filter grammar error
Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
…O_TARGET_DIR via GITHUB_ENV; add ldd diagnostics)
…ary) - Centralized canary and temp directory logic in jail::get_canary_dir() and jail::get_temp_dir()\n- Use ~/.httpjail_canary as fallback if no data dir available\n- Use ~/.httpjail_tmp for temporary files like resolv.conf\n- Remove unnecessary pre-build step from CI\n- Simplify weak mode test dependencies (just curl)\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Restored missing lifecycle management fields (heartbeat, orphan_timeout, etc)\n- Fixed constructor to match original signature\n- Simplified impl<J: Jail> instead of impl<J: crate::jail::Jail>\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Fixed missing ExitStatus import in managed.rs\n- Removed unused imports in jail/mod.rs\n- Replaced JailError with anyhow error handling\n- Made managed module available on both macOS and Linux\n- Moved test module to end of file to fix clippy warning\n- Restored get_temp_dir function\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
WeakJail doesn't create any system resources (just sets env vars), so it doesn't need lifecycle management with canary files. This also avoids permission issues when running weak tests after root tests.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Made PathBuf import conditional (Linux only) since it's not used on macOS\n- Applied cargo fmt\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Instead of using a separate CARGO_TARGET_DIR which causes a full recompilation, fix the permissions on the existing target directory. This should significantly speed up the CI pipeline.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Use --test weak_integration instead of just weak_integration\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
The weak_integration tests need the httpjail binary to exist. Added cargo build --release before running the tests.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Check for existing binary before building in tests\n- Support both debug and release binaries\n- Build binary during non-root test phase\n- Remove redundant build from weak test phase\n\nThis avoids unnecessary rebuilds and handles the case where the binary already exists from previous test runs.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Each test run will build the httpjail binary fresh using the dev profile. This ensures we're always testing the latest code and avoids any caching issues.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Use std::sync::Once to ensure the httpjail binary is only built once per test process lifetime. This significantly speeds up test execution when running multiple tests.\n\nAlso applied cargo fmt.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Replace unsafe static mut with std::sync::OnceLock for thread-safe one-time initialization. This fixes the clippy error about creating shared references to mutable statics, which is now denied by default in newer Rust versions.\n\nAlso applied cargo fmt.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
The weak_integration tests need the httpjail binary to exist. When running them separately in CI, we need to ensure the binary is built first.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Add helpful error messages when the binary is not found\n- Show the path being checked and current directory\n- Suggest manual build command if cargo fails\n- Display build output on failure\n\nThis will help debug CI failures more effectively.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
The test_https_allow function was accidentally nested inside test_https_blocking. This moves it to the module level so it can be imported by weak_integration tests.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
After building the httpjail binary, verify it actually exists and provide detailed diagnostics if not found, including the current directory and contents of target/debug.\n\nThis will help debug why the binary isn't being found in CI.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Build the httpjail binary once at the beginning of each CI job rather than separately for each test type. This ensures:\n- All tests use the same up-to-date binary\n- More efficient CI (build once, test many)\n- Better debugging with clear build output\n\nAlso added diagnostic output to show where the binary was built.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
- Check HTTPJAIL_BIN environment variable first\n- Handle CARGO_TARGET_DIR for finding the binary\n- Export the actual binary path in CI for tests to use\n- Provide better diagnostics when binary is not found\n\nThis should fix the issue where tests can't find the binary when cargo uses a different target directory.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Show more information about where we're looking for the binary and what we find, including directory listings before failing.\n\nThis will help us understand where cargo is actually putting the binary.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Instead of trying to figure out where cargo put the binary, just tell it exactly where to put it using --target-dir target.\n\nThis ensures the binary will always be at target/debug/httpjail.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Combine nested if statements into a single if-let with && condition as suggested by clippy.\n\nCo-authored-by: ammario <7416144+ammario@users.noreply.github.com>
Co-authored-by: ammario <7416144+ammario@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary\n- macOS: replace per-binary invocations with a single
cargo nextest run --profile ciso all unit + integration tests are exercised automatically.\n- Linux: run the entire suite as the regular user viacargo nextest run --profile ci, then runlinux_integrationunder sudo, and keep the feature-gated isolated-cleanup tests under sudo. This preserves root-only coverage without having to enumerate other tests.\n- Remove the dedicated “Weak Mode Integration Tests” job: weak tests are now covered by the macOS all-tests job (and by Linux as part of the all-tests run when applicable).\n\nRationale\n- Future-proofing: newly added tests (lib or tests/*.rs) are now picked up automatically on both OSes without editing CI.\n- OS safeguards: Linux-only tests are already cfg-gated intests/linux_integration.rs, so macOS won’t attempt to compile/run them; Linux continues to run those with sudo as before.\n\nVerification\n- Ensures the following are still exercised:\n - Unit tests and all integration tests including: smoke_test, script_integration, system_integration, weak_integration, test_flag, test_flag_methods.\n - linux_integration under sudo, plus theisolated-cleanup-testsfeature-gated cases via the explicit sudo step.\n\nNo code changes; CI-only.\n\nCo-authored-by: ammario 7416144+ammario@users.noreply.github.com