Skip to content

chore: fix GetActorEventsRaw tests #6319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
LesnyRumcajs merged 1 commit into from
Dec 8, 2025
Merged

chore: fix GetActorEventsRaw tests #6319

LesnyRumcajs merged 1 commit into from
Dec 8, 2025

Conversation

@LesnyRumcajs
Copy link
Member

@LesnyRumcajs LesnyRumcajs commented Dec 8, 2025 •
edited by coderabbitai bot
Loading

Summary of changes

RPC tests fail with:

2025-12-08T00:45:48.3212958Z           "forest_status": {
2025-12-08T00:45:48.3213172Z             "rejected": "filter matches too many events, try a more restricted filter"
2025-12-08T00:45:48.3213214Z           },
2025-12-08T00:45:48.3213261Z           "lotus_status": {
2025-12-08T00:45:48.3213468Z             "rejected": "get events for filter: failed to get events: filter matches too many events, try a more restricted filter"
2025-12-08T00:45:48.3213508Z           }

I guess Lotus added a bit of wrappers in the recent version. PassWithQuasiIdenticalError should resolve it (it makes the test pass if Forest error message is a substring of the Lotus error message).

Changes introduced in this pull request:

  • relaxed requirements on GetActorEventsRaw tests to allow for sub-errors.

Reference issue to close (if applicable)

Closes #6316

Other information and links

Change checklist

  • I have performed a self-review of my own code,
  • I have made corresponding changes to the documentation. All new code adheres to the team's documentation standards,
  • I have added tests that prove my fix is effective or that my feature works (if possible),
  • I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

Summary by CodeRabbit

Release Notes

  • Tests
    • Improved API comparison tests to more accurately recognize equivalent error responses across different client implementations. Quasi-identical errors are now properly validated during compatibility checks rather than being treated as failures. Updates applied to event query tests with various filters and configurations.

✏️ Tip: You can customize this high-level summary in your review settings.

@LesnyRumcajs LesnyRumcajs requested a review from a team as a code owner December 8, 2025 08:45
@LesnyRumcajs LesnyRumcajs requested review from hanabi1224 and sudo-shashank and removed request for a team December 8, 2025 08:45
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 8, 2025 •
edited
Loading

Walkthrough

Adds policyOnRejected: PassWithQuasiIdenticalError to multiple RpcTest identities in event-tests-with-tipset for various GetActorEventsRaw and related tests. This updates how rejected responses are handled when comparing Forest and Lotus implementations, allowing quasi-identical errors to pass validation.

Changes

Cohort / File(s) Summary
RPC test rejection policy updates
src/tool/subcommands/api_cmd/api_compare_tests.rs		 Adds policyOnRejected: PassWithQuasiIdenticalError policy to multiple RpcTest entries in event-tests-with-tipset, affecting GetActorEventsRaw tests with various filter configurations and tipset key combinations

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single file with repetitive, homogeneous configuration additions across test entries
  • No logic changes or control flow modifications
  • Straightforward policy application to existing test structures

Possibly related PRs

Suggested labels

RPC

Suggested reviewers

  • hanabi1224
  • sudo-shashank

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'chore: fix GetActorEventsRaw tests' accurately describes the main change—fixing failing RPC parity tests for GetActorEventsRaw by adjusting rejection handling policy.
Linked Issues check ✅ Passed The PR successfully addresses the linked issue #6316 by relaxing GetActorEventsRaw test assertions to handle error message format changes from Lotus, using PassWithQuasiIdenticalError to restore RPC parity test success.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing GetActorEventsRaw RPC parity tests by adjusting rejection policies; no unrelated modifications to other components or functionality are present.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-actor-events-tests
📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aabb26c and d87e7a0.

📒 Files selected for processing (1)
  • src/tool/subcommands/api_cmd/api_compare_tests.rs (6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings 
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 5907
File: src/rpc/methods/state.rs:523-570
Timestamp: 2025-08-06T15:44:33.467Z
Learning: LesnyRumcajs prefers to rely on BufWriter's Drop implementation for automatic flushing rather than explicit flush() calls in Forest codebase.
🧬 Code graph analysis (1)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)
src/rpc/client.rs (1)
  • request (258-271)
src/rpc/reflect/mod.rs (1)
  • request (290-300)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
  • GitHub Check: cargo-publish-dry-run
  • GitHub Check: Build MacOS
  • GitHub Check: Build Ubuntu
  • GitHub Check: Build forest binaries on Linux AMD64
  • GitHub Check: All lint checks
  • GitHub Check: tests
  • GitHub Check: tests-release
🔇 Additional comments (2)
src/tool/subcommands/api_cmd/api_compare_tests.rs (2)

657-658: Policy change looks appropriate, but verify sort_policy consistency.

The PassWithQuasiIdenticalError policy correctly handles Lotus adding wrapper text to error messages. However, this test lacks .sort_policy(SortPolicy::All) while all other GetActorEventsRaw tests in this function have it (lines 670, 682, 694, 710, 741).

Is this intentional because the None filter case always returns an error, or should this test also include the sort policy for consistency?

669-741: LGTM! Consistent policy application across all event filter variants.

The PassWithQuasiIdenticalError policy is correctly applied to all GetActorEventsRaw test variants (height filters, tipset keys, addresses, and field filters). This appropriately handles the case where Lotus wraps error messages with additional context while Forest returns the core error message.

Note that the policy implementation (line 2439-2441) allows bidirectional matching—either error message can be a substring of the other—which provides flexibility for either implementation to have different levels of error detail.

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

@LesnyRumcajs LesnyRumcajs added the RPC requires calibnet RPC checks to run on CI label Dec 8, 2025
@LesnyRumcajs LesnyRumcajs added this pull request to the merge queue Dec 8, 2025
Merged via the queue into main with commit de39853 Dec 8, 2025
42 of 66 checks passed
@LesnyRumcajs LesnyRumcajs deleted the fix-actor-events-tests branch December 8, 2025 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanabi1224 hanabi1224 hanabi1224 approved these changes

@sudo-shashank sudo-shashank sudo-shashank approved these changes

Assignees

No one assigned

Labels

RPC requires calibnet RPC checks to run on CI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[automated] RPC parity test failure @ 8/12/25 00:45

4 participants

@LesnyRumcajs @hanabi1224 @sudo-shashank