Skip to content

test(platform-wallet): pass the built-lock observer in the input-cap test helper - #5734

Merged
QuantumExplorer merged 1 commit into
v5.1-devfrom
claude/fix-asset-lock-test-observer-arg
Nov 3, 2026
Merged

QuantumExplorer merged 1 commit into
v5.1-devfrom
claude/fix-asset-lock-test-observer-arg

Conversation

@QuantumExplorer

@QuantumExplorer QuantumExplorer commented Nov 3, 2026 •

Copy link
Copy Markdown
Member

Basic explanation

v5.1-dev does not compile its platform-wallet tests, so "Rust workspace tests" fails on every pull request into it. Two pull requests crossed: #5712 added a test helper calling broadcast_funded_asset_lock_with_operation with 7 arguments, and #5476, merged after it from a branch last tested before #5712 existed, gave that method an 8th parameter (on_built). This passes None there, as #5476's own test calls do.

Before (v5.1-dev at b0ccf7b and later):

error[E0061]: this method takes 8 arguments but 7 arguments were supplied
    --> packages/rs-platform-wallet/src/wallet/asset_lock/build.rs:4741:14
error: could not compile `platform-wallet` (lib test) due to 1 previous error

After: the crate's tests compile, and the four input-cap tests that use the helper run.

Issue being fixed or feature implemented

Each passed CI alone; together they leave the helper one argument short. Failing runs include https://github.com/dashpay/platform/actions/runs/41598837399/job/125794341548.

What was done?

broadcast_exact_registration_lock passes None for on_built: the input-cap tests that call it check what is refused, broadcast and handed back, not the observer.

How Has This Been Tested?

  • cargo clippy -p platform-wallet --all-targets --all-features --locked -- --no-deps -D warnings (the CI clippy command, on this crate): clean.
  • cargo test -p platform-wallet --all-features --locked --lib -- asset_lock::build: 48 passed, among them the four input-cap tests that call the helper.

Breaking Changes

None. Test code only.

Checklist:

  • I have performed a self-review of my own code
  • My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
  • I have made corresponding changes to the documentation if needed
  • If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
  • If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · c220cb6

  • Bots — thepastaclaw not yet — /skip-bots proceeds without the ones not yet reported
  • Self-review — post /self-reviewed
  • Build running
  • Approvals
    • rs-platform-wallet (packages/rs-platform-wallet/src/wallet/asset_lock/build.rs) — HashEngineering or ZocoLini or llbartekll or romchornyi

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

Summary by CodeRabbit

  • Tests
    • Updated a test helper to pass the optional lock observer as None.

…test helper

#5712 added `broadcast_exact_registration_lock`, calling
`broadcast_funded_asset_lock_with_operation` with 7 arguments, and #5476,
merged after it from a branch last tested before #5712 existed, gave
that method an 8th parameter (`on_built`). v5.1-dev's platform-wallet
tests stopped compiling (E0061 at asset_lock/build.rs:4741), failing
"Rust workspace tests" on every pull request into it. The helper passes
`None`, as #5476's own test calls do.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@github-actions github-actions Bot added this to the v5.1.0 milestone Nov 3, 2026
@coderabbitai

coderabbitai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The exact-registration lock test helper now passes None for the optional observer argument to broadcast_funded_asset_lock_with_operation.

Changes: Asset-lock test helper

Layer / File(s) Summary
Pass the optional observer argument
packages/rs-platform-wallet/src/wallet/asset_lock/build.rs
The exact-registration lock test helper passes None to broadcast_funded_asset_lock_with_operation.

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

Change: Bug fix

Possibly related PRs

Suggested reviewers: llbartekll

Merge Risk: Minimal · up to c220c

The test helper now supplies the missing optional argument correctly. No actionable merge-blocking risk remains.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title names the change: a test helper in platform-wallet now passes the built-lock observer argument. The raw summary confirms that the exact-registration-lock test helper now passes None for the …
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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.
Full details: Title check

Explanation

The title names the change: a test helper in platform-wallet now passes the built-lock observer argument. The raw summary confirms that the exact-registration-lock test helper now passes None for the optional built-lock observer argument, and the objectives state the same fix for the missing on_built argument. The title is specific and matches the single changed file, so it is related to the main change. The title uses the test(...) prefix, which fits a test-only change.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches
Generate docstrings
  • Commit to this branch
  • Create a new PR
Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 5ec9631 and c220cb6.

Files selected for processing: 1
  • packages/rs-platform-wallet/src/wallet/asset_lock/build.rs

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

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Nov 3, 2026
@thepastaclaw

Copy link
Copy Markdown
Collaborator

🕓 Review not started yet because the new head is waiting for the 30-minute push debounce.

  • Request normal review — click when the PR is ready for review.
  • Request priority review — click to move this review to the front of the queue.

Commit c220cb6. Normal review starts when eligible; priority review starts as soon as a slot is available.

@QuantumExplorer
QuantumExplorer merged commit b3b664d into v5.1-dev Nov 3, 2026
13 of 14 checks passed
@QuantumExplorer
QuantumExplorer deleted the claude/fix-asset-lock-test-observer-arg branch November 3, 2026 09:54
QuantumExplorer added a commit that referenced this pull request Nov 3, 2026
Brings in #5734, which fixes the platform-wallet test helper that two
earlier base merges left calling broadcast_funded_asset_lock_with_operation
without its new observer argument (the Clippy lints failure here).

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

waiting-bots Waiting for the review bots to report on this head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants