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

fix race condition in a integration test #234

Closed
wants to merge 1 commit into from
Closed

fix race condition in a integration test #234

wants to merge 1 commit into from

Conversation

nhooyr
Copy link
Contributor

@nhooyr nhooyr commented Jan 22, 2018

FatalF must not be called from outside the testing goroutine or this leads to a race condition within the testing package.

See https://golang.org/pkg/testing/#T.FailNow

@nhooyr nhooyr self-assigned this Jan 22, 2018
@nhooyr
Copy link
Contributor Author

nhooyr commented Jan 22, 2018

This fixes the CI failure in #233

@nhooyr nhooyr changed the title fix race condition in test fix race condition in integration test Jan 22, 2018
@nhooyr nhooyr changed the title fix race condition in integration test fix race condition in a integration test Jan 22, 2018
@nhooyr
Copy link
Contributor Author

nhooyr commented Jan 24, 2018

Something is wrong with CI hmm.

@@ -444,7 +444,7 @@ func TestFsnotifyDeleteWatchedDir(t *testing.T) {
// Receive errors on the error channel on a separate goroutine
go func() {
for err := range watcher.Errors {
t.Fatalf("error received: %s", err)
t.Error("error received: %s", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be t.Errorf("error received: %s",err)

gdey added a commit that referenced this pull request Aug 30, 2018
The documentation for t.FailNow (which is called by t.Fatalf) states
that it should not be called in go routines outside of the test go
routine.
@gdey gdey closed this Aug 30, 2018
@nathany
Copy link
Contributor

nathany commented Aug 30, 2018

@nhooyr thanks for bringing this to our attention. #266 provides the fix.

nathany pushed a commit that referenced this pull request Aug 30, 2018
The documentation for t.FailNow (which is called by t.Fatalf) states
that it should not be called in go routines outside of the test go
routine.
radu-munteanu pushed a commit to radu-munteanu/fsnotify that referenced this pull request Feb 25, 2019
The documentation for t.FailNow (which is called by t.Fatalf) states
that it should not be called in go routines outside of the test go
routine.
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.

None yet

3 participants