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 #3159

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

Juneezee
Copy link
Contributor

Summary

A testing cleanup. 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.

This PR also refactor several cleanup logic by using t.Cleanup instead of defer.

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

Implementation details

  • Replaces ioutil.TempDir with t.TempDir
  • Setup function such as newTestDataClient(t *testing.T) no longer returns a cleanup func(). It now uses t.Cleanup to handle the cleanup when the test finishes.

Testing

make test and make run-integ-tests on Linux machine.

New tests cover the changes: yes

Description for the changelog

Enhancement - Use T.TempDir to create temporary test directory

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@singholt
Copy link
Contributor

singholt commented May 3, 2022

Hi @Juneezee, thanks a lot for submitting this change. I see that the target branch is "master", could you please open this PR against "dev"?

@Juneezee Juneezee changed the base branch from master to dev May 3, 2022 17:14
@Juneezee
Copy link
Contributor Author

Juneezee commented May 3, 2022

Hi @Juneezee, thanks a lot for submitting this change. I see that the target branch is "master", could you please open this PR against "dev"?

@singholt Oops sorry. I have changed it to dev.

@fierlion
Copy link
Member

Hi @Juneezee -- again sorry to have taken so long to get around to this. I see that the windows integration tests failed in the last test run. Unfortunately we'll need you to push an update to rebase against the latest dev to retrigger the tests. Would you be able to do this. If the tests continue to fail, I can share the failure logs with you and help in the debugging effort.

@Juneezee
Copy link
Contributor Author

Hi @fierlion, thanks for looking into this PR. I've rebased onto dev.

@Juneezee
Copy link
Contributor Author

Rebased onto dev and resolved conflicts.

This commit replaces `os.MkdirTemp` 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 `os.MkdirTemp`
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 <engzerjun@gmail.com>
@sparrc
Copy link
Contributor

sparrc commented Feb 11, 2023

closed by #3560, thank you for the contribution @Juneezee !!

@sparrc sparrc closed this Feb 11, 2023
@YashdalfTheGray YashdalfTheGray mentioned this pull request Feb 25, 2023
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.

6 participants