Skip to content

test: bump TestBlockchain.DefaultTimeout 10s -> 30s#11486

Merged
smartprogrammer93 merged 1 commit into
masterfrom
fix/test-blockchain-default-timeout-flake
May 5, 2026
Merged

test: bump TestBlockchain.DefaultTimeout 10s -> 30s#11486
smartprogrammer93 merged 1 commit into
masterfrom
fix/test-blockchain-default-timeout-flake

Conversation

@LukaszRozmej
Copy link
Copy Markdown
Member

Summary

  • executePayloadV1_accepts_already_known_block(true) flakes on TEST_USE_FLAT=1 runs with TaskCanceledException at exactly 10s 489ms, inside TestBlockchain.Build() while waiting for the NewHeadBlock event after genesis loads.
  • The cancellation token used there is this.CancellationToken, which derives from TestTimeout (default 10000), not the test's [CancelAfter(30000)].
  • PR test: bump 10s timeouts that flake on Flat DB CI #11380 bumped per-test [CancelAfter] values from 10s/5s to 30s but missed this internal one. Aligning DefaultTimeout with that fix.

Stack from the failing run:

System.Threading.Tasks.TaskCanceledException : A task was canceled.
   at Nethermind.Core.Events.Wait.ForEventCondition[T](...) in Nethermind.Core/Events/Wait.cs:33
   at Nethermind.Core.Test.Blockchain.TestBlockchain.Build(...) in TestBlockchain.cs:227
   at MergeTestBlockchain.Build(ISpecProvider) in BaseEngineModuleTests.cs:292
   at EngineModuleTests.executePayloadV1_accepts_already_known_block(...) in EngineModuleTests.V1.cs:464

Side effects of the bump:

  • BlockProcessorTests.cs:146 uses DefaultTimeout * 10 — 100s ceiling becomes 300s. Just a higher upper bound; passing tests don't notice.
  • TestRpcBlockchain.cs:72 uses it as a fallback when no explicit timeout is passed.

Test plan

  • CI green on Nethermind tests (Flat DB) — specifically Nethermind.Merge.Plugin.Test.
  • No regressions in existing tests that rely on the timeout.

🤖 Generated with Claude Code

executePayloadV1_accepts_already_known_block(true) trips
TaskCanceledException at 10s 489ms inside TestBlockchain.Build()
on TEST_USE_FLAT=1 runs. The wait there uses this.CancellationToken,
which is fed by TestTimeout (= DefaultTimeout), not the test's
[CancelAfter(30000)]. PR #11380 bumped per-test [CancelAfter] values
but missed this internal one; aligning the default with that fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the test label May 4, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

Claude finished @LukaszRozmej's task in 1m 18s —— View job


Review

  • Read changed file and related files
  • Check side effects mentioned in PR description
  • Review test infrastructure rules
  • Post final review

Change: DefaultTimeout = 10000DefaultTimeout = 30000 in TestBlockchain.cs:49

The fix is correct and well-targeted. The root cause is clear: TestBlockchain.Build() at line 225 uses this.CancellationToken, which is sourced from CreateCancellationSource()AutoCancelTokenSource.ThatCancelAfter(TestTimeout) (line 390), where TestTimeout initializes from DefaultTimeout. This is a separate timeout budget from the NUnit [CancelAfter(30000)] on the test method itself — so bumping the NUnit attribute in PR #11380 left this internal one at 10s, still too tight for Flat DB genesis initialization.

Side effects verified:

  • BlockProcessorTests.cs:146DefaultTimeout * 10 becomes 300s (was 100s). This is a guard ceiling for SemaphoreSlim.WaitAsync; a passing test will never approach it. No concern.
  • TestRpcBlockchain.cs:72 — used as the fallback TestTimeout when callers pass no explicit timeout. Raising from 10s to 30s is appropriate given the same Flat DB slowdowns apply to RPC blockchain tests.

No issues found. The change is minimal, focused, and consistent with the intent of PR #11380. All known consumers of DefaultTimeout are accounted for in the PR description.

Verdict: approve.

@smartprogrammer93 smartprogrammer93 merged commit 1ce5d31 into master May 5, 2026
442 checks passed
@smartprogrammer93 smartprogrammer93 deleted the fix/test-blockchain-default-timeout-flake branch May 5, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants