docs: clarify EventuallyWithT goroutine and FailNow behavior#1919
Open
jfjrh2014 wants to merge 1 commit into
Open
docs: clarify EventuallyWithT goroutine and FailNow behavior#1919jfjrh2014 wants to merge 1 commit into
jfjrh2014 wants to merge 1 commit into
Conversation
The EventuallyWithT condition function runs in its own goroutine per tick, but this was not documented. Users naturally expect they can call require assertions on the outer testing.T, but testing.T.FailNow must be called from the goroutine that created the test. Document this constraint and clarify what CollectT.FailNow actually terminates (the current tick, not the test). Also improves FailNow doc to cross-reference EventuallyWithT. Addresses stretchr#1396
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
EventuallyWithTcondition function runs in its own goroutine for each tick, but this was not documented. This PR clarifies what that means for users.Problem
As discussed in #1396, users naturally expect they can call
requireassertions on the outer*testing.Tfrom within the condition function. This is unsafe becausetesting.T.FailNowmust be called from the goroutine that created the test (t.Run).Additionally, the behavior of
CollectT.FailNow(that it terminates only the current tick and not the test) was not documented onEventuallyWithTitself.Changes
EventuallyWithT — Added two paragraphs documenting that:
FailNowon the outer*testing.Tis unsupported; the providedCollectTshould be used instead.CollectT.FailNowterminates only the current tick's condition function, notEventuallyWithTretries or test execution.CollectT.FailNow — Expanded the one-line doc to explain it marks
CollectTas failed and cross-referencesEventuallyWithT.The docs are written in prose first (no inline code snippets) per @brackendawson's feedback, and use Godoc link syntax (
[CollectT.FailNow],[testing.T.FailNow]) so they render correctly in both theassertand generatedrequirepackages.No behavioral changes. All existing tests pass.
Closes #1396