Skip to content

Fix flaky/environment-dependent tests (macOS) - #4115

Merged
shemnon merged 5 commits into
mainfrom
shemnon/mac-test-hygene
Sep 9, 2026
Merged

Fix flaky/environment-dependent tests (macOS)#4115
shemnon merged 5 commits into
mainfrom
shemnon/mac-test-hygene

Conversation

@shemnon

@shemnon shemnon commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Stop double-logging recovered panics in evmrpc once a panic hook is set (the hook currently has a single caller, a test, that only needs the notification and never depended on the print/stack trace also firing)
  • Fix TestFreezeModeDisablesMempoolTraffic flaking on macOS: it reserved a listen address via TestReserveAddr and handed it to node.Start, but net.Listen can't adopt that reservation and the freed :0 bind is stealable by another process in the window before Start runs; the test only calls rpcEnv in-process, so skip the TCP listener entirely
  • Fix a timezone-dependent flake in the mint-release test: NewMinter derived start/end dates from local wall-clock time instead of UTC calendar dates, and MintRelease re-derived the expected release amount by hand instead of via Minter.GetReleaseAmountToday

Test plan

  • go test ./evmrpc/...
  • go test ./sei-tendermint/node/...
  • go test ./testutil/processblock/...

recoverAndLog printed the panic and stack trace unconditionally, then
also invoked the panic hook if one was set. Tests that install a hook
to capture panics were getting the default stderr logging on top of
it, adding noise. Fall back to the default logging only when no hook
handles the panic.
TestFreezeModeDisablesMempoolTraffic reserved an RPC listen address
via tcp.TestReserveAddr and handed it to node Start, but net.Listen
cannot adopt a reservation made by closing a :0 bind, and that address
is stealable by another process on macOS in the window between
reservation and Start. The test only calls rpcEnv in-process, so skip
the TCP listener entirely by leaving RPC.ListenAddress empty.
NewMinter derived the minter's start/end dates from local wall-clock
time, so the recorded calendar dates could shift by a day relative to
the block time depending on the local timezone, making the test flaky
outside UTC. Anchor start/end to UTC calendar dates instead.

MintRelease independently recomputed the expected release amount from
parsed start/end dates rather than through the minter's own release
calculation, which no longer agreed with the UTC-anchored dates. Use
Minter.GetReleaseAmountToday instead of re-deriving it.
@cursor

cursor Bot commented Sep 9, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are limited to test helpers, one tendermint test config, and evmrpc panic logging order with no production behavior change beyond optional hook-only logging.

Overview
Hardens test and recovery behavior so CI is less sensitive to timezone and listen-address races on macOS, and panic hooks behave as documented.

In evmrpc, recovered panics now only invoke SetPanicHook when one is registered—the default fmt.Printf + stack trace run only if no hook handles the panic (documented as replacing default logging).

Mint processblock tests build minter start/end from UTC midnight calendar dates instead of local time.Now(), and MintRelease asserts expected mint via Minter.GetReleaseAmountToday rather than hand-parsed date span math (mirrored under giga/deps/testutil).

TestFreezeModeDisablesMempoolTraffic sets cfg.RPC.ListenAddress to empty so the node does not bind TCP; the test exercises rpcEnv in-process and avoids the flaky TestReserveAddr / stealable :0 window on macOS.

Reviewed by Cursor Bugbot for commit afac58d. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedSep 9, 2026, 9:15 PM

@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 60.25%. Comparing base (c78d1c5) to head (afac58d).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4115      +/-   ##
==========================================
- Coverage   61.21%   60.25%   -0.97%     
==========================================
  Files        2196     2087     -109     
  Lines      193436   180445   -12991     
==========================================
- Hits       118416   108719    -9697     
+ Misses      63790    61580    -2210     
+ Partials    11230    10146    -1084     
Flag Coverage Δ
sei-chain-pr 71.14% <100.00%> (?)
sei-db 69.80% <ø> (ø)
sei-db-state-db ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
evmrpc/utils.go 72.81% <100.00%> (+0.12%) ⬆️
testutil/processblock/genesismint.go 100.00% <100.00%> (ø)
testutil/processblock/verify/mint.go 100.00% <100.00%> (+18.18%) ⬆️

... and 110 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@seidroid seidroid Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Three small, well-targeted flake fixes: the evmrpc panic-hook change keeps its only test passing unchanged and leaves production logging intact (no hook is set outside tests), and dropping the RPC listener from TestFreezeModeDisablesMempoolTraffic is safe since every assertion goes through rpcEnv in-process. The mint fix is right in direction, but the new expected-amount computation uses the block time instead of the epoch start time the mint hook actually uses, leaving a narrow midnight-crossing window.

Findings: 0 blocking | 1 non-blocking | 1 posted inline

Blockers

  • None at the file/PR level.

Non-blocking

  • None at the file/PR level.
  • 1 suggestion(s)/nit(s) flagged inline on specific lines.

Comment thread testutil/processblock/verify/mint.go Outdated

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6cefa3e. Configure here.

Comment thread testutil/processblock/verify/mint.go Outdated
Review feedback (seidroid, Cursor Bugbot) on the previous commit:
MintRelease computed the expected release amount from the post-block
BlockTime, but AfterEpochEnd computes the actual mint from the ended
epoch's own CurrentEpochStartTime. Those two timestamps are roughly
one epoch duration apart, so they can land on different UTC calendar
dates around midnight and reintroduce the same flake this test was
fixed to remove. oldEpoch already holds the timestamp the hook used.
giga/deps/testutil/processblock carries a copy of NewMinter and
MintRelease with the same timezone- and BlockTime-dependent bugs just
fixed in testutil/processblock. Apply the identical fixes here:
UTC-anchored start/end calendar dates, and computing the expected
release amount via Minter.GetReleaseAmountToday(oldEpoch.CurrentEpochStartTime)
instead of re-deriving it by hand.
@shemnon
shemnon added this pull request to the merge queue Sep 9, 2026
Merged via the queue into main with commit c9c9549 Sep 9, 2026
79 checks passed
@shemnon
shemnon deleted the shemnon/mac-test-hygene branch September 9, 2026 22:29
yzang2019 added a commit that referenced this pull request Sep 10, 2026
* main:
  Fix flaky/environment-dependent tests (macOS) (#4115)
  ci: drop unused Chrome/Microsoft apt sources before apt-get update (#4114)
  Move storage bench to sei-db root (#4109)
  Validate block part Merkle proof totals (CON-412) (#4105)
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.

2 participants