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

Detect and fail on require.* methods called from a wrong goroutine #1499

Open
ash2k opened this issue Oct 31, 2023 · 5 comments
Open

Detect and fail on require.* methods called from a wrong goroutine #1499

ash2k opened this issue Oct 31, 2023 · 5 comments

Comments

@ash2k
Copy link

ash2k commented Oct 31, 2023

As already documented (via #1392), require.* functions/methods are only safe to use from the main test/benchmark goroutine and are unsafe otherwise. Go stdlib may fix this one way or another (golang/go#15758) but even if/when this happens, testify doesn't call those methods unless there is an assertion failure. This means the bug in the test can live in users code for a very long time.

I think ideally testify would fail the test (or panic) if a function/method from the require family is used incorrectly regardless of whether there is a failure in the test itself or not.

@dolmen
Copy link
Collaborator

dolmen commented Oct 31, 2023

No idea how this could be implemented (if ever that is a good idea in the first place).

@Antonboom
Copy link

@ash2k
Copy link
Author

ash2k commented Nov 3, 2023

@dolmen Perhaps the check could be implemented by the assertion looking at the goroutine's stacktrace? This might be fragile as stacktrace is not an API so there should be a test for this.

@Antonboom That's cool but static analysis likely cannot catch all cases.

@Antonboom
Copy link

@ash2k hi

but static analysis likely cannot catch all cases

I think these are very rare cases, because the linter works exactly the same way – it analyzes the call stack.
But it looks as poor workaround for the open source library.

At the moment the Go team is going the static analysis way (go vet's testinggoroutine check), and I'm not sure they'll check the stack rather than just remove the requirement for where these functions should be called.

Because when I was testing my linter, I noticed that many projects (especially the largest ones) use require in goroutines and your proposal will be breaking change for them and looks like candidate for v2 tag.

Anyway sorry for offtop 🙏

@dolmen
Copy link
Collaborator

dolmen commented Nov 23, 2023

@Antonboom Don't be sorry: this is totally on topic.

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

3 participants