Skip to content

test(cketh): Add retry mechanism to assert_has_unique_events_in_order#10099

Merged
gregorydemay merged 3 commits into
masterfrom
gdemay/EXC-1783-fix-ckerc20-int-tests
May 6, 2026
Merged

test(cketh): Add retry mechanism to assert_has_unique_events_in_order#10099
gregorydemay merged 3 commits into
masterfrom
gdemay/EXC-1783-fix-ckerc20-int-tests

Conversation

@gregorydemay
Copy link
Copy Markdown
Contributor

@gregorydemay gregorydemay commented May 6, 2026

Upcoming message-execution changes (#10090) caused some events to not yet be visible at the moment cketh integration tests called assert_has_unique_events_in_order, leading to test failures. Add a retry mechanism to assert_has_unique_events_in_order to make the tests more robust.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the ckETH test utilities to make event-order assertions robust to recent changes in message execution timing by retrying the assertion after advancing the state machine.

Changes:

  • Add a retry loop with env.tick() when asserting events are unique and in-order, to allow delayed event emission to settle.
  • Refactor the event uniqueness/order checking logic into a helper (check_unique_events_in_order) returning Result for reuse across retries.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@gregorydemay gregorydemay force-pushed the gdemay/EXC-1783-fix-ckerc20-int-tests branch from a73be81 to 3bb1d61 Compare May 6, 2026 08:54
@gregorydemay gregorydemay changed the base branch from alin/EXC-1783-no-execution-overhead-on-low-cycles to master May 6, 2026 08:55
gregorydemay and others added 2 commits May 6, 2026 08:57
Replace the brittle per-call-site `env.tick()` workaround in the cketh
integration tests with an internal retry loop in
`assert_has_unique_events_in_order` that ticks and re-fetches events up to
5 times before failing, so future changes to message execution don't
require sprinkling more ticks throughout the test suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gregorydemay gregorydemay force-pushed the gdemay/EXC-1783-fix-ckerc20-int-tests branch from 3bb1d61 to 81f7caa Compare May 6, 2026 08:57
@gregorydemay gregorydemay changed the title fix: add ticks due to change in message execution. test(cketh): Add retry mechanism to assert_has_unique_events_in_order May 6, 2026
@github-actions github-actions Bot added test and removed fix labels May 6, 2026
@gregorydemay gregorydemay marked this pull request as ready for review May 6, 2026 09:00
@gregorydemay gregorydemay requested a review from a team as a code owner May 6, 2026 09:00
@github-actions github-actions Bot added the @defi label May 6, 2026
Copy link
Copy Markdown
Contributor

@mducroux mducroux left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@mbjorkqvist mbjorkqvist left a comment

Choose a reason for hiding this comment

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

Thanks @gregorydemay, just one minor comment!

Comment thread rs/ethereum/cketh/test_utils/src/events.rs Outdated
Address review feedback on PR #10099: distinguish the transient
"missing event" failure (which the tick-and-retry loop is meant to
recover from) from the structural "duplicate" and "out-of-order"
failures, which can never become correct by ticking. Replace the
String error with a CheckError enum and fail fast on the structural
variants.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gregorydemay gregorydemay enabled auto-merge May 6, 2026 10:03
@gregorydemay gregorydemay added this pull request to the merge queue May 6, 2026
Merged via the queue into master with commit 26db60a May 6, 2026
37 checks passed
@gregorydemay gregorydemay deleted the gdemay/EXC-1783-fix-ckerc20-int-tests branch May 6, 2026 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants