Skip to content

style(drive-abci): rustfmt the keep-history deletion replay test - #4642

Closed
llbartekll wants to merge 1 commit into
v4.2-devfrom
style/rustfmt-keep-history-deletion-test
Closed

style(drive-abci): rustfmt the keep-history deletion replay test#4642
llbartekll wants to merge 1 commit into
v4.2-devfrom
style/rustfmt-keep-history-deletion-test

Conversation

@llbartekll

@llbartekll llbartekll commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Issue being fixed or feature implemented

cargo fmt --all -- --check currently fails on v4.2-dev itself. The
Rust wallet tests (macOS) job runs that check before the tests, so it exits 1
after ~14s and every PR branched off v4.2-dev shows red — including ones
that touch none of the affected code.

Reproduced on a pristine v4.2-dev checkout with nothing else applied:

Diff in packages/rs-drive-abci/src/execution/validation/state_transition/
state_transitions/batch/tests/document/deletion.rs:406

That is the only formatting violation in the whole workspace.

What was done?

Ran cargo fmt --all. The test name
test_document_delete_on_document_type_that_keeps_history_replays_protocol_version_13
is long enough that rustfmt wants the opening brace on its own line and the
call broken before .await; the merged form kept both on one line.

Pure rustfmt output — no logic, no test semantics, one file, six lines.

Introduced by 88b0427 (#4218).

How Has This Been Tested?

  • cargo fmt --all -- --checkexit 1 before, exit 0 after

The change is whitespace only, so it cannot affect behaviour; the surrounding
deletion_tests module is unchanged otherwise.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Style
    • Reformatted a document deletion test for improved readability.
    • No functional behavior changed.

`cargo fmt --all -- --check` fails on `v4.2-dev` at
`batch/tests/document/deletion.rs:406`. The test name is long enough that
rustfmt wants the opening brace on its own line and the call broken before
`.await`; the merged form kept both on one line.

This is the only formatting violation in the workspace, and it fails the
"Rust wallet tests (macOS)" job for every PR branched off `v4.2-dev` — the job
runs the fmt check before the tests, so nothing downstream of it gets to run.

Introduced by 88b0427 (#4218). Pure rustfmt output, no logic touched.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions github-actions Bot added this to the v4.2.0 milestone Sep 9, 2026
@coderabbitai

coderabbitai Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 11fac4c3-1f50-4ece-aa38-ce0627310ab0

📥 Commits

Reviewing files that changed from the base of the PR and between 22055ec and ef7c09a.

📒 Files selected for processing (1)
  • packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The pull request reformats one document deletion test. It does not change test behavior or public declarations.

Changes

Document deletion test

Layer / File(s) Summary
Test formatting
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs
The protocol version 13 test signature and helper call were split across multiple lines.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to ef7c0

This change only reformats a document-deletion replay test and does not alter its behavior or production functionality. It is ready to merge.

Suggested reviewers: claudius-maginificent, shumkov

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the formatting-only change to the keep-history deletion replay test and matches the pull request objectives.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch style/rustfmt-keep-history-deletion-test

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

thepastaclaw commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

🕓 Queued for automated review — 21st in line, estimated start in ~7 h (commit ef7c09a)
Estimated review time once started: ~40 min (two-phase automated review; median of recent runs).

  • Request priority review — tick this box and the review moves to the front of the queue.

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.38%. Comparing base (caf2b29) to head (ef7c09a).
⚠️ Report is 2 commits behind head on v4.2-dev.

Additional details and impacted files
@@             Coverage Diff              @@
##           v4.2-dev    #4642      +/-   ##
============================================
+ Coverage     85.34%   85.38%   +0.04%     
============================================
  Files          2795     2796       +1     
  Lines        373566   373611      +45     
============================================
+ Hits         318827   319020     +193     
+ Misses        54739    54591     -148     
Components Coverage Δ
dpp 84.50% <ø> (+0.13%) ⬆️
drive 84.95% <ø> (+0.01%) ⬆️
drive-abci 88.95% <ø> (+<0.01%) ⬆️
sdk ∅ <ø> (∅)
dapi-client ∅ <ø> (∅)
platform-version ∅ <ø> (∅)
platform-value 92.92% <ø> (ø)
platform-wallet ∅ <ø> (∅)
drive-proof-verifier 41.44% <ø> (ø)
🚀 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.

@llbartekll

Copy link
Copy Markdown
Contributor Author

Superseded — v4.2-dev picked up the same rustfmt fix via 22055ec (#4586), so cargo fmt --all -- --check is green on the current base. Closing as no longer needed.

🤖 Closed by Claude Code

@llbartekll llbartekll closed this Sep 9, 2026
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.

2 participants