-
Notifications
You must be signed in to change notification settings - Fork 369
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
Cleanup: more consistent naming of inner *testing.T
#508
Comments
Nr.2 for sure :P |
Also, honestly it doesn't matter a whole lot. |
So you suggest to simply ignore that we are using different patterns? |
IMO this isn’t really a coding pattern. Simply a variable shadowing in different scopes. It currently seems harmless, and doesn’t lead to code duplication etc. If it bothers you a lot, feel free to write some code to enforce it. Maybe there’s a pattern we can check. |
Issues go stale after 90d of inactivity. If this issue is safe to close now please do so with Send feedback to sig-testing, kubernetes/test-infra and/or fejta. |
Go 1.14 has improved support for deferred cleanup routines in tests. This should also be handled, either here or elsewhere |
#606 suggests using go1.14 as well. It sounds like we just need to go do it. |
There are basically two patterns in krew:
Shadow the outer
t *testing.T
.Use an inner
tt *testing.T
Both variants have their pro's and con's:
Shadowing can be confusing because one might overlook that there is an inner variable that shadows the outer one. However, in the great majority of cases, only the inner
*testing.T
should be used for assertions, so this also prevents errors where a test uses the outer one. So either way is fine.Let's decide which pattern we want to follow and stick to it. This should also come with a source code test.
The text was updated successfully, but these errors were encountered: