Fix flaky epoch test

[janezpodhostnik]

It should wait for the Throw to happen before trying to assert the epoch events len.

Summary by CodeRabbit

• Tests
  • Enhanced test reliability by improving synchronization timing in protocol event validation tests, ensuring assertions execute in the correct sequence.

[github-actions]

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

[coderabbitai]

📝 Walkthrough

Walkthrough

Test synchronization improvements were added to epoch_lookup_test.go to ensure proper callback ordering. New throwCalled channels coordinate the timing of assertions with the invocation of the Throw callback, preventing race conditions in asynchronous test operations.

Changes

Cohort / File(s)	Summary
Test Synchronization module/epochs/epoch_lookup_test.go	Added throwCalled channel synchronization to TestProtocolEvents_EpochExtended_SanityChecks test to block assertion checks until the Throw callback is invoked, ensuring proper ordering in async test flows.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A hop, a signal, a moment to wait,
Before checking if tests align just right,
Channels aligned at the perfect gate,
Throw callbacks dance in the test-async light,
Synchronization blooms—no more race in sight! 🌸

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing a flaky test in the epoch-related code by adding synchronization logic.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch janez/flaky-test-fix

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

module/epochs/epoch_lookup_test.go (2)

225-236: ⚠️ Potential issue | 🟡 Minor

Same race condition exists here — apply the throwCalled fix consistently.

This subtest has the identical pattern: Throw carries assertions, yet the Eventually only gates on len(suite.lookup.epochEvents) == 0. The event can be dequeued from the channel before Throw is invoked, so the Eventually can succeed without the Throw assertions ever running — exactly the flake fixed in the third subtest.

Proposed fix

+		throwCalled := make(chan struct{})
 		ctx.On("Throw", mock.AnythingOfType("*errors.errorString")).Run(func(args mock.Arguments) {
 			err, ok := args.Get(0).(error)
 			assert.True(suite.T(), ok)
 			assert.Contains(suite.T(), err.Error(), fmt.Sprintf(invalidExtensionFinalView, suite.currEpoch.finalView, extension.FinalView))
+			close(throwCalled)
 		})

 		suite.lookup.EpochExtended(suite.currEpoch.epochCounter, nil, extension)

 		// wait for the protocol event to be processed (async)
 		assert.Eventually(suite.T(), func() bool {
-			return len(suite.lookup.epochEvents) == 0
+			select {
+			case <-throwCalled:
+				return len(suite.lookup.epochEvents) == 0
+			default:
+				return false
+			}
 		}, 2*time.Second, 50*time.Millisecond)

---

244-258: ⚠️ Potential issue | 🟡 Minor

Same race condition in the second subtest — needs the same synchronization.

Identical to the first subtest: Throw mock contains assertions but Eventually only checks the queue length, so the test can pass without Throw ever firing.

Proposed fix

+		throwCalled := make(chan struct{})
 		ctx.On("Throw", mock.AnythingOfType("*errors.errorString")).Run(func(args mock.Arguments) {
 			err, ok := args.Get(0).(error)
 			assert.True(suite.T(), ok)
 			assert.Contains(suite.T(), err.Error(), fmt.Sprintf(mismatchEpochCounter, suite.currEpoch.epochCounter, unknownCounter))
+			close(throwCalled)
 		})

 		suite.lookup.EpochExtended(unknownCounter, nil, flow.EpochExtension{
 			FirstView: suite.currEpoch.finalView + 1,
 			FinalView: suite.currEpoch.finalView + 100,
 		})

 		// wait for the protocol event to be processed (async)
 		assert.Eventually(suite.T(), func() bool {
-			return len(suite.lookup.epochEvents) == 0
+			select {
+			case <-throwCalled:
+				return len(suite.lookup.epochEvents) == 0
+			default:
+				return false
+			}
 		}, 2*time.Second, 50*time.Millisecond)

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!