Config stores env.Env, mostly for DumpEnv

Author: pda

internal/config/config.go

@@ -38,6 +38,9 @@ type Config struct {
 	Branch string
 	// JobRetryCount is the count of the number of times the job has been retried.
 	JobRetryCount int
+	// Env provides access to environment variables.
+	// It's public because many tests in other packages reference it (perhaps they should not).
+	Env env.Env
 	// errs is a map of environment variables name and the validation errors associated with them.
 	errs InvalidConfigError
 }
@@ -46,6 +49,7 @@ type Config struct {
 // It returns Config struct and an InvalidConfigError if there is an invalid configuration.
 func New(env env.Env) (Config, error) {
 	c := Config{
+		Env:  env,
 		errs: InvalidConfigError{},
 	}
 

internal/config/config_test.go

@@ -27,7 +27,9 @@ func getExampleEnv() env.Env {
 }
 
 func TestNewConfig(t *testing.T) {
-	c, err := New(getExampleEnv())
+	env := getExampleEnv()
+
+	c, err := New(env)
 	if err != nil {
 		t.Errorf("config.New() error = %v", err)
 	}
@@ -44,6 +46,7 @@ func TestNewConfig(t *testing.T) {
 		SuiteSlug:        "my_suite",
 		TestRunner:       "rspec",
 		JobRetryCount:    0,
+		Env:              env,
 		errs:             InvalidConfigError{},
 	}
 
@@ -82,6 +85,7 @@ func TestNewConfig_MissingConfigWithDefault(t *testing.T) {
 		TestRunner:       "rspec",
 		ResultPath:       "tmp/rspec.json",
 		JobRetryCount:    0,
+		Env:              env,
 	}
 
 	if diff := cmp.Diff(c, want, cmpopts.IgnoreUnexported(Config{})); diff != "" {

internal/config/env.go

@@ -35,7 +35,7 @@ func getIntEnvWithDefault(env env.Env, key string, defaultValue int) (int, error
 	return valueInt, nil
 }
 
-func (c Config) DumpEnv(env env.Env) map[string]string {
+func (c Config) DumpEnv() map[string]string {
 	keys := []string{
 		"BUILDKITE_BUILD_ID",
 		"BUILDKITE_JOB_ID",
@@ -58,7 +58,7 @@ func (c Config) DumpEnv(env env.Env) map[string]string {
 
 	envs := make(map[string]string)
 	for _, key := range keys {
-		envs[key] = env.Get(key)
+		envs[key] = c.Env.Get(key)
 	}
 
 	envs["BUILDKITE_TEST_ENGINE_IDENTIFIER"] = c.Identifier

main.go

@@ -86,7 +86,7 @@ func main() {
 		OrganizationSlug: cfg.OrganizationSlug,
 	})
 
-	testPlan, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, env, files, testRunner)
+	testPlan, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, files, testRunner)
 	if err != nil {
 		logErrorAndExit(16, "Couldn't fetch or create test plan: %v", err)
 	}
@@ -120,7 +120,7 @@ func main() {
 	// At this point, the runner is expected to have completed
 
 	if !testPlan.Fallback {
-		sendMetadata(ctx, apiClient, cfg, env, timeline, runResult.Statistics())
+		sendMetadata(ctx, apiClient, cfg, timeline, runResult.Statistics())
 	}
 
 	printReport(runResult, testPlan.SkippedTests, testRunner.Name())
@@ -204,10 +204,10 @@ func createTimestamp() string {
 	return time.Now().Format(time.RFC3339Nano)
 }
 
-func sendMetadata(ctx context.Context, apiClient *api.Client, cfg config.Config, env env.Env, timeline []api.Timeline, statistics runner.RunStatistics) {
+func sendMetadata(ctx context.Context, apiClient *api.Client, cfg config.Config, timeline []api.Timeline, statistics runner.RunStatistics) {
 	err := apiClient.PostTestPlanMetadata(ctx, cfg.SuiteSlug, cfg.Identifier, api.TestPlanMetadataParams{
 		Timeline:   timeline,
-		Env:        cfg.DumpEnv(env),
+		Env:        cfg.DumpEnv(),
 		Version:    version.Version,
 		Statistics: statistics,
 	})
@@ -297,7 +297,7 @@ func logErrorAndExit(exitCode int, format string, v ...any) {
 
 // fetchOrCreateTestPlan fetches a test plan from the server, or creates a
 // fallback plan if the server is unavailable or returns an error plan.
-func fetchOrCreateTestPlan(ctx context.Context, apiClient *api.Client, cfg config.Config, env env.Env, files []string, testRunner TestRunner) (plan.TestPlan, error) {
+func fetchOrCreateTestPlan(ctx context.Context, apiClient *api.Client, cfg config.Config, files []string, testRunner TestRunner) (plan.TestPlan, error) {
 	debug.Println("Fetching test plan")
 
 	// Fetch the plan from the server's cache.
@@ -339,7 +339,7 @@ func fetchOrCreateTestPlan(ctx context.Context, apiClient *api.Client, cfg confi
 
 	debug.Println("No test plan found, creating a new plan")
 	// If the cache is empty, create a new plan.
-	params, err := createRequestParam(ctx, cfg, env, files, *apiClient, testRunner)
+	params, err := createRequestParam(ctx, cfg, files, *apiClient, testRunner)
 	if err != nil {
 		return handleError(err)
 	}
@@ -366,7 +366,7 @@ func fetchOrCreateTestPlan(ctx context.Context, apiClient *api.Client, cfg confi
 // createRequestParam generates the parameters needed for a test plan request.
 // For runners other than "rspec", it constructs the test plan parameters with all test files.
 // For the "rspec" runner, it filters the test files through the Test Engine API and splits the filtered files into examples.
-func createRequestParam(ctx context.Context, cfg config.Config, env env.Env, files []string, client api.Client, runner TestRunner) (api.TestPlanParams, error) {
+func createRequestParam(ctx context.Context, cfg config.Config, files []string, client api.Client, runner TestRunner) (api.TestPlanParams, error) {
 	testFiles := []plan.TestCase{}
 	for _, file := range files {
 		testFiles = append(testFiles, plan.TestCase{
@@ -394,7 +394,7 @@ func createRequestParam(ctx context.Context, cfg config.Config, env env.Env, fil
 	// The SplitByExample flag indicates whether to filter slow files for splitting by example.
 	// Regardless of the flag's state, the API will still filter other files that need to be split by example, such as those containing skipped tests.
 	// Therefore, we must filter and split files even when SplitByExample is disabled.
-	testParams, err := filterAndSplitFiles(ctx, cfg, env, client, testFiles, runner)
+	testParams, err := filterAndSplitFiles(ctx, cfg, client, testFiles, runner)
 	if err != nil {
 		return api.TestPlanParams{}, err
 	}
@@ -411,12 +411,12 @@ func createRequestParam(ctx context.Context, cfg config.Config, env env.Env, fil
 // filterAndSplitFiles filters the test files through the Test Engine API and splits the filtered files into examples.
 // It returns the test plan parameters with the examples from the filtered files and the remaining files.
 // An error is returned if there is a failure in any of the process.
-func filterAndSplitFiles(ctx context.Context, cfg config.Config, env env.Env, client api.Client, files []plan.TestCase, runner TestRunner) (api.TestPlanParamsTest, error) {
+func filterAndSplitFiles(ctx context.Context, cfg config.Config, client api.Client, files []plan.TestCase, runner TestRunner) (api.TestPlanParamsTest, error) {
 	// Filter files that need to be split.
 	debug.Printf("Filtering %d files", len(files))
 	filteredFiles, err := client.FilterTests(ctx, cfg.SuiteSlug, api.FilterTestsParams{
 		Files: files,
-		Env:   cfg.DumpEnv(env),
+		Env:   cfg.DumpEnv(),
 	})
 
 	if err != nil {

main_test.go

@@ -342,12 +342,12 @@ func TestFetchOrCreateTestPlan(t *testing.T) {
 	defer svr.Close()
 
 	ctx := context.Background()
-	env := env.Map{}
 	cfg := config.Config{
 		NodeIndex:     0,
 		Parallelism:   10,
 		Identifier:    "identifier",
 		ServerBaseUrl: svr.URL,
+		Env:           env.Map{},
 	}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
@@ -363,7 +363,7 @@ func TestFetchOrCreateTestPlan(t *testing.T) {
 		},
 	}
 
-	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, env, files, testRunner)
+	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, files, testRunner)
 	if err != nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) error = %v", cfg, files, err)
 	}
@@ -418,8 +418,8 @@ func TestFetchOrCreateTestPlan_CachedPlan(t *testing.T) {
 		OrganizationSlug: "org",
 		SuiteSlug:        "suite",
 		Branch:           "tat-123/my-cool-feature",
+		Env:              env.Map{},
 	}
-	env := env.Map{}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl:    cfg.ServerBaseUrl,
 		OrganizationSlug: cfg.OrganizationSlug,
@@ -437,7 +437,7 @@ func TestFetchOrCreateTestPlan_CachedPlan(t *testing.T) {
 		},
 	}
 
-	got, err := fetchOrCreateTestPlan(context.Background(), apiClient, cfg, env, tests, testRunner)
+	got, err := fetchOrCreateTestPlan(context.Background(), apiClient, cfg, tests, testRunner)
 	if err != nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) error = %v", cfg, tests, err)
 	}
@@ -466,16 +466,16 @@ func TestFetchOrCreateTestPlan_PlanError(t *testing.T) {
 		Identifier:    "identifier",
 		Branch:        "tat-123/my-cool-feature",
 		ServerBaseUrl: svr.URL,
+		Env:           env.Map{},
 	}
-	env := env.Map{}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
 	})
 
 	// we want the function to return a fallback plan
 	want := plan.CreateFallbackPlan(files, cfg.Parallelism)
 
-	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, env, files, TestRunner)
+	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, files, TestRunner)
 	if err != nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) error = %v", cfg, files, err)
 	}
@@ -505,16 +505,16 @@ func TestFetchOrCreateTestPlan_InternalServerError(t *testing.T) {
 		Identifier:    "identifier",
 		Branch:        "tat-123/my-cool-feature",
 		ServerBaseUrl: svr.URL,
+		Env:           env.Map{},
 	}
-	env := env.Map{}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
 	})
 
 	// we want the function to return a fallback plan
 	want := plan.CreateFallbackPlan(files, cfg.Parallelism)
 
-	got, err := fetchOrCreateTestPlan(fetchCtx, apiClient, cfg, env, files, testRunner)
+	got, err := fetchOrCreateTestPlan(fetchCtx, apiClient, cfg, files, testRunner)
 	if err != nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) error = %v", cfg, files, err)
 	}
@@ -541,16 +541,16 @@ func TestFetchOrCreateTestPlan_BadRequest(t *testing.T) {
 		Identifier:    "identifier",
 		Branch:        "",
 		ServerBaseUrl: svr.URL,
+		Env:           env.Map{},
 	}
-	env := env.Map{}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
 	})
 
 	// we want the function to return an empty test plan and an error
 	want := plan.TestPlan{}
 
-	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, env, files, testRunner)
+	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, files, testRunner)
 	if err == nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) want error, got %v", cfg, files, err)
 	}
@@ -577,16 +577,16 @@ func TestFetchOrCreateTestPlan_BillingError(t *testing.T) {
 		Identifier:    "identifier",
 		Branch:        "",
 		ServerBaseUrl: svr.URL,
+		Env:           env.Map{},
 	}
-	env := env.Map{}
 	apiClient := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
 	})
 
 	// we want the function to return a fallback plan
 	want := plan.CreateFallbackPlan(files, cfg.Parallelism)
 
-	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, env, files, testRunner)
+	got, err := fetchOrCreateTestPlan(ctx, apiClient, cfg, files, testRunner)
 	if err != nil {
 		t.Errorf("fetchOrCreateTestPlan(ctx, %v, %v) error = %v", cfg, files, err)
 	}
@@ -614,8 +614,8 @@ func TestCreateRequestParams(t *testing.T) {
 		Parallelism:      7,
 		Branch:           "",
 		TestRunner:       "rspec",
+		Env:              env.Map{},
 	}
-	env := env.Map{}
 
 	client := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: svr.URL,
@@ -630,7 +630,7 @@ func TestCreateRequestParams(t *testing.T) {
 		"testdata/rspec/spec/fruits/grape_spec.rb",
 	}
 
-	got, err := createRequestParam(context.Background(), cfg, env, files, *client, runner.Rspec{
+	got, err := createRequestParam(context.Background(), cfg, files, *client, runner.Rspec{
 		RunnerConfig: runner.RunnerConfig{
 			TestCommand: "rspec",
 		},
@@ -708,8 +708,8 @@ func TestCreateRequestParams_NonRSpec(t *testing.T) {
 				Parallelism:      7,
 				Branch:           "",
 				TestRunner:       r.Name(),
+				Env:              env.Map{},
 			}
-			env := env.Map{}
 
 			client := api.NewClient(api.ClientConfig{
 				ServerBaseUrl: svr.URL,
@@ -720,7 +720,7 @@ func TestCreateRequestParams_NonRSpec(t *testing.T) {
 				"testdata/fruits/cherry.spec.js",
 			}
 
-			got, err := createRequestParam(context.Background(), cfg, env, files, *client, r)
+			got, err := createRequestParam(context.Background(), cfg, files, *client, r)
 
 			if err != nil {
 				t.Errorf("createRequestParam() error = %v", err)
@@ -761,8 +761,8 @@ func TestCreateRequestParams_FilterTestsError(t *testing.T) {
 		Parallelism:      7,
 		Branch:           "",
 		SplitByExample:   true,
+		Env:              env.Map{},
 	}
-	env := env.Map{}
 
 	client := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: svr.URL,
@@ -777,7 +777,7 @@ func TestCreateRequestParams_FilterTestsError(t *testing.T) {
 		"grape_spec.rb",
 	}
 
-	_, err := createRequestParam(context.Background(), cfg, env, files, *client, runner.Rspec{})
+	_, err := createRequestParam(context.Background(), cfg, files, *client, runner.Rspec{})
 
 	if err.Error() != "filter tests: forbidden" {
 		t.Errorf("createRequestParam() error = %v, want forbidden error", err)
@@ -800,8 +800,8 @@ func TestCreateRequestParams_NoFilteredFiles(t *testing.T) {
 		Parallelism:      7,
 		Branch:           "",
 		SplitByExample:   true,
+		Env:              env.Map{},
 	}
-	env := env.Map{}
 
 	client := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: svr.URL,
@@ -816,7 +816,7 @@ func TestCreateRequestParams_NoFilteredFiles(t *testing.T) {
 		"testdata/rspec/spec/fruits/grape_spec.rb",
 	}
 
-	got, err := createRequestParam(context.Background(), cfg, env, files, *client, runner.Rspec{
+	got, err := createRequestParam(context.Background(), cfg, files, *client, runner.Rspec{
 		RunnerConfig: runner.RunnerConfig{
 			TestCommand: "rspec",
 		},
@@ -934,6 +934,7 @@ func TestSendMetadata(t *testing.T) {
 		SuiteSlug:        "rspec",
 		Identifier:       "fruitsabc",
 		ServerBaseUrl:    svr.URL,
+		Env:              env,
 	}
 	client := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
@@ -943,7 +944,7 @@ func TestSendMetadata(t *testing.T) {
 		Total: 3,
 	}
 
-	sendMetadata(context.Background(), client, cfg, env, timeline, statistics)
+	sendMetadata(context.Background(), client, cfg, timeline, statistics)
 }
 
 func TestSendMetadata_Unauthorized(t *testing.T) {
@@ -957,8 +958,8 @@ func TestSendMetadata_Unauthorized(t *testing.T) {
 		SuiteSlug:        "my-suite",
 		Identifier:       "identifier",
 		ServerBaseUrl:    svr.URL,
+		Env:              env.Map{},
 	}
-	env := env.Map{}
 	client := api.NewClient(api.ClientConfig{
 		ServerBaseUrl: cfg.ServerBaseUrl,
 	})
@@ -969,5 +970,5 @@ func TestSendMetadata_Unauthorized(t *testing.T) {
 		Total: 3,
 	}
 
-	sendMetadata(context.Background(), client, cfg, env, timeline, statistics)
+	sendMetadata(context.Background(), client, cfg, timeline, statistics)
 }