Skip to content

test(e2e): replace hardcoded time.Sleep with condition-polling in gated tests (#998)#1010

Merged
oskarszoon merged 8 commits into
bsv-blockchain:mainfrom
oskarszoon:fix/test-sleep
Jun 2, 2026
Merged

test(e2e): replace hardcoded time.Sleep with condition-polling in gated tests (#998)#1010
oskarszoon merged 8 commits into
bsv-blockchain:mainfrom
oskarszoon:fix/test-sleep

Conversation

@oskarszoon

@oskarszoon oskarszoon commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

What

Replace hardcoded time.Sleep "sleep-then-assert" waits with poll-until-condition (or delete redundant sleeps) across the running gated e2e tests. Cuts serial wall-clock (the gated suites run -parallel 1) and removes a flakiness source. Test-only — no production code, CI, or Makefile changes.

Closes #998.

Changes

  • banlist — deleted 8 redundant "wait for node ready" sleeps. daemon.NewTestDaemon already blocks on readiness before returning (readyCh 30s + WaitForHealthLiveness 10s + FSM RUNNING); the first client/RPC call self-errors if not ready. Kept the banDuration + 1s ban-TTL wait — genuine wall-clock, polling can't shorten it.
  • multi_node_send_2_tx — 10s sleep → require.Eventually polling GetTransactionHashes for len == 2.
  • utxo (TestDeleteAtHeightHappyPath) — deleted a 10s sleep that guarded commented-out assertions (//require.Error, // require.Nil).
  • block_assembly_restart — deleted 3× 2s sleeps that each immediately preceded WaitForTransactionInBlockAssembly (already polls).
  • block_persister — 2× 2s sleeps → require.Eventually polling GetBlocksNotPersisted empty (15s/500ms).
  • pruner_parent_before_child — :204 → WaitForPruner (real completion barrier: EnablePruner: true registers the observer); :362 → WaitForTransactionInBlockAssembly.
  • legacy_sync — comments only: documented the intentional inter-block timestamp pacing (svnode rejects blocks with too-close timestamps).

Verified

  • go build + go vet clean across test/e2e/daemon/ready/... and test/sequentialtest/block_assembly/...; gofmt clean on all 7 files.
  • Ran locally (without -race, see note): banlist (3×), multi_node_send_2_tx, utxo, block_assembly_restart, block_persister (both), pruner_parent (both) — all PASS. Per-file wall-clock dropped: banlist ~27s/run, multi_node ~9s, utxo ~10s, block_assembly_restart ~6s.
  • legacy_sync: build + vet only — needs an SV node; relies on the CI legacy-sync gate.

Out of scope (left intentionally — no benefit)

  • reorg_test.go and smoke_test.go target sleeps: all inside t.Skip'd tests (TestDynamicSubtreeSize, TestTracing, TestSendTxDeleteParentResendTx, TestTransactionPurgeAndSyncConflicting). They never run, so converting saves zero wall-clock and can't be run-verified. The issue's ~99s estimate counted ~17s here that doesn't execute.
  • legacy_sync 500ms sleeps: genuine inter-block timestamp pacing + ticks inside existing poll loops — not the anti-pattern.
  • wip/* sleeps: not run by the smoke gate.

Notes

  • -race currently fails on a pre-existing libp2p NAT data race (go-libp2p natpmp.randomPort), unrelated to this change — fails identically before and after on these commits.
  • pruner_parent_before_child has a latent WaitForBlockPersisted timeout under container contention (~line 390), pre-existing (akin to Flaky: services/blockassembly system tests (FSM-startup / gRPC connection races under CI load) #997).
  • WaitForPruner at :204 waits for a completed prune cycle, not provably the child1-DAH-height cycle — same property as the sleep it replaced. The :204 assertions only check transactions that must survive, so an early return can't mask a false negative; the same helper backs the stable :392 "parent was pruned" assertion.

oskarszoon added 7 commits June 1, 2026 17:14
@github-actions

github-actions Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🤖 Claude Code Review

Status: Complete

This PR correctly replaces fixed time.Sleep delays with condition-based polling or removes redundant waits. The changes are test-only, methodical, and well-justified. All helper methods (WaitForPruner, WaitForTransactionInBlockAssembly, require.Eventually) exist and function correctly.

No issues found.

The PR description accurately documents changes, verification steps, and out-of-scope items. The intentional sleeps (ban TTL expiration, inter-block timestamp pacing) are correctly preserved with explanatory comments.

@github-actions

github-actions Bot commented Jun 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Benchmark Comparison Report

Baseline: main (unknown)

Current: PR-1010 (33063d0)

Summary

  • Regressions: 0
  • Improvements: 0
  • Unchanged: 144
  • Significance level: p < 0.05
All benchmark results (sec/op)
Benchmark Baseline Current Change p-value
_NewBlockFromBytes-4 1.782µ 1.783µ ~ 1.000
SplitSyncedParentMap_SetIfNotExists/256_buckets-4 61.50n 61.62n ~ 0.700
SplitSyncedParentMap_SetIfNotExists/16_buckets-4 61.57n 62.62n ~ 0.200
SplitSyncedParentMap_SetIfNotExists/1_bucket-4 61.48n 61.64n ~ 0.300
SplitSyncedParentMap_ConcurrentSetIfNotExists/256_buckets... 30.25n 30.95n ~ 0.400
SplitSyncedParentMap_ConcurrentSetIfNotExists/16_buckets_... 50.77n 52.26n ~ 0.100
SplitSyncedParentMap_ConcurrentSetIfNotExists/1_bucket_pa... 108.9n 105.5n ~ 0.700
MiningCandidate_Stringify_Short-4 259.4n 259.1n ~ 1.000
MiningCandidate_Stringify_Long-4 1.916µ 1.914µ ~ 0.600
MiningSolution_Stringify-4 967.9n 974.8n ~ 0.200
BlockInfo_MarshalJSON-4 1.802µ 1.803µ ~ 1.000
NewFromBytes-4 129.2n 155.2n ~ 0.700
AddTxBatchColumnar_Validation-4 2.610µ 2.461µ ~ 0.400
OffsetValidationLoop-4 633.8n 636.8n ~ 1.000
Mine_EasyDifficulty-4 67.62µ 67.40µ ~ 0.700
Mine_WithAddress-4 7.010µ 7.676µ ~ 0.100
DiskTxMap_SetIfNotExists-4 3.518µ 3.551µ ~ 1.000
DiskTxMap_SetIfNotExists_Parallel-4 3.297µ 3.409µ ~ 0.700
DiskTxMap_ExistenceOnly-4 305.3n 312.9n ~ 0.700
Queue-4 195.6n 196.3n ~ 0.400
AtomicPointer-4 4.702n 5.086n ~ 0.700
ReorgOptimizations/DedupFilterPipeline/Old/10K-4 895.7µ 879.1µ ~ 0.200
ReorgOptimizations/DedupFilterPipeline/New/10K-4 825.7µ 789.4µ ~ 0.100
ReorgOptimizations/AllMarkFalse/Old/10K-4 109.6µ 110.4µ ~ 1.000
ReorgOptimizations/AllMarkFalse/New/10K-4 62.93µ 62.81µ ~ 0.700
ReorgOptimizations/HashSlicePool/Old/10K-4 59.75µ 62.75µ ~ 1.000
ReorgOptimizations/HashSlicePool/New/10K-4 12.49µ 11.52µ ~ 0.100
ReorgOptimizations/NodeFlags/Old/10K-4 4.661µ 4.849µ ~ 0.700
ReorgOptimizations/NodeFlags/New/10K-4 1.623µ 1.640µ ~ 0.200
ReorgOptimizations/DedupFilterPipeline/Old/100K-4 9.655m 10.118m ~ 0.100
ReorgOptimizations/DedupFilterPipeline/New/100K-4 10.138m 9.827m ~ 0.400
ReorgOptimizations/AllMarkFalse/Old/100K-4 1.144m 1.160m ~ 0.700
ReorgOptimizations/AllMarkFalse/New/100K-4 686.1µ 689.8µ ~ 1.000
ReorgOptimizations/HashSlicePool/Old/100K-4 662.4µ 548.1µ ~ 0.100
ReorgOptimizations/HashSlicePool/New/100K-4 315.9µ 315.1µ ~ 1.000
ReorgOptimizations/NodeFlags/Old/100K-4 51.53µ 53.09µ ~ 0.700
ReorgOptimizations/NodeFlags/New/100K-4 18.18µ 18.66µ ~ 0.600
TxMapSetIfNotExists-4 52.25n 52.34n ~ 1.000
TxMapSetIfNotExistsDuplicate-4 40.60n 40.62n ~ 1.000
ChannelSendReceive-4 596.2n 600.9n ~ 0.700
BlockAssembler_AddTx-4 0.02719n 0.02694n ~ 0.700
AddNode-4 10.76 10.81 ~ 1.000
AddNodeWithMap-4 11.20 11.08 ~ 1.000
DirectSubtreeAdd/4_per_subtree-4 58.14n 56.57n ~ 0.200
DirectSubtreeAdd/64_per_subtree-4 28.89n 29.48n ~ 0.700
DirectSubtreeAdd/256_per_subtree-4 28.62n 27.97n ~ 0.700
DirectSubtreeAdd/1024_per_subtree-4 27.02n 26.65n ~ 0.700
DirectSubtreeAdd/2048_per_subtree-4 26.71n 26.20n ~ 0.700
SubtreeProcessorAdd/4_per_subtree-4 299.3n 314.9n ~ 0.100
SubtreeProcessorAdd/64_per_subtree-4 291.7n 294.1n ~ 0.200
SubtreeProcessorAdd/256_per_subtree-4 288.7n 299.4n ~ 0.100
SubtreeProcessorAdd/1024_per_subtree-4 281.8n 297.6n ~ 0.100
SubtreeProcessorAdd/2048_per_subtree-4 280.3n 291.8n ~ 0.200
SubtreeProcessorRotate/4_per_subtree-4 288.4n 288.7n ~ 1.000
SubtreeProcessorRotate/64_per_subtree-4 287.1n 289.9n ~ 0.700
SubtreeProcessorRotate/256_per_subtree-4 290.0n 285.6n ~ 1.000
SubtreeProcessorRotate/1024_per_subtree-4 285.2n 285.8n ~ 1.000
SubtreeNodeAddOnly/4_per_subtree-4 55.00n 55.85n ~ 1.000
SubtreeNodeAddOnly/64_per_subtree-4 36.03n 36.09n ~ 0.200
SubtreeNodeAddOnly/256_per_subtree-4 34.87n 35.47n ~ 0.100
SubtreeNodeAddOnly/1024_per_subtree-4 34.45n 34.73n ~ 0.700
SubtreeCreationOnly/4_per_subtree-4 111.1n 111.4n ~ 0.500
SubtreeCreationOnly/64_per_subtree-4 351.9n 354.1n ~ 0.400
SubtreeCreationOnly/256_per_subtree-4 1.250µ 1.257µ ~ 1.000
SubtreeCreationOnly/1024_per_subtree-4 3.823µ 3.786µ ~ 0.400
SubtreeCreationOnly/2048_per_subtree-4 6.968µ 6.977µ ~ 1.000
SubtreeProcessorOverheadBreakdown/64_per_subtree-4 284.7n 298.3n ~ 0.100
SubtreeProcessorOverheadBreakdown/1024_per_subtree-4 281.2n 297.7n ~ 0.100
ParallelGetAndSetIfNotExists/1k_nodes-4 2.044m 2.054m ~ 0.400
ParallelGetAndSetIfNotExists/10k_nodes-4 5.334m 5.461m ~ 0.200
ParallelGetAndSetIfNotExists/50k_nodes-4 7.492m 7.901m ~ 0.100
ParallelGetAndSetIfNotExists/100k_nodes-4 10.28m 10.67m ~ 0.100
SequentialGetAndSetIfNotExists/1k_nodes-4 1.808m 1.793m ~ 0.200
SequentialGetAndSetIfNotExists/10k_nodes-4 5.244m 4.821m ~ 0.200
SequentialGetAndSetIfNotExists/50k_nodes-4 14.08m 13.90m ~ 0.100
SequentialGetAndSetIfNotExists/100k_nodes-4 26.21m 25.47m ~ 0.100
ProcessOwnBlockSubtreeNodesParallel/1k_nodes-4 2.138m 2.108m ~ 0.200
ProcessOwnBlockSubtreeNodesParallel/10k_nodes-4 8.721m 8.676m ~ 0.700
ProcessOwnBlockSubtreeNodesParallel/100k_nodes-4 14.10m 13.98m ~ 0.400
ProcessOwnBlockSubtreeNodesSequential/1k_nodes-4 1.792m 1.830m ~ 0.400
ProcessOwnBlockSubtreeNodesSequential/10k_nodes-4 8.158m 8.686m ~ 0.100
ProcessOwnBlockSubtreeNodesSequential/100k_nodes-4 43.37m 45.79m ~ 0.200
CalcBlockWork-4 471.8n 474.7n ~ 0.400
CalculateWork-4 648.4n 660.8n ~ 0.700
BuildBlockLocatorString_Helpers/Size_10-4 1.386µ 1.543µ ~ 0.100
BuildBlockLocatorString_Helpers/Size_100-4 13.28µ 13.25µ ~ 0.700
BuildBlockLocatorString_Helpers/Size_1000-4 133.2µ 131.9µ ~ 0.700
CatchupWithHeaderCache-4 104.7m 104.6m ~ 0.700
_BufferPoolAllocation/16KB-4 4.091µ 5.494µ ~ 0.100
_BufferPoolAllocation/32KB-4 8.531µ 8.801µ ~ 0.200
_BufferPoolAllocation/64KB-4 22.14µ 17.18µ ~ 0.100
_BufferPoolAllocation/128KB-4 35.91µ 27.47µ ~ 0.200
_BufferPoolAllocation/512KB-4 121.0µ 117.9µ ~ 0.700
_BufferPoolConcurrent/32KB-4 20.57µ 21.35µ ~ 0.700
_BufferPoolConcurrent/64KB-4 32.78µ 37.27µ ~ 0.700
_BufferPoolConcurrent/512KB-4 159.8µ 165.3µ ~ 0.400
_SubtreeDeserializationWithBufferSizes/16KB-4 660.6µ 723.5µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/32KB-4 668.1µ 721.4µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/64KB-4 655.4µ 714.0µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/128KB-4 653.7µ 701.8µ ~ 0.100
_SubtreeDeserializationWithBufferSizes/512KB-4 674.0µ 674.6µ ~ 1.000
_SubtreeDataDeserializationWithBufferSizes/16KB-4 37.20m 37.37m ~ 0.200
_SubtreeDataDeserializationWithBufferSizes/32KB-4 37.01m 37.39m ~ 0.100
_SubtreeDataDeserializationWithBufferSizes/64KB-4 37.11m 37.29m ~ 0.700
_SubtreeDataDeserializationWithBufferSizes/128KB-4 37.04m 37.34m ~ 0.400
_SubtreeDataDeserializationWithBufferSizes/512KB-4 36.95m 37.49m ~ 0.100
_PooledVsNonPooled/Pooled-4 740.9n 748.7n ~ 0.100
_PooledVsNonPooled/NonPooled-4 8.717µ 8.953µ ~ 0.700
_MemoryFootprint/Current_512KB_32concurrent-4 6.752µ 7.225µ ~ 0.100
_MemoryFootprint/Proposed_32KB_32concurrent-4 9.909µ 12.767µ ~ 0.100
_MemoryFootprint/Alternative_64KB_32concurrent-4 9.659µ 10.556µ ~ 0.100
_prepareTxsPerLevel-4 411.2m 408.7m ~ 1.000
_prepareTxsPerLevelOrdered-4 3.717m 3.472m ~ 0.100
_prepareTxsPerLevel_Comparison/Original-4 407.9m 409.8m ~ 0.700
_prepareTxsPerLevel_Comparison/Optimized-4 3.437m 3.490m ~ 0.700
SubtreeSizes/10k_tx_4_per_subtree-4 1.267m 1.274m ~ 1.000
SubtreeSizes/10k_tx_16_per_subtree-4 303.5µ 303.5µ ~ 1.000
SubtreeSizes/10k_tx_64_per_subtree-4 72.25µ 72.96µ ~ 1.000
SubtreeSizes/10k_tx_256_per_subtree-4 18.14µ 17.93µ ~ 0.700
SubtreeSizes/10k_tx_512_per_subtree-4 8.950µ 8.958µ ~ 1.000
SubtreeSizes/10k_tx_1024_per_subtree-4 4.405µ 4.405µ ~ 0.800
SubtreeSizes/10k_tx_2k_per_subtree-4 2.182µ 2.218µ ~ 1.000
BlockSizeScaling/10k_tx_64_per_subtree-4 69.98µ 71.35µ ~ 0.400
BlockSizeScaling/10k_tx_256_per_subtree-4 17.61µ 17.88µ ~ 0.100
BlockSizeScaling/10k_tx_1024_per_subtree-4 4.413µ 4.394µ ~ 1.000
BlockSizeScaling/50k_tx_64_per_subtree-4 369.2µ 371.3µ ~ 0.400
BlockSizeScaling/50k_tx_256_per_subtree-4 87.33µ 87.53µ ~ 0.400
BlockSizeScaling/50k_tx_1024_per_subtree-4 22.03µ 21.71µ ~ 0.400
SubtreeAllocations/small_subtrees_exists_check-4 149.5µ 151.1µ ~ 0.700
SubtreeAllocations/small_subtrees_data_fetch-4 158.6µ 158.0µ ~ 0.400
SubtreeAllocations/small_subtrees_full_validation-4 311.1µ 314.8µ ~ 0.400
SubtreeAllocations/medium_subtrees_exists_check-4 8.881µ 8.828µ ~ 1.000
SubtreeAllocations/medium_subtrees_data_fetch-4 9.306µ 9.344µ ~ 0.400
SubtreeAllocations/medium_subtrees_full_validation-4 17.64µ 18.21µ ~ 0.100
SubtreeAllocations/large_subtrees_exists_check-4 2.113µ 2.089µ ~ 0.200
SubtreeAllocations/large_subtrees_data_fetch-4 2.257µ 2.222µ ~ 0.100
SubtreeAllocations/large_subtrees_full_validation-4 4.357µ 4.532µ ~ 0.100
StoreBlock_Sequential/BelowCSVHeight-4 333.7µ 335.5µ ~ 0.400
StoreBlock_Sequential/AboveCSVHeight-4 335.7µ 342.1µ ~ 0.100
GetUtxoHashes-4 255.4n 256.7n ~ 1.000
GetUtxoHashes_ManyOutputs-4 42.22µ 44.19µ ~ 0.100
_NewMetaDataFromBytes-4 227.6n 230.0n ~ 0.200
_Bytes-4 393.5n 394.4n ~ 0.700
_MetaBytes-4 135.9n 135.8n ~ 1.000

Threshold: >10% with p < 0.05 | Generated: 2026-06-02 06:49 UTC

ordishs
ordishs approved these changes Jun 2, 2026
View reviewed changes

@ordishs ordishs left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Approve. Clean, test-only change replacing sleep-then-assert anti-patterns with poll-until-condition.

Verified:

  • go vet clean on both changed packages; no unused time imports introduced.
  • WaitForPruner is a genuine completion barrier here — both pruner tests set EnablePruner: true, which registers the observer, so the nil-observer no-op path can't be silently hit.
  • require.Eventually conversions are sound (captured-error checked in predicate, value preserved for later assertions).

Good judgment on what to leave alone (real ban-TTL / timestamp-pacing sleeps) and honest scoping of skipped/wip tests.

Minor (non-blocking) follow-ups:

  • block_persister lost the per-block height/hash diagnostic logging on failure — consider restoring it so a timeout shows which blocks lagged.
  • Port the :204 'waits for a prune cycle, not provably the child1-DAH cycle' caveat into a code comment, not just the PR description.

@sonarqubecloud

sonarqubecloud Bot commented Jun 2, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@oskarszoon oskarszoon merged commit dd63020 into bsv-blockchain:main Jun 2, 2026
48 of 50 checks passed
@oskarszoon oskarszoon mentioned this pull request Jun 2, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ordishs ordishs ordishs approved these changes

@sugh01 sugh01 sugh01 approved these changes

@freemans13 freemans13 Awaiting requested review from freemans13

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Speed up gated e2e tests: replace hardcoded time.Sleep with condition-polling

3 participants

@oskarszoon @ordishs @sugh01