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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 1 commit into from
May 31, 2022
Merged

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

merged 1 commit into from
May 31, 2022

Conversation

Juneezee
Copy link
Contributor

@Juneezee Juneezee commented May 30, 2022 •
edited
Loading

What problem does this PR solve?

Issue Number: close #5659

What is changed and how it works?

This pull request replaces ioutil.TempDir and os.MkdirTemp 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.

This saves us at least 2 lines (error check, and cleanup) on every instance, or in some cases adds cleanup that we forgot.

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

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

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

Check List

Tests

  • Unit test
  • No code

Questions

Will it cause performance regression or break compatibility?

No

Do you need to update user documentation, design documentation or monitoring documentation?

No

Release note 

None

@Juneezee
test: use T.TempDir to create temporary test directory
e6af1e4 
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 <engzerjun@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented May 30, 2022 •
edited
Loading

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • lance6716
  • overvenus

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/needs-triage-completed labels May 30, 2022
@sre-bot
Copy link

sre-bot commented May 30, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot ti-chi-bot added contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. labels May 30, 2022
@ti-chi-bot
Copy link
Member

Welcome @Juneezee!

It looks like this is your first PR to pingcap/tiflow 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/tiflow. 😃

@ti-chi-bot ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 30, 2022
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 30, 2022
overvenus
overvenus approved these changes May 30, 2022
View reviewed changes
Copy link
Member

@overvenus overvenus left a comment

Choose a reason for hiding this comment

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

Thanks!

@ti-chi-bot ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 30, 2022
@lance6716
Copy link
Contributor

/check-issue-triage-complete

@lance6716
Copy link
Contributor

/merge

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: e6af1e4

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label May 31, 2022
@lance6716
Copy link
Contributor

/run-dm-integration-test

1 similar comment
@lance6716
Copy link
Contributor

/run-dm-integration-test

@ti-chi-bot ti-chi-bot merged commit bc4d01c into pingcap:master May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lance6716 lance6716 lance6716 approved these changes

@overvenus overvenus overvenus approved these changes

Assignees
No one assigned
Labels
contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

test: use T.TempDir to create temporary test directory
5 participants
@Juneezee @ti-chi-bot @sre-bot @lance6716 @overvenus