Skip to content

Commit

Permalink
tests: ensure that tests restore env-var values (#11309)
Browse files Browse the repository at this point in the history
Fix a test corruption issue, where a test accidentally unsets
the `NOMAD_LICENSE` environment variable, that's relied on by some
tests.

As a habit, tests should always restore the environment variable value
on test completion. Golang 1.17 introduced
[`t.Setenv`](https://pkg.go.dev/testing#T.Setenv) to address this issue.
However, as 1.0.x and 1.1.x branches target golang 1.15 and 1.16, I
opted to use a helper function to ease backports.
  • Loading branch information
Mahmood Ali authored Oct 13, 2021
1 parent 6852f21 commit ff1b2f7
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 22 deletions.
3 changes: 1 addition & 2 deletions command/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,8 +346,7 @@ job "example" {
}
`

os.Setenv("NOMAD_VAR_var4", "from-envvar")
defer os.Unsetenv("NOMAD_VAR_var4")
setEnv(t, "NOMAD_VAR_var4", "from-envvar")

cliArgs := []string{`var2=from-cli`}
fileVars := `var3 = "from-varfile"`
Expand Down
29 changes: 9 additions & 20 deletions command/meta_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import (
"os"
"reflect"
"sort"
"strings"
"testing"

"github.com/kr/pty"
"github.com/mitchellh/cli"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestMeta_FlagSet(t *testing.T) {
Expand Down Expand Up @@ -90,7 +89,7 @@ func TestMeta_Colorize(t *testing.T) {
{
Name: "disable colors via env var",
SetupFn: func(t *testing.T, m *Meta) {
os.Setenv(EnvNomadCLINoColor, "1")
setEnv(t, EnvNomadCLINoColor, "1")
m.SetupUi([]string{})
},
ExpectColor: false,
Expand All @@ -105,7 +104,7 @@ func TestMeta_Colorize(t *testing.T) {
{
Name: "force colors via env var",
SetupFn: func(t *testing.T, m *Meta) {
os.Setenv(EnvNomadCLIForceColor, "1")
setEnv(t, EnvNomadCLIForceColor, "1")
m.SetupUi([]string{})
},
ExpectColor: true,
Expand All @@ -120,7 +119,7 @@ func TestMeta_Colorize(t *testing.T) {
{
Name: "no color take predecence over force color via env var",
SetupFn: func(t *testing.T, m *Meta) {
os.Setenv(EnvNomadCLINoColor, "1")
setEnv(t, EnvNomadCLINoColor, "1")
m.SetupUi([]string{"-force-color"})
},
ExpectColor: false,
Expand All @@ -131,34 +130,24 @@ func TestMeta_Colorize(t *testing.T) {
t.Run(tc.Name, func(t *testing.T) {
// Create fake test terminal.
_, tty, err := pty.Open()
if err != nil {
t.Fatalf("%v", err)
}
require.NoError(t, err)
defer tty.Close()

oldStdout := os.Stdout
defer func() { os.Stdout = oldStdout }()
os.Stdout = tty

// Make sure Nomad environment variables are clean.
for _, envVar := range os.Environ() {
if strings.HasPrefix(envVar, "NOMAD") {
k := strings.SplitN(envVar, "=", 2)[0]
os.Unsetenv(k)
}
}
// Make sure color related environment variables are clean.
setEnv(t, EnvNomadCLIForceColor, "")
setEnv(t, EnvNomadCLINoColor, "")

// Run test case.
m := &Meta{}
if tc.SetupFn != nil {
tc.SetupFn(t, m)
}

if tc.ExpectColor {
assert.False(t, m.Colorize().Disable)
} else {
assert.True(t, m.Colorize().Disable)
}
require.Equal(t, !tc.ExpectColor, m.Colorize().Disable)
})
}
}
15 changes: 15 additions & 0 deletions command/util_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package command

import (
"os"
"testing"

"github.com/hashicorp/nomad/api"
Expand Down Expand Up @@ -106,3 +107,17 @@ func testMultiRegionJob(jobID, region, datacenter string) *api.Job {

return job
}

// setEnv wraps os.Setenv(key, value) and restores the environment variable to initial value in test cleanup
func setEnv(t *testing.T, key, value string) {
initial, ok := os.LookupEnv(key)
os.Setenv(key, value)

t.Cleanup(func() {
if ok {
os.Setenv(key, initial)
} else {
os.Unsetenv(key)
}
})
}

0 comments on commit ff1b2f7

Please sign in to comment.