Skip to content

fix: Remove use of require in require.Eventually in tests #3110

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

Merged
merged 4 commits into from
Jul 22, 2022

Conversation

mafredri
Copy link
Member

Because require uses t.FailNow() and require.Eventually runs the
function in a goroutine, which is not allowed.

Shout out to @johnstcn for showing us the way!

@mafredri mafredri self-assigned this Jul 22, 2022
@mafredri mafredri requested review from johnstcn and a team July 22, 2022 10:43
@Emyrk
Copy link
Member

Emyrk commented Jul 22, 2022

cough cough linter rule cough

I joke, but this could be a linter rule

@mafredri
Copy link
Member Author

Good suggestion @Emyrk, I'll take a shot at writing one.

@mafredri
Copy link
Member Author

@Emyrk added, and it caught one more instance! 😄

Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

🚀

@mafredri mafredri enabled auto-merge (squash) July 22, 2022 16:21
@mafredri mafredri disabled auto-merge July 22, 2022 16:36
mafredri and others added 4 commits July 22, 2022 19:37
Because require uses `t.FailNow()` and `require.Eventually` runs the
function in a goroutine, which is not allowed.
Co-authored-by: Cian Johnston <cian@coder.com>
@mafredri mafredri force-pushed the mafredri/fix-use-of-require-in-require-eventually branch from 7b7124d to f6167b6 Compare July 22, 2022 16:38
@mafredri mafredri merged commit 51dd1fd into main Jul 22, 2022
@mafredri mafredri deleted the mafredri/fix-use-of-require-in-require-eventually branch July 22, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants