ISSUE-696: Explore use of t.TempDir()

[Evalle]

PR Summary:

• Fixes Explore use of t.TempDir() #696

For the reviewers:
I'm not sure this is the right implementation, as @ahmetb asked to deprecate the implementation of testutil.NewTempDir(t) should I add some deprecation notice? Thanks in advance for the advices/reviews.

[k8s-ci-robot]

Welcome @Evalle!

It looks like this is your first PR to kubernetes-sigs/krew 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/krew has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[ahmetb]

Ideally we'd get rid of this type.

[Evalle]

@ahmetb thanks for the feedback! Just to be clear, you would like to get rid of NewTempDir completely or of type Tempdir I'm pretty new in golang so my question could sound strange. Thanks!

[corneliusweig]

@Evalle Can you have a look how the methods on the TempDir type are used? If there are just a few usages, it would be nice to get rid of the tempdir.go file and move the methods as free functions elsewhere.

If there are a lot of usages, your current PR looks good to me.

[Evalle]

@corneliusweig thanks for the comment. My first intention was to get rid of NewTempDir completely in favor of the native t.TempDir() but I can see that there are a lot of methods that use TempDir:

• https://github.com/kubernetes-sigs/krew/blob/master/internal/testutil/tempdir.go#L53
• https://github.com/kubernetes-sigs/krew/blob/master/internal/testutil/tempdir.go#L59
• https://github.com/kubernetes-sigs/krew/blob/master/internal/testutil/tempdir.go#L67
• https://github.com/kubernetes-sigs/krew/blob/master/internal/testutil/tempdir.go#L79
• https://github.com/kubernetes-sigs/krew/blob/master/internal/testutil/tempdir.go#L88

And the above functionality used by many tests, for example:

• https://github.com/kubernetes-sigs/krew/blob/master/cmd/krew/cmd/internal/setup_check_test.go#L29
• https://github.com/kubernetes-sigs/krew/blob/master/cmd/krew/cmd/internal/setup_check_test.go#L31

[ahmetb]

Yeah I was expecting maybe we have simpler unit test usages that just do NewTestUtil() in a single test function. For those, t.TempDir() can be better. But right now, there's no reason to split the usage into two. We can just accept this.
/lgtm
/approve

Any developer/build system will need to update to go1.16 go1.15.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahmetb, Evalle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [ahmetb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Evalle]

Thanks @ahmetb and @corneliusweig for the reviews