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

add EventuallyWithT assertion #1264

Merged

Conversation

tobikris
Copy link
Contributor

@tobikris tobikris commented Sep 13, 2022

Summary

This adds a version of Eventually that allows to do assertions in the condition.

Changes

  • Added EventuallyWithT assertion.
  • Added CollectT helper struct.

Motivation

The current Eventually only returns one summary error in case of a failure. This is not very helpful if several conditions need to be matched in the condition function. This version collects all failures of a tick and replays the last tick's failures after the timeout.
Thus the last iteration's errors can be shown to the user.

Example Usage

externalValue := false
go func() {
	time.Sleep(8*time.Second)
	externalValue = true
}()
assert.EventuallyWithTf(t, func(c *assert.CollectT) {
	// add assertions as needed; any assertion failure will fail the current tick
	assert.True(c, externalValue, "expected 'externalValue' to be true")
}, 1*time.Second, 10*time.Second, "external state has not changed to 'true'; still: %v", externalState)

Related issues

Closes #902

arikkfir and others added 4 commits October 20, 2022 08:25
This change updates the "EventuallyWithT" assertion variants (regular, formatted,
requirement) to consider a condition as "met" if no assertion errors were raised
in a tick.

This allows to write easier conditions which simply contain assertions, without
needing to return a bool. The equivalent of a condition returning true in the
previous implementation would be a a condition with a single "assert.True(..)" call.
Change "EventuallyWithT" condition acceptance to no-errors raised
@mrIncompetent
Copy link

@boyan-soubachov if you have a minute 🙏

@l-abels
Copy link

l-abels commented Apr 14, 2023

I implemented something similar about a year ago, this looks like a good solution. What needs to happen to get this merged? I'm running up against this use case again and would love to have this available.

Copy link
Collaborator

@boyan-soubachov boyan-soubachov left a comment

Choose a reason for hiding this comment

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

Thank you

@boyan-soubachov boyan-soubachov merged commit 4b2f4d2 into stretchr:master May 9, 2023
@tobikris tobikris deleted the eventually-with-assertions branch May 23, 2023 18:28
@dolmen
Copy link
Collaborator

dolmen commented Jul 22, 2023

Methods Reset and Copy of CollectT should have been made private.

CollectT should not have been exposed directly (this is an implementation detail) but only an subset of the testing.TB interface in the signature of the condition argument of EventuallyWithT.

But all this is now part of the public API. :(

@dolmen
Copy link
Collaborator

dolmen commented Jul 22, 2023

I can't understand how adding such a feature has gone into a patch release of testify. Does this gives us (maintainers) the permission to also fix it in a patch release?

Usage in the wild on GitHub: https://github.com/search?q=content%3Aassert.EventuallyWithT+language%3Ago+NOT+user%3Astretchr+NOT+path%3Aassert%2F+NOT+path%3Arequire%2F&type=code

Cc: @brackendawson

@brackendawson
Copy link
Collaborator

Probably the minor version should have been bumped when this was released. Fixing it should be a patch unless we chnage the API. I think we should do the right things, perhaps this discussion deserves an issue to tackle release higyne? I'm also somewhat interested in some automated testing before releases using a lot of real modules that import testify, there have been some regressions in recent releases.

@dolmen dolmen added the assert.Eventually About assert.Eventually/EventuallyWithT label Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert.Eventually About assert.Eventually/EventuallyWithT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better alternative to Eventually()
7 participants