From 471cbea4fa61ab98d8f0b3ee226becce449d69f8 Mon Sep 17 00:00:00 2001 From: Eng Zer Jun Date: Mon, 26 Sep 2022 16:46:02 +0800 Subject: [PATCH] test: use `t.Setenv` to set env vars in tests (#6333) 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 Signed-off-by: Eng Zer Jun --- api/http_client_test.go | 10 ++------ git/git_test.go | 14 ++--------- internal/ghrepo/repo_test.go | 5 +--- pkg/cmd/auth/login/login_test.go | 7 +----- pkg/cmd/browse/browse_test.go | 11 +-------- pkg/cmd/factory/default_test.go | 13 ++--------- pkg/cmd/factory/remote_resolver_test.go | 6 ----- pkg/cmdutil/factory_test.go | 5 +--- pkg/iostreams/color_test.go | 31 +++++-------------------- 9 files changed, 16 insertions(+), 86 deletions(-) diff --git a/api/http_client_test.go b/api/http_client_test.go index ebd360af9..02955fcb2 100644 --- a/api/http_client_test.go +++ b/api/http_client_test.go @@ -165,18 +165,12 @@ func TestNewHTTPClient(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - oldDebug := os.Getenv("DEBUG") - oldGhDebug := os.Getenv("GH_DEBUG") - os.Setenv("DEBUG", tt.envDebug) + t.Setenv("DEBUG", tt.envDebug) if tt.setGhDebug { - os.Setenv("GH_DEBUG", tt.envGhDebug) + t.Setenv("GH_DEBUG", tt.envGhDebug) } else { os.Unsetenv("GH_DEBUG") } - t.Cleanup(func() { - os.Setenv("DEBUG", oldDebug) - os.Setenv("GH_DEBUG", oldGhDebug) - }) ios, _, _, stderr := iostreams.Test() client, err := NewHTTPClient(HTTPClientOptions{ diff --git a/git/git_test.go b/git/git_test.go index 4b7d5a74e..b5812af9e 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -1,24 +1,14 @@ package git import ( - "os" "reflect" "testing" "github.com/cli/cli/v2/internal/run" ) -func setGitDir(t *testing.T, dir string) { - // TODO: also set XDG_CONFIG_HOME, GIT_CONFIG_NOSYSTEM - old_GIT_DIR := os.Getenv("GIT_DIR") - os.Setenv("GIT_DIR", dir) - t.Cleanup(func() { - os.Setenv("GIT_DIR", old_GIT_DIR) - }) -} - func TestLastCommit(t *testing.T) { - setGitDir(t, "./fixtures/simple.git") + t.Setenv("GIT_DIR", "./fixtures/simple.git") c, err := LastCommit() if err != nil { t.Fatalf("LastCommit error: %v", err) @@ -32,7 +22,7 @@ func TestLastCommit(t *testing.T) { } func TestCommitBody(t *testing.T) { - setGitDir(t, "./fixtures/simple.git") + t.Setenv("GIT_DIR", "./fixtures/simple.git") body, err := CommitBody("6f1a2405cace1633d89a79c74c65f22fe78f9659") if err != nil { t.Fatalf("CommitBody error: %v", err) diff --git a/internal/ghrepo/repo_test.go b/internal/ghrepo/repo_test.go index b013dd42c..1530f8f24 100644 --- a/internal/ghrepo/repo_test.go +++ b/internal/ghrepo/repo_test.go @@ -4,7 +4,6 @@ import ( "errors" "fmt" "net/url" - "os" "testing" ) @@ -195,9 +194,7 @@ func TestFromFullName(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.hostOverride != "" { - old := os.Getenv("GH_HOST") - os.Setenv("GH_HOST", tt.hostOverride) - defer os.Setenv("GH_HOST", old) + t.Setenv("GH_HOST", tt.hostOverride) } r, err := FromFullName(tt.input) if tt.wantErr != nil { diff --git a/pkg/cmd/auth/login/login_test.go b/pkg/cmd/auth/login/login_test.go index c5b2d0803..d573ba76e 100644 --- a/pkg/cmd/auth/login/login_test.go +++ b/pkg/cmd/auth/login/login_test.go @@ -3,7 +3,6 @@ package login import ( "bytes" "net/http" - "os" "regexp" "runtime" "testing" @@ -27,11 +26,7 @@ func stubHomeDir(t *testing.T, dir string) { case "plan9": homeEnv = "home" } - oldHomeDir := os.Getenv(homeEnv) - os.Setenv(homeEnv, dir) - t.Cleanup(func() { - os.Setenv(homeEnv, oldHomeDir) - }) + t.Setenv(homeEnv, dir) } func Test_NewCmdLogin(t *testing.T) { diff --git a/pkg/cmd/browse/browse_test.go b/pkg/cmd/browse/browse_test.go index ba12f6bdf..91197cc96 100644 --- a/pkg/cmd/browse/browse_test.go +++ b/pkg/cmd/browse/browse_test.go @@ -149,15 +149,6 @@ func TestNewCmdBrowse(t *testing.T) { } } -func setGitDir(t *testing.T, dir string) { - // taken from git_test.go - old_GIT_DIR := os.Getenv("GIT_DIR") - os.Setenv("GIT_DIR", dir) - t.Cleanup(func() { - os.Setenv("GIT_DIR", old_GIT_DIR) - }) -} - type testGitClient struct{} func (gc *testGitClient) LastCommit() (*git.Commit, error) { @@ -166,7 +157,7 @@ func (gc *testGitClient) LastCommit() (*git.Commit, error) { func Test_runBrowse(t *testing.T) { s := string(os.PathSeparator) - setGitDir(t, "../../../git/fixtures/simple.git") + t.Setenv("GIT_DIR", "../../../git/fixtures/simple.git") tests := []struct { name string opts BrowseOptions diff --git a/pkg/cmd/factory/default_test.go b/pkg/cmd/factory/default_test.go index 86c93d1d0..feef3e81f 100644 --- a/pkg/cmd/factory/default_test.go +++ b/pkg/cmd/factory/default_test.go @@ -4,7 +4,6 @@ import ( "net/http" "net/http/httptest" "net/url" - "os" "testing" "github.com/cli/cli/v2/git" @@ -245,9 +244,7 @@ func Test_OverrideBaseRepo(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if tt.envOverride != "" { - old := os.Getenv("GH_REPO") - os.Setenv("GH_REPO", tt.envOverride) - defer os.Setenv("GH_REPO", old) + t.Setenv("GH_REPO", tt.envOverride) } f := New("1") rr := &remoteResolver{ @@ -324,13 +321,7 @@ func Test_ioStreams_pager(t *testing.T) { t.Run(tt.name, func(t *testing.T) { if tt.env != nil { for k, v := range tt.env { - old := os.Getenv(k) - os.Setenv(k, v) - if k == "GH_PAGER" { - defer os.Unsetenv(k) - } else { - defer os.Setenv(k, old) - } + t.Setenv(k, v) } } f := New("1") diff --git a/pkg/cmd/factory/remote_resolver_test.go b/pkg/cmd/factory/remote_resolver_test.go index 07ae5863d..bc20686a9 100644 --- a/pkg/cmd/factory/remote_resolver_test.go +++ b/pkg/cmd/factory/remote_resolver_test.go @@ -2,7 +2,6 @@ package factory import ( "net/url" - "os" "testing" "github.com/cli/cli/v2/git" @@ -17,11 +16,6 @@ func (it identityTranslator) Translate(u *url.URL) *url.URL { } func Test_remoteResolver(t *testing.T) { - orig_GH_HOST := os.Getenv("GH_HOST") - t.Cleanup(func() { - os.Setenv("GH_HOST", orig_GH_HOST) - }) - tests := []struct { name string remotes func() (git.RemoteSet, error) diff --git a/pkg/cmdutil/factory_test.go b/pkg/cmdutil/factory_test.go index d0d5cf7f4..66b43dda3 100644 --- a/pkg/cmdutil/factory_test.go +++ b/pkg/cmdutil/factory_test.go @@ -48,10 +48,7 @@ func Test_executable(t *testing.T) { } oldPath := os.Getenv("PATH") - t.Cleanup(func() { - os.Setenv("PATH", oldPath) - }) - os.Setenv("PATH", strings.Join([]string{bin1, bin2, bin3, oldPath}, string(os.PathListSeparator))) + t.Setenv("PATH", strings.Join([]string{bin1, bin2, bin3, oldPath}, string(os.PathListSeparator))) if got := executable(""); got != bin2Exe { t.Errorf("executable() = %q, want %q", got, bin2Exe) diff --git a/pkg/iostreams/color_test.go b/pkg/iostreams/color_test.go index 74cb92048..e88fe79e3 100644 --- a/pkg/iostreams/color_test.go +++ b/pkg/iostreams/color_test.go @@ -1,22 +1,12 @@ package iostreams import ( - "os" "testing" "github.com/stretchr/testify/assert" ) func TestEnvColorDisabled(t *testing.T) { - orig_NO_COLOR := os.Getenv("NO_COLOR") - orig_CLICOLOR := os.Getenv("CLICOLOR") - orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") - t.Cleanup(func() { - os.Setenv("NO_COLOR", orig_NO_COLOR) - os.Setenv("CLICOLOR", orig_CLICOLOR) - os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) - }) - tests := []struct { name string NO_COLOR string @@ -62,9 +52,9 @@ func TestEnvColorDisabled(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv("NO_COLOR", tt.NO_COLOR) - os.Setenv("CLICOLOR", tt.CLICOLOR) - os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + t.Setenv("NO_COLOR", tt.NO_COLOR) + t.Setenv("CLICOLOR", tt.CLICOLOR) + t.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) if got := EnvColorDisabled(); got != tt.want { t.Errorf("EnvColorDisabled(): want %v, got %v", tt.want, got) @@ -74,15 +64,6 @@ func TestEnvColorDisabled(t *testing.T) { } func TestEnvColorForced(t *testing.T) { - orig_NO_COLOR := os.Getenv("NO_COLOR") - orig_CLICOLOR := os.Getenv("CLICOLOR") - orig_CLICOLOR_FORCE := os.Getenv("CLICOLOR_FORCE") - t.Cleanup(func() { - os.Setenv("NO_COLOR", orig_NO_COLOR) - os.Setenv("CLICOLOR", orig_CLICOLOR) - os.Setenv("CLICOLOR_FORCE", orig_CLICOLOR_FORCE) - }) - tests := []struct { name string NO_COLOR string @@ -135,9 +116,9 @@ func TestEnvColorForced(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - os.Setenv("NO_COLOR", tt.NO_COLOR) - os.Setenv("CLICOLOR", tt.CLICOLOR) - os.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) + t.Setenv("NO_COLOR", tt.NO_COLOR) + t.Setenv("CLICOLOR", tt.CLICOLOR) + t.Setenv("CLICOLOR_FORCE", tt.CLICOLOR_FORCE) if got := EnvColorForced(); got != tt.want { t.Errorf("EnvColorForced(): want %v, got %v", tt.want, got)