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

chore: use t.TempDir to create temporary test directory #1856

Closed
wants to merge 2 commits into from
Closed

chore: use t.TempDir to create temporary test directory #1856

wants to merge 2 commits into from

Conversation

Juneezee
Copy link

This pull request replaces os.MkdirTemp with t.TempDir in tests. 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
	dir, err := os.MkdirTemp("", "")
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(dir)

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

@Juneezee Juneezee requested a review from a team as a code owner June 24, 2023 13:37
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`. 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>
Copy link
Collaborator

@hessjcg hessjcg left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution. This Looks good to me. Nice simplification of the test cases.

@enocom enocom added the tests: run Label to trigger Github Action tests. label Jun 29, 2023
@github-actions github-actions bot removed the tests: run Label to trigger Github Action tests. label Jun 29, 2023
@enocom enocom self-requested a review June 29, 2023 17:37
@enocom enocom added the tests: run Label to trigger Github Action tests. label Jun 29, 2023
@github-actions github-actions bot removed the tests: run Label to trigger Github Action tests. label Jun 29, 2023
@enocom enocom changed the title test: use t.TempDir to create temporary test directory chore: use t.TempDir to create temporary test directory Jun 29, 2023
@enocom enocom self-requested a review June 30, 2023 01:27
Copy link
Member

@enocom enocom left a comment

Choose a reason for hiding this comment

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

All the integration tests for unix sockets are failing because the temp dir is too long.

Looks like t.TempDir always prepends the test name to the directory name which probably makes it infeasible for use here. See https://docs.studygolang.com/src/testing/testing.go?s=31998:32031#L962.

I'm not sure we can accept this, but if you have other ideas I'd be curious to hear them.

@Juneezee Juneezee closed this Jun 30, 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.

None yet

3 participants