test(interpreter): make the stray-bytecode e2e portable and cache-isolated - #1544
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
9af4d14 to
0334d5d
Compare
✨ Aspect Workflows Tasks📅 Thu Sep 17 04:55:14 UTC 2026 ✅ 44 successful tasks
⏱ Last updated Thu Sep 17 05:12:22 UTC 2026 · 📊 GitHub API quota 2,180/7,700 (28% used, resets in 24m) |
0334d5d to
9754307
Compare
py_binary startup benchmark
sys.path quality
Bazel analysis benchmark
py_image_layer benchmark
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9754307bb6
ℹ️ 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
- 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 address that feedback".
…lated The cleanup used `rmdir --ignore-fail-on-non-empty`, a GNU-only flag that BSD rmdir rejects, so on macOS the planted `__pycache__` directories were left behind. Remove each planted file and directory individually, tolerating a failed rmdir, with no dependence on xargs or rmdir flags. The test writes into a fetched interpreter repo. With the repo contents cache, on by default since Bazel 9, `external/<repo>` is a symlink into a cache shared by every workspace of the user, so the planted files were visible to unrelated builds while the test ran and would outlive it if the shell were killed. Disable the cache for this workspace so the repo is fetched into its own output base. The flag exists on Bazel 8 too, where the cache is off by default, so the setting is a no-op there. Verified on macOS with a fresh output base under Bazel 8.6 and 9.2: the repo is a plain directory, the script passes, and no planted file remains in either the output base or the shared cache.
9754307 to
82defc4
Compare
The
interpreter-toolchain-settingse2e added in #1540 plants bytecode inside a fetched interpreter repo and cleans up on exit.rmdir --ignore-fail-on-non-empty, a GNU-only flag that BSDrmdirrejects, so on macOS the planted__pycache__directories were left behind. Each directory is now removed individually, tolerating a failure.external/<repo>is a symlink into a cache shared by every workspace of the user, so the planted files were visible to unrelated builds during the run and would survive a killed shell. The workspace.bazelrcnow disables that cache so the repo is fetched into its own output base. The flag exists on Bazel 8 as well, where the cache is already off by default.Changes are visible to end-users: no
Test plan
e2e/interpreter-toolchain-settings/test.shon macOS with a fresh output base under Bazel 8.6 and 9.2: the interpreter repo is a plain directory, the script passes, and no planted file remains in the output base or the shared cache.