Skip to content

test(partitions): stop the removal test panicking when run as root - #4754

Open
coding-by-sea wants to merge 1 commit into
apache:masterfrom
coding-by-sea:removal-test-root-panic
Open

coding-by-sea wants to merge 1 commit into
apache:masterfrom
coding-by-sea:removal-test-root-panic

Conversation

@coding-by-sea

@coding-by-sea coding-by-sea commented Nov 4, 2026 •

Copy link
Copy Markdown

Which issue does this PR address?

Closes #4753

Rationale

a_removal_cut_short_leaves_the_marker_in_place panics when the partitions tests run as root, for example in a Docker container. The panic also stops cargo test -p partitions before the crate's integration tests run.

What changed?

Root ignores the 0o500 mode the test uses to cut the removal short, so remove_partition_dir deleted the whole partition directory. The test then restored the mode on the deleted offsets directory, and that unwrap() panicked before the early return for root.

The test now returns before the removal when the probe shows the mode is ignored. Runs as a regular user take the same path as before.

Local Execution

  • Passed. Toolchain 1.99.1 from rust-toolchain.toml, on master d0fc19a.
    • Linux, rust:1.99.1-slim-trixie as root: cargo test -p partitions --features poll-diagnostics passes, 753 tests. On master the test panics with NotFound.
    • Linux as a regular user (uid 1000): the same suite passes, 753 tests.
    • macOS: the same suite passes, 736 tests.
    • cargo clippy --all-features --all-targets -- -D warnings on the whole workspace (Linux) and cargo fmt --all --check are clean.
  • Pre-commit hooks ran: prek run --from-ref origin/master --to-ref HEAD, all applicable hooks pass.

AI Usage

  1. Claude (Claude Code).
  2. It found the bug while running the tests in a Linux container, wrote the issue text, this change and this description, and ran the checks above.
  3. I ran the test as root in a stock Rust container before and after the change, and the full crate suite as root, as a regular user and on macOS. The team-review skill ran on this head (2674c4ca) after the PR opened: all four reviewers approved with no findings. One optional simplification was raised, dropping the mode restore in the root branch, and I kept it so every path restores the mode like the sibling tests.
  4. Yes.

Root ignores the 0o500 mode the test uses to cut the removal short, so
remove_partition_dir deleted the whole partition directory. The test then
restored the mode on the deleted offsets directory, and that unwrap
panicked before the early return for root could run. The panic also
stopped cargo test before the crate's integration tests.

Return before the removal when the probe shows the mode is ignored, which
is what the test's comment already intends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

If AI was used to create this PR, you must run the team-review skill before opening it.

Slash commands (own line, regular comment) move it around the queue:

  • /ready - back to S-waiting-on-review after addressing feedback
  • /author - flip to S-waiting-on-author while you finish changes
  • /request-review @user-or-team - request a reviewer
  • /pin - exempt the PR from the stale bot, /unpin to undo

See CONTRIBUTING.md for details.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Nov 4, 2026
@codecov

codecov Bot commented Nov 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.66%. Comparing base (d0fc19a) to head (2674c4c).

Files with missing lines Patch % Lines
core/partitions/src/partition_storage.rs 66.66% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4754       +/-   ##
=============================================
- Coverage     91.03%   82.66%    -8.37%     
+ Complexity     3878     3875        -3     
=============================================
  Files          1561     1553        -8     
  Lines        403726   303949    -99777     
  Branches     337601   237827    -99774     
=============================================
- Hits         367515   251259   -116256     
- Misses        28851    45576    +16725     
+ Partials       7360     7114      -246     
Components Coverage Δ
Rust Core 81.83% <66.66%> (-10.45%) ⬇️
Java SDK 79.36% <ø> (-0.01%) ⬇️
C# SDK 84.37% <ø> (-0.06%) ⬇️
Python SDK 86.51% <ø> (ø)
PHP SDK 92.23% <ø> (ø)
Node SDK 96.62% <ø> (+0.02%) ⬆️
Go SDK 71.86% <ø> (+0.15%) ⬆️
C++ SDK 86.84% <ø> (ø)
Files with missing lines Coverage Δ
core/partitions/src/partition_storage.rs 85.69% <66.66%> (-0.13%) ⬇️

... and 516 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(partitions): a_removal_cut_short_leaves_the_marker_in_place panics when tests run as root

1 participant