Skip to content

Conversation

@Juneezee
Copy link
Contributor

Which problem is this PR solving?

Short description of the changes

This PR replaces os.Setenv with t.Setenv. Starting from Go 1.17, we can use t.Setenv to set environment variable in test. The environment variable is automatically restored to its original value when the test and all its subtests complete. This ensures that each test does not start with leftover environment variables from previous completed tests.

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

func TestFoo(t *testing.T) {
	// before
	os.Setenv(key, "new value")
	defer os.Unsetenv(key)
	
	// after
	t.Setenv(key, "new value")
}

This commit replaces `os.Setenv` with `t.Setenv` in tests. The
environment variable is automatically restored to its original value
when the test and all its subtests complete.

Reference: https://pkg.go.dev/testing#T.Setenv
Signed-off-by: Eng Zer Jun <[email protected]>
@Juneezee Juneezee requested a review from a team as a code owner January 19, 2023 15:11
@Juneezee Juneezee requested a review from yurishkuro January 19, 2023 15:11
@codecov
Copy link

codecov bot commented Jan 19, 2023

Codecov Report

Base: 97.10% // Head: 97.10% // No change to project coverage 👍

Coverage data is based on head (e17c166) compared to base (0237ca4).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4169   +/-   ##
=======================================
  Coverage   97.10%   97.10%           
=======================================
  Files         296      296           
  Lines       17501    17501           
=======================================
  Hits        16995    16995           
  Misses        407      407           
  Partials       99       99           
Flag Coverage Δ
unittests 97.10% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@yurishkuro yurishkuro 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! Great to see these clean-up PRs.

@yurishkuro yurishkuro merged commit c7df877 into jaegertracing:main Jan 19, 2023
shubbham12 pushed a commit to shubbham12/jaeger that referenced this pull request Feb 22, 2023
## Which problem is this PR solving?

## Short description of the changes

This PR replaces `os.Setenv` with `t.Setenv`. Starting from Go 1.17, we
can use `t.Setenv` to set environment variable in test. The environment
variable is automatically restored to its original value when the test
and all its subtests complete. This ensures that each test does not
start with leftover environment variables from previous completed tests.

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

```go
func TestFoo(t *testing.T) {
	// before
	os.Setenv(key, "new value")
	defer os.Unsetenv(key)
	
	// after
	t.Setenv(key, "new value")
}
```

Signed-off-by: Eng Zer Jun <[email protected]>
shubbham12 pushed a commit to shubbham12/jaeger that referenced this pull request Mar 5, 2023
## Which problem is this PR solving?

## Short description of the changes

This PR replaces `os.Setenv` with `t.Setenv`. Starting from Go 1.17, we
can use `t.Setenv` to set environment variable in test. The environment
variable is automatically restored to its original value when the test
and all its subtests complete. This ensures that each test does not
start with leftover environment variables from previous completed tests.

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

```go
func TestFoo(t *testing.T) {
	// before
	os.Setenv(key, "new value")
	defer os.Unsetenv(key)

	// after
	t.Setenv(key, "new value")
}
```

Signed-off-by: Eng Zer Jun <[email protected]>
Signed-off-by: shubbham1215 <[email protected]>
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.

2 participants