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

Deprecate the test teardown function #1599

Merged
merged 2 commits into from Dec 12, 2022

Conversation

armsnyder
Copy link
Contributor

@armsnyder armsnyder commented Dec 11, 2022

See https://pkg.go.dev/testing#T.Cleanup

Cleanup registers a function to be called when the test (or subtest) and all its subtests complete. Cleanup functions will be called in last added, first called order.

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, but I'm not going to merge something like this unless the PR "fixes" it for the whole package (to preserve consistency and maintainability). Not asking you to do that, just saying I'm not open to introduce a new pattern without cleaning up the old one...

@armsnyder
Copy link
Contributor Author

Reasonable. I didn't want to "fix" the whole project unless you were okay with the change first. I may do it then.

@svanharmelen
Copy link
Member

Check... Additionally I'm not sure if having one cleanup function that will teardown once is going to work for all tests, but I guess that is something we should, uuh test (no pun intended)...

@armsnyder
Copy link
Contributor Author

Updated. It was a very simple find-and-replace with no corner cases. Every time setup() was called was at the start of a test and immediately followed by defer teardown().

Copy link
Member

@svanharmelen svanharmelen left a comment

Choose a reason for hiding this comment

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

Ok... Then I'll take it :) Thanks!

@svanharmelen svanharmelen merged commit b56a655 into xanzy:master Dec 12, 2022
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.

None yet

2 participants