`Eventually` hangs on failed/aborted `condition`

[ubunatic]

Description

Calling runtime.Goexit(), as called by t.FailNow(), as called by require functions, or panicking inside Eventually will prevent the condition to exit with a result. The channel assignment ch <- condition() in the assert.Eventually will block and hang the test until the timeout is reached.

There is currently no way to wait for these kinds of unclean exits.

Step To Reproduce

See TestFailInsideEventually test.

Note

The issue cannot be reproduced using MockT and only happens with real testing.T.
I am currently working on a non-testing.T test to have proper tests later.

Expected behavior

If the condition fails to complete and thus not return a result, the Eventually assertion should stop waiting and forward whatever result the condition produced.

1. Condition exited because user called t.FailNow() or runtime.Goexit() manually \
   -> unclean exit -> mark test as failed -> stop
2. Condition exited with a panic
   -> unclean exit -> mark test as failed -> stop & panic

Note

Update: added expected panic vs. runtime.Goexit() behavior

Actual behavior

If the condition fails to complete and thus not return a result, the Eventually assertion will block until the timeout.

Proposed Fix

See #1809 which aims to fix all issues from unclean exits (excl. panics)

AI Assessment

I asked the AI:

Sometimes, my Eventually condition hangs and the ticks stop even though I do not have any delays in my condition func. It stops after the first tick and only fails after the timeout occurs.

It found the issue exactly in ch <- condition(), explaining the runtime.Goexit issue, and proposed to use only assert in the condition.

I followed up with:

Are there other places in the code that have such blocking behavior.

And it found the same issue in Never.

Next Question:

In Eventually the test at least fails. What are the implications for Never?

And it found:

... the select then just waits for the timeout, and when it finally arrives the assertion reports success. So you get a false positive: the test claims the condition stayed false for the whole duration, even though we only ever evaluated it once and then stopped checking altogether. The convenience wrapper in require.go simply delegates to assert.Never, so it inherits the same blind spot.