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

How to handle failed expectations inside of a goroutine? #772

Open
trivigy opened this issue May 24, 2019 · 6 comments
Open

How to handle failed expectations inside of a goroutine? #772

trivigy opened this issue May 24, 2019 · 6 comments

Comments

@trivigy
Copy link

trivigy commented May 24, 2019

I found some conversations about this topic but could not find any workarounds for this issue. If anyone knows I would greatly appreciate the help. Basically I simply want to fail an entire test from within a goroutine running inside of the test. Is there a way to do that and if so how do I do that?

@xordspar0
Copy link
Contributor

Maybe there should be documentation or a guide on using testify with goroutines in general.

@sneko
Copy link

sneko commented Oct 14, 2021

Hi,

@ernesto-jimenez @boyan-soubachov as major contributors to the library, do you have any advice or good reading to follow? It's a bit dark what should be done?

Also @trivigy @xordspar0 in the meantime, did you find the way to go?

Thanks,

EDIT: an old post has a "solution" but it seems a bit... chatty just for failing a test from a goroutine :/ ipfs/kubo#2043 (comment)

@brackendawson
Copy link
Collaborator

Using testify assert and require with goroutines is the same as using t.Errorf() and t.FailNow() respectively. assert doesn't document this but require sort of does. t.FailNow() must be called from the goroutine running the test, I think this is the issue the OP is mentioning? t.Errorf() does not have the same restriction.

The Go 1.16 release notes also speak about this because calling t.FailNow() in a goroutine became a vet warning (use of the require package does not trigger this warning), they give the most simple example of how to solve it; just use t.Error(). Translating this to testify we would just say "use assert in goroutines, not require".

That method is good enough™️ because either the test will fail, or if the assertion fails after the test has "finshed" then the assertion will cause a panic. But that's not great, the next fix is to use proper synchronisation and the issue that @sneko linked to has reasonable examples of this.

I think this is more of a Go issue than a testify one, but that's not to say testify shouldn't give helpful advice and examples. I'd consider doing:

  • Document in the assert package that failing assertions will call t.Errorf().
  • Document in the testify (module root) package that require is not appropriate in a goroutine but assert is.
  • Expand one of the assertion examples in the testify package to show an assertion in a goroutine, one of these two maybe:
func TestProbablyPanics(t *testing.T) {
	go func() {
		// Use assert in goroutines, not require.
		assert.Equal(t, 1, 2)
	}()
}

func TestFails(t *testing.T) {
	syn := make(chan struct{})
	// Allow goroutines to finish before the end of the test so their assertions
	// are reported without a panic
	defer func() { <-syn }()
	go func() {
		defer close(syn)
		assert.Equal(t, 1, 2)
	}()
}

What do you think?

@Antonboom
Copy link

Covered by https://github.com/Antonboom/testifylint#go-require.

@Antonboom
Copy link

Related to #1499

@ccoVeille
Copy link
Contributor

here is an interesting PR that rewrote the code using go routines in the tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants