Skip to content

remove event assertion crate#763

Open
ozgunozerk wants to merge 1 commit into
mainfrom
remove-event-assertion
Open

remove event assertion crate#763
ozgunozerk wants to merge 1 commit into
mainfrom
remove-event-assertion

Conversation

@ozgunozerk

@ozgunozerk ozgunozerk commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Fixes #762

PR Checklist

  • Tests
  • Documentation

Summary by CodeRabbit

  • Documentation

    • Updated guidance to use typed event structs and direct event-log checks in examples and testing notes.
    • Removed references to the old test utility from project maps and structure docs.
  • Tests

    • Switched contract tests to validate emitted events directly from the test environment.
    • Improved event checks to verify both event counts and expected payloads.
  • Chores

    • Removed an unused test helper package and related workspace entries.

@ozgunozerk ozgunozerk requested a review from brozorec July 1, 2026 13:02
@ozgunozerk ozgunozerk self-assigned this Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The stellar-event-assertion crate is removed entirely (manifest and source), along with its workspace member entry and dependency references across all consuming crates. Documentation files describing the crate and its usage are updated. All test files across access, accounts, contract-utils, and tokens packages are migrated from EventAssertion helper calls to direct assertions on Soroban's captured event log, using e.events().all().events().len() for counts and to_xdr(&e, &address) comparisons for payload validation.

Changes

Event assertion crate removal and test migration

Layer / File(s) Summary
Documentation and workspace config
.claude/commands/code-quality.md, CLAUDE.md, Architecture.md, README.md, Cargo.toml
Removes references to the test-utils/event-assertion crate and documents the new typed #[contractevent] XDR-based event assertion approach.
Crate deletion and dependency cleanup
packages/test-utils/event-assertion/*, packages/access/Cargo.toml, packages/accounts/Cargo.toml, packages/contract-utils/Cargo.toml, packages/tokens/Cargo.toml
Deletes the stellar-event-assertion crate manifest and implementation, removes it from dependent Cargo.toml files, and adds replacement dev-dependencies (ark-*, k256, p256) where applicable.
Access control test migration
packages/access/src/access_control/test.rs, packages/access/src/ownable/test.rs
Replaces EventAssertion::new(...).assert_event_count(...) calls with direct e.events().all().events().len() checks.
Contract-utils test migration
packages/contract-utils/src/merkle_distributor/test.rs
Replaces event-count assertions with direct event log length checks.
Confidential token test migration
packages/tokens/src/confidential/*/test.rs
Migrates auditor, compliance, verifier, and core confidential tests to direct event-count assertions.
Fungible token test migration
packages/tokens/src/fungible/test.rs, packages/tokens/src/fungible/extensions/burnable/test.rs
Replaces EventAssertion checks with direct event collection and to_xdr payload comparisons for Mint/Burn/Approve/Transfer events.
Non-fungible token test migration
packages/tokens/src/non_fungible/**/test.rs
Migrates burnable, consecutive, and enumerable extension tests plus the core non-fungible test suite to direct event/XDR comparisons.
RWA module test migration
packages/tokens/src/rwa/**/test.rs
Updates compliance, doc manager, identity verification, identity registry storage, and token binder tests to direct event log length assertions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested reviewers: brozorec

Poem

A crate of helpers, hopped away,
No more "EventAssertion" to save the day.
Now XDR bytes compare with grace,
Each mint and burn found in its place. 🐰
Fewer lines to tend the burrow,
Less code to weed tomorrow!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The template’s change summary is missing; it only includes the issue reference and checklist, so the PR description is incomplete. Add a brief summary of the code and documentation changes, plus any needed context, under the description section while keeping the Fixes line and checklist.
Out of Scope Changes check ⚠️ Warning The added ark-grumpkin/ark-ec/ark-ff/ark-serialize and k256/p256 dev-dependencies are unrelated to removing the event assertion crate. Remove or justify the extra dependency additions unless they are required for the event-assertion replacement work.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly states the main change: removing the event assertion crate.
Linked Issues check ✅ Passed The code removes the crate, updates workspace/package deps, and rewrites tests to compare events via XDR, matching #762.
Docstring Coverage ✅ Passed Docstring coverage is 98.78% which is sufficient. The required threshold is 80.00%.
✨ 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 remove-event-assertion

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

@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.56%. Comparing base (7a3cd00) to head (639408b).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #763      +/-   ##
==========================================
- Coverage   96.63%   96.56%   -0.07%     
==========================================
  Files          72       71       -1     
  Lines        7615     7346     -269     
==========================================
- Hits         7359     7094     -265     
+ Misses        256      252       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (3)
packages/tokens/src/rwa/identity_verification/identity_registry_storage/test.rs (1)

4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Migration correct; counts verified across all seven test cases.

All event-count assertions (2, 3, 4, 3, 3, 5, 4) correctly match the documented add/modify/remove/recover event sequences.

As with the other files in this layer, only counts are checked rather than payload comparisons via .to_xdr(&e, &address) per coding guidelines; consistent with the stack's accepted tradeoff.

Also applies to: 52-52, 112-112, 194-194, 242-242, 276-276, 343-343, 710-710

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/tokens/src/rwa/identity_verification/identity_registry_storage/test.rs`
around lines 4 - 7, No code changes are needed here: the event-count assertions
in identity_registry_storage::test already match the expected seven test cases,
and the current count-only checks are consistent with the accepted testing
approach in this layer. Keep the existing imports and assertions as-is in
test_identity_registry_storage and the related event-verification test blocks.

Source: Coding guidelines

packages/tokens/src/rwa/test.rs (1)

4-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Migration correct; counts verified across all nine test cases.

All event-count assertions correctly account for setup_all_contracts (2 events) plus subsequent operation-specific events; math checks out for mint/burn/freeze/recover scenarios.

Also applies to: 131-131, 155-155, 179-179, 207-207, 313-313, 366-366, 393-393, 457-457

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/tokens/src/rwa/test.rs` around lines 4 - 8, The event-count
assertions in the RWA tests are already correct, so no logic change is needed;
keep the existing counts aligned with setup_all_contracts contributing 2 events
plus the operation-specific events in each of the listed test cases. If you
touch these tests, preserve the same event math in the relevant assertions so
the counts stay consistent across mint, burn, freeze, and recover scenarios.

Source: Coding guidelines

packages/tokens/src/rwa/identity_verification/claim_topics_and_issuers/test.rs (1)

3-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Migration correct; counts verified.

Import restructuring and all five event-count assertions (1, 3, 3, 4, 7) match the documented event math for each scenario.

Per coding guidelines, event assertions should ideally compare against the typed #[contractevent] struct via .to_xdr(&e, &address) rather than only counting. This file only checks counts everywhere; since the PR's linked issue explicitly accepts count-only assertions outside of the primary event-defining tests, this is likely intentional and consistent with the stack-wide approach, but flagging for awareness.

Also applies to: 37-37, 116-116, 248-248, 405-405, 566-566

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/tokens/src/rwa/identity_verification/claim_topics_and_issuers/test.rs`
around lines 3 - 7, The event-count assertions in the test module are
intentionally count-only here, so no behavioral change is needed for this file;
if you do touch these tests later, prefer asserting against the typed
#[contractevent] structs via .to_xdr(&e, &address) in the relevant scenario
tests rather than adding more count-only checks, and keep the existing import
setup around Events, Env, Address, and Vec as-is.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@packages/tokens/src/rwa/identity_verification/claim_topics_and_issuers/test.rs`:
- Around line 3-7: The event-count assertions in the test module are
intentionally count-only here, so no behavioral change is needed for this file;
if you do touch these tests later, prefer asserting against the typed
#[contractevent] structs via .to_xdr(&e, &address) in the relevant scenario
tests rather than adding more count-only checks, and keep the existing import
setup around Events, Env, Address, and Vec as-is.

In
`@packages/tokens/src/rwa/identity_verification/identity_registry_storage/test.rs`:
- Around line 4-7: No code changes are needed here: the event-count assertions
in identity_registry_storage::test already match the expected seven test cases,
and the current count-only checks are consistent with the accepted testing
approach in this layer. Keep the existing imports and assertions as-is in
test_identity_registry_storage and the related event-verification test blocks.

In `@packages/tokens/src/rwa/test.rs`:
- Around line 4-8: The event-count assertions in the RWA tests are already
correct, so no logic change is needed; keep the existing counts aligned with
setup_all_contracts contributing 2 events plus the operation-specific events in
each of the listed test cases. If you touch these tests, preserve the same event
math in the relevant assertions so the counts stay consistent across mint, burn,
freeze, and recover scenarios.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: cf33aad6-a57a-464f-a4c4-c50b0eff3b49

📥 Commits

Reviewing files that changed from the base of the PR and between 7a3cd00 and 639408b.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (34)
  • .claude/commands/code-quality.md
  • Architecture.md
  • CLAUDE.md
  • Cargo.toml
  • README.md
  • packages/access/Cargo.toml
  • packages/access/src/access_control/test.rs
  • packages/access/src/ownable/test.rs
  • packages/accounts/Cargo.toml
  • packages/contract-utils/Cargo.toml
  • packages/contract-utils/src/merkle_distributor/test.rs
  • packages/test-utils/event-assertion/Cargo.toml
  • packages/test-utils/event-assertion/src/lib.rs
  • packages/tokens/Cargo.toml
  • packages/tokens/src/confidential/auditor/test.rs
  • packages/tokens/src/confidential/compliance/test.rs
  • packages/tokens/src/confidential/test.rs
  • packages/tokens/src/confidential/verifier/test.rs
  • packages/tokens/src/fungible/extensions/burnable/test.rs
  • packages/tokens/src/fungible/test.rs
  • packages/tokens/src/non_fungible/extensions/burnable/test.rs
  • packages/tokens/src/non_fungible/extensions/consecutive/test.rs
  • packages/tokens/src/non_fungible/extensions/enumerable/test.rs
  • packages/tokens/src/non_fungible/test.rs
  • packages/tokens/src/rwa/compliance/modules/supply_limit/test.rs
  • packages/tokens/src/rwa/compliance/test.rs
  • packages/tokens/src/rwa/extensions/doc_manager/test.rs
  • packages/tokens/src/rwa/identity_verification/claim_issuer/test.rs
  • packages/tokens/src/rwa/identity_verification/claim_topics_and_issuers/test.rs
  • packages/tokens/src/rwa/identity_verification/identity_claims/test.rs
  • packages/tokens/src/rwa/identity_verification/identity_registry_storage/test.rs
  • packages/tokens/src/rwa/identity_verification/test.rs
  • packages/tokens/src/rwa/test.rs
  • packages/tokens/src/rwa/utils/token_binder/test.rs
💤 Files with no reviewable changes (9)
  • packages/test-utils/event-assertion/Cargo.toml
  • Architecture.md
  • README.md
  • packages/accounts/Cargo.toml
  • packages/access/Cargo.toml
  • packages/contract-utils/Cargo.toml
  • Cargo.toml
  • packages/tokens/Cargo.toml
  • packages/test-utils/event-assertion/src/lib.rs

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.

Remove event assertion crate

1 participant