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

Merged
merged 1 commit into from
Jan 25, 2023
Merged

test: use t.TempDir to create temporary test directory #6533

merged 1 commit into from
Jan 25, 2023

Conversation

Juneezee
Copy link
Contributor

What type of PR is this:

/kind tests
/kind code-refactoring

What does this PR do / why we need it:

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.

This saves us at least 2 lines (error check, and cleanup) on every instance.

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

func TestFoo(t *testing.T) {
	// before
	tmpDir, err := ioutil.TempDir("", "")
	if e != nil {
		t.Fatal(e)
	}
	defer os.RemoveAll(tmpDir)

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

PR acceptance criteria:

  • Unit test

  • Integration test

  • Documentation

How to test changes / Special notes to the reviewer:

@Juneezee
test: use t.TempDir to create temporary test directory
e2dda16 
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>
@netlify
Copy link

netlify bot commented Jan 24, 2023 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview canceled.

Name Link
🔨 Latest commit e2dda16
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/63d003a576af8d0008262be7

@openshift-ci openshift-ci bot added kind/tests needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. Required by Prow. labels Jan 24, 2023
@openshift-ci
Copy link

openshift-ci bot commented Jan 24, 2023

Hi @Juneezee. Thanks for your PR.

I'm waiting for a redhat-developer 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.

@openshift-ci openshift-ci bot requested review from kadel and valaparthvi January 24, 2023 16:13
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023 •
edited
Loading

OpenShift Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023

NoCluster Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023

Unit Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023

Validate Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023

Kubernetes Tests on commit finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jan 24, 2023

Windows Tests (OCP) on commit finished with errors.
View logs: TXT HTML

@feloy
Copy link
Contributor

feloy commented Jan 25, 2023

/ok-to-test

@openshift-ci openshift-ci 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. Required by Prow. labels Jan 25, 2023
feloy
feloy approved these changes Jan 25, 2023
View reviewed changes
Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

Thanks @Juneezee for this contribution

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jan 25, 2023
@feloy
Copy link
Contributor

feloy commented Jan 25, 2023

/override windows-integration-test/Windows-test
Not related

@openshift-ci
Copy link

openshift-ci bot commented Jan 25, 2023

@feloy: Overrode contexts on behalf of feloy: windows-integration-test/Windows-test

In response to this:

/override windows-integration-test/Windows-test
Not related

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.

@openshift-merge-robot openshift-merge-robot merged commit 0f1d226 into redhat-developer:main Jan 25, 2023
@rm3l rm3l added this to the v3.7.0 🚀 milestone Jan 25, 2023
@rm3l rm3l added the area/refactoring Issues or PRs related to code refactoring label Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@feloy feloy feloy approved these changes

@kadel kadel Awaiting requested review from kadel

@valaparthvi valaparthvi Awaiting requested review from valaparthvi

Assignees

@feloy feloy

Labels
area/refactoring Issues or PRs related to code refactoring lgtm Indicates that a PR is ready to be merged. Required by Prow. ok-to-test Indicates a non-member PR verified by an org member that is safe to test.
Projects
None yet
Milestone
v3.7.0 🚀
Development

Successfully merging this pull request may close these issues.
4 participants
@Juneezee @feloy @rm3l @openshift-merge-robot