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

test: use T.TempDir to create temporary test directory #1660

Merged
merged 2 commits into from
Apr 21, 2022
Merged

test: use T.TempDir to create temporary test directory #1660

merged 2 commits into from
Apr 21, 2022

Conversation

Juneezee
Copy link
Contributor

Description

A testing cleanup.

This pull request replaces ioutil.TempDir with t.TempDir. We can use the T.TempDir function from the testing package to create temporary directory. The directory created by T.TempDir is automatically removed when the test and all its subtests complete.

Reference: https://pkg.go.dev/testing#T.TempDir

func TestFoo(t *testing.T) {
	// before
	tmpDir, err := ioutil.TempDir("", "")
	require.NoError(t, err)
	defer require.NoError(os.RemoveAll(tmpDir))

	// now
	tmpDir := t.TempDir()
}

Changes

  • Replace ioutil.TempDir with t.TempDir
  • Remove redundant os.RemoveAll(tmpDir) since the temporary directory will be removed automatically once the test complete.

/lint

This commit replaces `ioutil.TempDir` with `t.TempDir` in tests. The
directory created by `t.TempDir` is automatically removed when the test
and all its subtests complete.

Prior to this commit, temporary directory created using `ioutil.TempDir`
needs to be removed manually by calling `os.RemoveAll`, which is omitted
in some tests. The error handling boilerplate e.g.
	defer func() {
		if err := os.RemoveAll(dir); err != nil {
			t.Fatal(err)
		}
	}
is also tedious, but `t.TempDir` handles this for us nicely.

Reference: https://pkg.go.dev/testing#T.TempDir
Signed-off-by: Eng Zer Jun <[email protected]>
@knative-prow knative-prow bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 19, 2022
Copy link

@knative-prow knative-prow bot left a comment

Choose a reason for hiding this comment

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

@Juneezee: 0 warnings.

In response to this:

Description

A testing cleanup.

This pull request replaces ioutil.TempDir with t.TempDir. We can use the T.TempDir function from the testing package to create temporary directory. The directory created by T.TempDir is automatically removed when the test and all its subtests complete.

Reference: https://pkg.go.dev/testing#T.TempDir

func TestFoo(t *testing.T) {
  // before
  tmpDir, err := ioutil.TempDir("", "")
  require.NoError(t, err)
  defer require.NoError(os.RemoveAll(tmpDir))

  // now
  tmpDir := t.TempDir()
}

Changes

  • Replace ioutil.TempDir with t.TempDir
  • Remove redundant os.RemoveAll(tmpDir) since the temporary directory will be removed automatically once the test complete.

/lint

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow
Copy link

knative-prow bot commented Apr 19, 2022

Hi @Juneezee. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow knative-prow bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 19, 2022
Signed-off-by: Eng Zer Jun <[email protected]>
@Juneezee Juneezee marked this pull request as ready for review April 19, 2022 16:10
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 19, 2022
@rhuss
Copy link
Contributor

rhuss commented Apr 19, 2022

/ok-to-test

@knative-prow knative-prow bot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 19, 2022
@rhuss
Copy link
Contributor

rhuss commented Apr 19, 2022

Thanks, looks good to me! Let's get that in right after we did the 1.4 release.

/approve
/lgtm
/hold

@knative-prow knative-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 19, 2022
@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Apr 19, 2022
@knative-prow
Copy link

knative-prow bot commented Apr 19, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Juneezee, rhuss

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:

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

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 19, 2022
@Juneezee
Copy link
Contributor Author

/retest

@codecov
Copy link

codecov bot commented Apr 21, 2022

Codecov Report

Merging #1660 (5bb63ec) into main (d8ab3d5) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1660   +/-   ##
=======================================
  Coverage   79.48%   79.48%           
=======================================
  Files         171      171           
  Lines       12963    12963           
=======================================
  Hits        10304    10304           
  Misses       1941     1941           
  Partials      718      718           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d8ab3d5...5bb63ec. Read the comment docs.

@rhuss
Copy link
Contributor

rhuss commented Apr 21, 2022

/unhold

@knative-prow knative-prow bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 21, 2022
@knative-prow knative-prow bot merged commit 736c7c2 into knative:main Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants