diff --git a/dev/sg/internal/run/BUILD.bazel b/dev/sg/internal/run/BUILD.bazel index 3b77e0c9210..af9d85ceeba 100644 --- a/dev/sg/internal/run/BUILD.bazel +++ b/dev/sg/internal/run/BUILD.bazel @@ -43,7 +43,6 @@ go_test( srcs = [ "docker_command_test.go", "logger_test.go", - "run_test.go", ], embed = [":run"], tags = [TAG_INFRA_DEVINFRA], diff --git a/dev/sg/internal/run/command.go b/dev/sg/internal/run/command.go index 83afffd6fa9..a20237f7902 100644 --- a/dev/sg/internal/run/command.go +++ b/dev/sg/internal/run/command.go @@ -265,7 +265,7 @@ type outputOptions struct { } func startSgCmd(ctx context.Context, cmd SGConfigCommand, parentEnv map[string]string) (*startedCmd, error) { - ex, err := cmd.GetExecCmd(ctx) + exec, err := cmd.GetExecCmd(ctx) if err != nil { return nil, err } @@ -279,12 +279,9 @@ func startSgCmd(ctx context.Context, cmd SGConfigCommand, parentEnv map[string]s } opts := commandOptions{ - name: conf.Name, - exec: ex, - // The ordering here is quite important because we want to allow the `parentEnv` and `secretsEnv` - // to override the config values for a command. This ensures env vars in the sg overwrite file - // are always the final authority. - env: makeEnv(conf.Env, parentEnv, secretsEnv), + name: conf.Name, + exec: exec, + env: makeEnv(parentEnv, secretsEnv, conf.Env), dir: conf.RepositoryRoot, stdout: outputOptions{ignore: conf.IgnoreStdout}, stderr: outputOptions{ignore: conf.IgnoreStderr}, diff --git a/dev/sg/internal/run/run_test.go b/dev/sg/internal/run/run_test.go deleted file mode 100644 index 7936a4f7eb9..00000000000 --- a/dev/sg/internal/run/run_test.go +++ /dev/null @@ -1,56 +0,0 @@ -package run - -import ( - "os" - "testing" -) - -func TestMakeEnvMap(t *testing.T) { - env1 := map[string]string{ - "SRC_TEST_ENV_NAME": "SRC_TEST_ENV_VALUE", - } - env2 := map[string]string{ - "SRC_TEST_ENV_NAME": "SRC_TEST_ENV_VALUE2", - } - env3 := map[string]string{ - "SRC_TEST_ENV_NAME": "SRC_TEST_ENV_VALUE3", - } - - testcases := []struct { - envs []map[string]string - want string - }{ - { - envs: []map[string]string{env1}, - want: "SRC_TEST_ENV_VALUE", - }, - { - envs: []map[string]string{env1, env2}, - want: "SRC_TEST_ENV_VALUE2", - }, - { - envs: []map[string]string{env1, env2, env3}, - want: "SRC_TEST_ENV_VALUE3", - }, - } - - t.Run("rightmost value takes precedence", func(t *testing.T) { - for _, tc := range testcases { - if got := makeEnvMap(tc.envs...); got["SRC_TEST_ENV_NAME"] != tc.want { - t.Errorf("makeEnvMap(%v) = %v, want %v", tc.envs, got, tc.want) - } - } - }) - - t.Run("os.Environ() is included and not overwritten", func(t *testing.T) { - if err := os.Setenv("SRC_TEST_ENV_NAME", "PROCESS_ENV_VALUE"); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - os.Unsetenv("SRC_TEST_ENV_NAME") - }) - if got := makeEnvMap(env1); got["SRC_TEST_ENV_NAME"] != "PROCESS_ENV_VALUE" { - t.Errorf("makeEnvMap(%v) = %v, want %v", env1, got, "PROCESS_ENV_VALUE") - } - }) -}