Skip to content

Reduce flakiness in unicast authorization tests#8603

Open
janezpodhostnik wants to merge 2 commits into
masterfrom
janez/fix-flaky-unicast-authorization-stream-reset
Open

Reduce flakiness in unicast authorization tests#8603
janezpodhostnik wants to merge 2 commits into
masterfrom
janez/fix-flaky-unicast-authorization-stream-reset

Conversation

@janezpodhostnik

@janezpodhostnik janezpodhostnik commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Problem

The rejection-path tests in TestUnicastAuthorizationTestSuite intermittently fail with a benign stream reset error on the sender's Unicast call. When the receiver rejects an unauthorized message it resets the inbound stream, which races with the sender closing its side. The error is documented as benign, but the tests asserted require.NoError.

Example failing CI run: https://github.com/onflow/flow-go/actions/runs/28946085223/job/85880463137

Change

Replace require.NoError with a helper (requireStreamAcceptedOrReset) in the rejection-path tests, which tolerates a stream reset error while still failing on any other error. Happy-path tests are unchanged. The authoritative assertion (violation reported via the slashing violations consumer) is preserved.

Summary by CodeRabbit

  • Bug Fixes
    • Improved handling of occasional stream-reset responses during unicast authorization, making these cases behave consistently instead of being treated as failures.
    • Updated authorization coverage to reflect this more resilient behavior in scenarios where receiver-side resets can happen.

@janezpodhostnik
janezpodhostnik requested a review from a team as a code owner July 9, 2026 16:56
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

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

Scanned Files

None

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Added a requireStreamAcceptedOrReset helper method to UnicastAuthorizationTestSuite that accepts either a nil error or an error containing "stream reset". Updated nine call sites across unicast authorization tests to use this helper instead of strict require.NoError assertions or an inline conditional check.

Changes

Unicast authorization test error handling

Layer / File(s) Summary
Add stream reset tolerance helper
network/test/cohort2/unicast_authorization_test.go
Introduces requireStreamAcceptedOrReset(err error), which passes when err is nil or its message contains "stream reset".
Apply helper across test cases
network/test/cohort2/unicast_authorization_test.go
Replaces require.NoError(u.T(), err) assertions and one inline "stream reset" conditional across eight test functions with calls to u.requireStreamAcceptedOrReset(err).

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • onflow/flow-go#8409: Both PRs modify network/test/cohort2/unicast_authorization_test.go to change how "stream reset" race errors are asserted in unicast authorization tests.

Suggested reviewers: fxamacker, holyfuchs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: making unicast authorization tests less flaky by tolerating benign stream reset errors.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch janez/fix-flaky-unicast-authorization-stream-reset

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

Comment @coderabbitai help to get the list of available commands.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@janezpodhostnik janezpodhostnik self-assigned this Jul 9, 2026
@janezpodhostnik
janezpodhostnik requested a review from a team July 9, 2026 17:03
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