Skip to content

test: reduce macOS integration flakiness#5135

Open
eval-exec wants to merge 1 commit intonervosnetwork:developfrom
eval-exec:exec/macos-ci
Open

test: reduce macOS integration flakiness#5135
eval-exec wants to merge 1 commit intonervosnetwork:developfrom
eval-exec:exec/macos-ci

Conversation

@eval-exec
Copy link
Collaborator

@eval-exec eval-exec commented Mar 10, 2026

What problem does this PR solve?

Problem Summary:
Reduce flaky macOS integration failures in TransactionRelayConflict and SyncChurn.

What is changed and how it works?

What's Changed:

  • use tx status checks in TransactionRelayConflict
  • use a longer non-Linux sync timeout in SyncChurn

Related changes

Check List

Tests

  • Integration test
  • Manual test (add detailed scripts or steps below)

Manual test:

  • TransactionRelayConflict
  • SyncChurn

Copilot AI review requested due to automatic review settings March 10, 2026 02:45
@eval-exec eval-exec requested a review from a team as a code owner March 10, 2026 02:45
@eval-exec eval-exec requested review from quake and removed request for a team March 10, 2026 02:45
@eval-exec eval-exec marked this pull request as draft March 10, 2026 02:46
Copy link

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 fixes two sources of macOS CI integration test flakiness. TransactionRelayConflict was checking .transaction.is_some() which can be false even after the transaction is accepted into the tx pool — this is replaced with a tx status check. SyncChurn was timing out on slower non-Linux runners, so a configurable timeout mechanism is introduced with a longer value for non-Linux platforms.

Changes:

  • Introduced waiting_for_sync_with_timeout in test/src/node.rs to allow callers to specify a custom sync timeout, while keeping the existing waiting_for_sync as a 120-second-default wrapper.
  • Updated SyncChurn to use platform-specific sync timeouts (120s for Linux, 240s for non-Linux).
  • Switched TransactionRelayConflict relay assertions from checking transaction.is_some() to checking tx_status.status for Pending | Proposed, which more reliably reflects the intended pool membership check.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
test/src/node.rs Adds waiting_for_sync_with_timeout function; refactors waiting_for_sync to delegate to it with default timeout
test/src/specs/sync/sync_churn.rs Adds platform-specific SYNC_TIMEOUT_SECS constants and uses the new timeout-aware sync function
test/src/specs/relay/transaction_relay.rs Replaces .transaction.is_some() checks with tx_status.status pattern matching against `Pending

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

Copy link

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

test/src/specs/relay/transaction_relay.rs:335

  • This assertion message refers to relaying to node1, but the condition being waited on is that node0 sees tx2 (re-broadcast from node1). Update the message to match the actual direction/target to avoid confusing failures.

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

@eval-exec eval-exec marked this pull request as ready for review March 12, 2026 02:35
@zhangsoledad zhangsoledad added this pull request to the merge queue Mar 13, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Mar 13, 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.

3 participants