Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

assert: fix flaky TestNeverTrue #1417

Merged
merged 1 commit into from
Jul 31, 2023
Merged

Conversation

dolmen
Copy link
Collaborator

@dolmen dolmen commented Jul 11, 2023

Summary

Fix flaky TestNeverTrue in assert package: use a channel to make a synchronized counter to avoid a race condition.

Changes

Refactor TestNeverTrue to synchronize the counter.

Motivation

Fix flaky test which make CI unreliable.

$ go test -race -failfast -count=100 ./...

Related issues

@dolmen dolmen added pkg-assert Change related to package testify/assert concurrency labels Jul 11, 2023
@dolmen
Copy link
Collaborator Author

dolmen commented Jul 11, 2023

Cc: @SuperQ @brackendawson for review

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Way more clear/correct.

assert/assertions_test.go Outdated Show resolved Hide resolved
Fix flaky TestNeverTrue: use a channel to make a synchronized list of
return values to avoid a race condition.
@dolmen dolmen force-pushed the assert-fix-flaky-TestNeverTrue branch from 55b9534 to cd26798 Compare July 22, 2023 07:03
@dolmen
Copy link
Collaborator Author

dolmen commented Jul 22, 2023

@brackendawson Fixed and rebased. Ready for merge.

@dolmen dolmen merged commit 81667ad into master Jul 31, 2023
@dolmen dolmen deleted the assert-fix-flaky-TestNeverTrue branch July 31, 2023 17:07
@dolmen dolmen added the assert.Eventually About assert.Eventually/EventuallyWithT label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert.Eventually About assert.Eventually/EventuallyWithT concurrency pkg-assert Change related to package testify/assert
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants