diff --git a/internal/cmd/cmd_test.go b/internal/cmd/cmd_test.go index dc25c8cb..49a6ee91 100644 --- a/internal/cmd/cmd_test.go +++ b/internal/cmd/cmd_test.go @@ -955,7 +955,7 @@ func (p *selectionPlatform) SanityCheck(ctx context.Context) error { func TestResolveTestEnvironment(t *testing.T) { original := detectPlatform t.Cleanup(func() { detectPlatform = original }) - p := &selectionPlatform{framework: framework.NewJest()} + p := &selectionPlatform{framework: framework.NewJest(platform.NewJavaScript())} calls := 0 detectPlatform = func() (platform.Platform, error) { calls++ @@ -980,7 +980,7 @@ func TestCommandsRejectSelectionErrorsBeforePlanningOrExecution(t *testing.T) { original := detectPlatform t.Cleanup(func() { detectPlatform = original }) failure := errors.New("selection failed") - p := &selectionPlatform{framework: framework.NewJest()} + p := &selectionPlatform{framework: framework.NewJest(platform.NewJavaScript())} detectPlatform = func() (platform.Platform, error) { if stage == "platform" { return nil, failure @@ -1027,7 +1027,7 @@ func TestPlanDoesNotCheckTracerPrerequisites(t *testing.T) { probeReached := errors.New("runtime tag probe reached") p := &selectionPlatform{ platformName: name, - framework: framework.NewJest(), + framework: framework.NewJest(platform.NewJavaScript()), sanityErr: errors.New("tracer is not installed"), tagsErr: probeReached, } diff --git a/internal/compatibility/cucumber_test.go b/internal/compatibility/cucumber_test.go index 0c696c15..c825d6a6 100644 --- a/internal/compatibility/cucumber_test.go +++ b/internal/compatibility/cucumber_test.go @@ -10,6 +10,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" ) func TestCucumberAdapterIntegration(t *testing.T) { @@ -63,7 +64,7 @@ Given('a failing step', function () { throw new Error('unassigned file ran') }) } configureFramework(shellCommand(cucumberBinary), "") - cucumber := framework.NewCucumber() + cucumber := framework.NewCucumber(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() discovered, err := cucumber.DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: cucumber.TestPattern()}) @@ -74,7 +75,7 @@ Given('a failing step', function () { throw new Error('unassigned file ran') }) if !slices.Equal(discovered, wantFiles) { t.Fatalf("discovered = %v", discovered) } - if err := cucumber.RunTests(ctx, []string{"features/included.feature"}, nil); err != nil { + if err := cucumber.RunTests(ctx, []string{"features/included.feature"}, map[string]string{"NODE_OPTIONS": ""}); err != nil { t.Fatalf("selected-file run failed: %v", err) } } diff --git a/internal/compatibility/cypress_test.go b/internal/compatibility/cypress_test.go index 9b9c305b..3635878a 100644 --- a/internal/compatibility/cypress_test.go +++ b/internal/compatibility/cypress_test.go @@ -10,6 +10,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" ) func TestCypressAdapterIntegration(t *testing.T) { @@ -109,7 +110,7 @@ func TestCypressAdapterIntegration(t *testing.T) { } configureFramework(shellCommand(binary, "run", "--project", test.projectName), "") - cypress := framework.NewCypress() + cypress := framework.NewCypress(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) files, err := cypress.DiscoverTestFiles(ctx, discovery.TestFileSet{}) cancel() @@ -159,7 +160,7 @@ func TestCypressAdapterExecutionIntegration(t *testing.T) { command = []string{xvfb, "-a", binary, "run"} } configureFramework(shellCommand(command...), "") - cypress := framework.NewCypress() + cypress := framework.NewCypress(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() @@ -170,7 +171,7 @@ func TestCypressAdapterExecutionIntegration(t *testing.T) { wantFiles := []string{"cypress/e2e/selected.cy.js", "cypress/e2e/unselected.cy.js"} requireFiles(t, files, wantFiles) - if err := cypress.RunTests(ctx, []string{"cypress/e2e/selected.cy.js"}, nil); err != nil { + if err := cypress.RunTests(ctx, []string{"cypress/e2e/selected.cy.js"}, map[string]string{"NODE_OPTIONS": ""}); err != nil { t.Fatalf("selected-file run failed: %v", err) } } diff --git a/internal/compatibility/framework_env_test.go b/internal/compatibility/framework_env_test.go new file mode 100644 index 00000000..755a2f0d --- /dev/null +++ b/internal/compatibility/framework_env_test.go @@ -0,0 +1,16 @@ +package compatibility + +import ( + "testing" + + "github.com/DataDog/ddtest/internal/framework" +) + +func frameworkRunEnv(t *testing.T, f framework.Framework) map[string]string { + t.Helper() + env, err := f.Platform().RunEnv(framework.RuntimeOptions{ESM: f.Name() == "vitest"}) + if err != nil { + t.Fatal(err) + } + return env +} diff --git a/internal/compatibility/jest_test.go b/internal/compatibility/jest_test.go index 54ed2300..5e476709 100644 --- a/internal/compatibility/jest_test.go +++ b/internal/compatibility/jest_test.go @@ -11,6 +11,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" "github.com/DataDog/ddtest/internal/testdrive" "github.com/stretchr/testify/require" ) @@ -47,7 +48,7 @@ process.on('exit', () => { t.Setenv("NODE_OPTIONS", "--require "+strconv.Quote(filepath.Join(root, "noisy.cjs"))) t.Chdir(root) - jest := framework.NewJest() + jest := framework.NewJest(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() @@ -59,7 +60,7 @@ process.on('exit', () => { wantFiles := []string{"tests/selected.test.js", "tests/unselected.test.js"} requireFiles(t, files, wantFiles) - if err := jest.RunTests(ctx, []string{"tests/selected.test.js"}, map[string]string{"DDTEST_JEST_WORKER": "selected"}); err != nil { + if err := jest.RunTests(ctx, []string{"tests/selected.test.js"}, map[string]string{"DDTEST_JEST_WORKER": "selected", "NODE_OPTIONS": os.Getenv("NODE_OPTIONS")}); err != nil { t.Fatalf("selected-file run failed: %v", err) } } diff --git a/internal/compatibility/minitest_test.go b/internal/compatibility/minitest_test.go index 3c787c9c..89bc89cd 100644 --- a/internal/compatibility/minitest_test.go +++ b/internal/compatibility/minitest_test.go @@ -8,6 +8,8 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" + "github.com/DataDog/ddtest/internal/settings" ) func TestMinitestAdapterIntegration(t *testing.T) { @@ -50,8 +52,8 @@ end `) t.Chdir(root) - minitest := framework.NewMinitest() - minitest.SetPlatformEnv(map[string]string{"RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument"}) + minitest := framework.NewMinitest(platform.NewRuby(settings.TestSkippingLevelTest)) + t.Setenv("RUBYOPT", "-rbundler/setup -rdatadog/ci/auto_instrument") ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() diff --git a/internal/compatibility/mocha_test.go b/internal/compatibility/mocha_test.go index 226ae92d..4341f117 100644 --- a/internal/compatibility/mocha_test.go +++ b/internal/compatibility/mocha_test.go @@ -28,7 +28,7 @@ func TestMochaAdapterIntegration(t *testing.T) { writeFixture(t, root, "test/unselected.spec.js", `describe("unselected", () => { it("must not run", () => { throw new Error("unselected file ran") }) })`) t.Chdir(root) - mocha := framework.NewMocha() + mocha := framework.NewMocha(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() files, err := mocha.DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: mocha.TestPattern()}) @@ -37,7 +37,7 @@ func TestMochaAdapterIntegration(t *testing.T) { } want := []string{"test/selected.spec.js", "test/unselected.spec.js"} requireFiles(t, files, want) - if err := mocha.RunTests(ctx, []string{"test/selected.spec.js"}, nil); err != nil { + if err := mocha.RunTests(ctx, []string{"test/selected.spec.js"}, map[string]string{"NODE_OPTIONS": ""}); err != nil { t.Fatalf("selected-file run failed: %v", err) } @@ -67,7 +67,7 @@ func TestMochaAdapterCustomLocationAndCommandIntegration(t *testing.T) { t.Chdir(root) configureFramework(shellCommand(wrapper, mochaCommand), "spec/**/*.js") - mocha := framework.NewMocha() + mocha := framework.NewMocha(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() files, err := mocha.DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: mocha.TestPattern()}) @@ -75,7 +75,7 @@ func TestMochaAdapterCustomLocationAndCommandIntegration(t *testing.T) { t.Fatal(err) } requireFiles(t, files, []string{"spec/custom.spec.js"}) - if err := mocha.RunTests(ctx, files, nil); err != nil { + if err := mocha.RunTests(ctx, files, map[string]string{"NODE_OPTIONS": ""}); err != nil { t.Fatalf("custom-command run failed: %v", err) } } @@ -131,9 +131,9 @@ func testMochaActionPreloadIntegration(t *testing.T, explicit bool) { fw, err = javascript.DetectFramework() require.NoError(t, err) if explicit { - require.Empty(t, fw.GetPlatformEnv(), "worker should inherit the customer's absolute preload") + require.Empty(t, frameworkRunEnv(t, fw), "worker should inherit the customer's absolute preload") } else { - require.Equal(t, "-r "+strconv.Quote(preload), fw.GetPlatformEnv()["NODE_OPTIONS"]) + require.Equal(t, "-r "+strconv.Quote(preload), frameworkRunEnv(t, fw)["NODE_OPTIONS"]) } files, err = fw.DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: fw.TestPattern()}) require.NoError(t, err) diff --git a/internal/compatibility/playwright_test.go b/internal/compatibility/playwright_test.go index 603a568b..b008df02 100644 --- a/internal/compatibility/playwright_test.go +++ b/internal/compatibility/playwright_test.go @@ -13,6 +13,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" ) func TestPlaywrightAdapterIntegration(t *testing.T) { @@ -72,7 +73,7 @@ func TestPlaywrightAdapterIntegration(t *testing.T) { baseCommand := []string{binary, "test", "--config", "apps/web/playwright.config.js"} configureFramework(shellCommand(baseCommand...), "") - playwright := framework.NewPlaywright() + playwright := framework.NewPlaywright(platform.NewJavaScript()) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() files, err := playwright.DiscoverTestFiles(ctx, discovery.TestFileSet{}) @@ -84,8 +85,8 @@ func TestPlaywrightAdapterIntegration(t *testing.T) { projectCommand := append(append([]string{}, baseCommand...), "--project", "one") configureFramework(shellCommand(projectCommand...), "") - projectPlaywright := framework.NewPlaywright() - if err := projectPlaywright.RunTests(ctx, []string{"apps/web/tests/a.spec.ts"}, nil); err != nil { + projectPlaywright := framework.NewPlaywright(platform.NewJavaScript()) + if err := projectPlaywright.RunTests(ctx, []string{"apps/web/tests/a.spec.ts"}, map[string]string{"NODE_OPTIONS": ""}); err != nil { t.Fatalf("running one assigned file failed: %v", err) } if source, ok := playwright.SourceFileForSuite("a.spec.ts"); !ok || source != "apps/web/tests/a.spec.ts" { @@ -94,7 +95,7 @@ func TestPlaywrightAdapterIntegration(t *testing.T) { emptyCommand := append(append([]string{}, baseCommand...), "__ddtest_no_match__") configureFramework(shellCommand(emptyCommand...), "") - emptyPlaywright := framework.NewPlaywright() + emptyPlaywright := framework.NewPlaywright(platform.NewJavaScript()) if files, err := emptyPlaywright.DiscoverTestFiles(ctx, discovery.TestFileSet{}); err != nil || len(files) != 0 { t.Fatalf("empty native discovery = %v, %v", files, err) } @@ -102,7 +103,7 @@ func TestPlaywrightAdapterIntegration(t *testing.T) { writeFixture(t, projectRoot, "tests/broken.spec.ts", "throw new Error('collection exploded')\n") brokenCommand := append(append([]string{}, baseCommand...), "broken.spec.ts") configureFramework(shellCommand(brokenCommand...), "") - brokenPlaywright := framework.NewPlaywright() + brokenPlaywright := framework.NewPlaywright(platform.NewJavaScript()) if _, err := brokenPlaywright.DiscoverTestFiles(ctx, discovery.TestFileSet{}); err == nil { t.Fatal("collection failure was accepted as an empty discovery") } diff --git a/internal/compatibility/project_environment_test.go b/internal/compatibility/project_environment_test.go index 5e71134f..fec3dcce 100644 --- a/internal/compatibility/project_environment_test.go +++ b/internal/compatibility/project_environment_test.go @@ -182,6 +182,27 @@ atexit.register(lambda: print('shutdown stderr', file=sys.stderr)) tags, err := python.CreateTagsMap(ctx) require.NoError(t, err, "planning tags must not require an installed tracer") requireRuntimeTags(t, tags, "python") + resetSettingsAfterTest(t) + // A custom discovery command can supply the report without importing ddtrace. + writeFixture(t, root, "discovery.py", `import json, os +from pathlib import Path +report = Path(os.environ["DD_TEST_OPTIMIZATION_DISCOVERY_FILE"]) +report.parent.mkdir(parents=True, exist_ok=True) +report.write_text(json.dumps({"name": "test_example", "suite": "test_example.py", "suiteSourceFile": "test_example.py"})) +`) + writeFixture(t, root, "test_example.py", "def test_example(): pass\n") + configureFramework(shellCommand("python", filepath.Join(root, "discovery.py")), "") + pytest := framework.NewPytest(python) + files := discovery.TestFileSet{Pattern: "test_*.py"} + tests, err := pytest.DiscoverTests(ctx, files) + require.NoError(t, err) + require.Len(t, tests, 1) + require.Equal(t, "test_example", tests[0].Name) + require.Equal(t, "test_example.py", tests[0].SuiteSourceFile) + discovered, err := pytest.DiscoverTestFiles(ctx, files) + require.NoError(t, err) + require.Equal(t, []string{"test_example.py"}, discovered) + t.Setenv("PYTHONPATH", filepath.Join(root, "packages")) require.NoError(t, python.SanityCheck(ctx)) version, err := python.DetectTracer(ctx, platform.TracerOptions{Command: "python"}) @@ -210,7 +231,7 @@ func TestRubyTagsWithoutTracer(t *testing.T) { resetSettingsAfterTest(t) writeFixture(t, root, "discovery.rb", "File.write('discovery-ran', 'unexpected')\n") configureFramework(shellCommand("ruby", filepath.Join(root, "discovery.rb")), "") - for _, fw := range []framework.Framework{framework.NewRSpec(), framework.NewMinitest()} { + for _, fw := range []framework.Framework{framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), framework.NewMinitest(platform.NewRuby(settings.TestSkippingLevelTest))} { t.Run(fw.Name(), func(t *testing.T) { t.Cleanup(func() { _ = os.Remove(filepath.Join(root, "discovery-ran")) }) writeFixture(t, root, "example_test.rb", "# File discovery needs no tracer.\n") diff --git a/internal/compatibility/pytest_test.go b/internal/compatibility/pytest_test.go index 3a7a6a3d..e85dcdb0 100644 --- a/internal/compatibility/pytest_test.go +++ b/internal/compatibility/pytest_test.go @@ -7,6 +7,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" ) func TestPyTestAdapterIntegration(t *testing.T) { @@ -29,8 +30,8 @@ def test_preserves_worker_environment(): t.Chdir(root) configureFramework(shellCommand(python, "-m", "pytest"), "") - pytest := framework.NewPytest() - pytest.SetPlatformEnv(map[string]string{"PYTEST_ADDOPTS": "--ddtrace"}) + pytest := framework.NewPytest(platform.NewPython()) + t.Setenv("PYTEST_ADDOPTS", "--ddtrace") ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() diff --git a/internal/compatibility/rspec_test.go b/internal/compatibility/rspec_test.go index 3fe2fc51..cd8591eb 100644 --- a/internal/compatibility/rspec_test.go +++ b/internal/compatibility/rspec_test.go @@ -8,6 +8,8 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" + "github.com/DataDog/ddtest/internal/settings" ) func TestRSpecAdapterIntegration(t *testing.T) { @@ -35,8 +37,8 @@ end `) t.Chdir(root) - rspec := framework.NewRSpec() - rspec.SetPlatformEnv(map[string]string{"RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument"}) + rspec := framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)) + t.Setenv("RUBYOPT", "-rbundler/setup -rdatadog/ci/auto_instrument") ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() diff --git a/internal/compatibility/vitest_test.go b/internal/compatibility/vitest_test.go index 8f08e9c4..75b0b07d 100644 --- a/internal/compatibility/vitest_test.go +++ b/internal/compatibility/vitest_test.go @@ -18,6 +18,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" "github.com/DataDog/ddtest/internal/settings" ) @@ -95,7 +96,14 @@ test('must not run', () => { t.Chdir(root) configureVitest("vitest.unit.mjs") - vitest := framework.NewVitest() + vitest := framework.NewVitest(platform.NewJavaScript()) + runVitest := func(ctx context.Context, files []string, env map[string]string) error { + if env == nil { + env = make(map[string]string) + } + env["NODE_OPTIONS"] = "" + return framework.NewVitest(platform.NewJavaScript()).RunTests(ctx, files, env) + } ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() @@ -108,7 +116,7 @@ test('must not run', () => { requireFiles(t, files, wantFiles) lifecycle := filepath.Join(root, "lifecycle.txt") - if err := vitest.RunTests(ctx, []string{"checks/selected.check.js"}, map[string]string{ + if err := runVitest(ctx, []string{"checks/selected.check.js"}, map[string]string{ "DDTEST_VITEST_WORKER": "selected", "DDTEST_VITEST_CONFIG_EVENTS": lifecycle, }); err != nil { t.Fatalf("selected-file run failed: %v", err) @@ -128,7 +136,7 @@ test('must not run', () => { // Leave ample startup time, but fail if shutdown waits on the leaked timer. shutdownCtx, cancel := context.WithTimeout(t.Context(), 10*time.Second) defer cancel() - err := framework.NewVitest().RunTests(shutdownCtx, []string{"checks/selected.check.js"}, map[string]string{ + err := runVitest(shutdownCtx, []string{"checks/selected.check.js"}, map[string]string{ "DDTEST_VITEST_WORKER": "selected", "DDTEST_LEAK_HANDLE": "true", "DDTEST_VITEST_TEARDOWN": teardown, }) if contents, readErr := os.ReadFile(teardown); readErr != nil || string(contents) != "completed" { @@ -174,7 +182,7 @@ test('runs only in its assigned batch', () => { } configureVitest("vitest.overlap.mjs") name := filepath.Base(filepath.Dir(tc.selected)) - if err := framework.NewVitest().RunTests(ctx, []string{tc.selected}, map[string]string{ + if err := runVitest(ctx, []string{tc.selected}, map[string]string{ "DDTEST_VITEST_WORKER": name, "DDTEST_VITEST_EVENTS": events, "DDTEST_SHARD": tc.shard, }); err != nil { t.Fatalf("exact-file run failed: %v", err) @@ -191,11 +199,11 @@ test('runs only in its assigned batch', () => { // An empty batch must not turn into an unfiltered full-suite run. configureVitest("vitest.overlap.mjs") - if err := framework.NewVitest().RunTests(ctx, nil, nil); err != nil { + if err := runVitest(ctx, nil, nil); err != nil { t.Fatal(err) } // Preserve the framework's nonzero exit status on an assigned test failure. - err := framework.NewVitest().RunTests(ctx, []string{"src/endOfYear/test.js"}, map[string]string{"DDTEST_VITEST_WORKER": "wrong"}) + err := runVitest(ctx, []string{"src/endOfYear/test.js"}, map[string]string{"DDTEST_VITEST_WORKER": "wrong"}) if err == nil || !strings.Contains(err.Error(), "exit status 1") { t.Fatalf("expected assigned-test failure, got %v", err) } @@ -244,7 +252,7 @@ test('must not run', () => { throw new Error('file assignment lost') }) want = append(want, "two") } configureVitest("multi-project.config.mjs") - if err := framework.NewVitest().RunTests(ctx, []string{"project-checks/selected.test.js"}, map[string]string{"DDTEST_VITEST_EVENTS": events, "DDTEST_PROJECT_FILTER": project}); err != nil { + if err := runVitest(ctx, []string{"project-checks/selected.test.js"}, map[string]string{"DDTEST_VITEST_EVENTS": events, "DDTEST_PROJECT_FILTER": project}); err != nil { t.Fatal(err) } contents, err := os.ReadFile(events) @@ -286,7 +294,7 @@ test('must not run', () => { throw new Error('file assignment lost') }) test('snapshot policy', () => { expect({ assigned: true }).toMatchSnapshot() }) `) configureVitest("snapshot.config.mjs") - err := framework.NewVitest().RunTests(ctx, []string{"snapshot.test.js"}, map[string]string{"CI": "true"}) + err := runVitest(ctx, []string{"snapshot.test.js"}, map[string]string{"CI": "true"}) if err == nil || !strings.Contains(err.Error(), "exit status 1") { t.Fatalf("expected missing-snapshot failure in CI, got %v", err) } @@ -294,7 +302,7 @@ test('snapshot policy', () => { expect({ assigned: true }).toMatchSnapshot() }) if _, err := os.Stat(snapshot); !os.IsNotExist(err) { t.Fatalf("snapshot should not be written without explicit update, got %v", err) } - if err := framework.NewVitest().RunTests(ctx, []string{"snapshot.test.js"}, map[string]string{"CI": "true", "DDTEST_UPDATE_SNAPSHOTS": "true"}); err != nil { + if err := runVitest(ctx, []string{"snapshot.test.js"}, map[string]string{"CI": "true", "DDTEST_UPDATE_SNAPSHOTS": "true"}); err != nil { t.Fatalf("explicit snapshot update failed: %v", err) } if _, err := os.Stat(snapshot); err != nil { @@ -313,7 +321,7 @@ import { add } from './math.js' test('records coverage', () => { expect(add(1, 2)).toBe(3) }) `) configureVitest("coverage.config.mjs") - if err := framework.NewVitest().RunTests(ctx, []string{"coverage.test.js"}, nil); err != nil { + if err := runVitest(ctx, []string{"coverage.test.js"}, nil); err != nil { t.Fatal(err) } report, err := os.ReadFile(filepath.Join(root, "coverage-report/coverage-final.json")) @@ -341,11 +349,11 @@ test('records coverage', () => { expect(add(1, 2)).toBe(3) }) t.Run("configuration errors fail discovery and execution", func(t *testing.T) { writeFixture(t, root, "broken.config.mjs", `throw new Error('invalid fixture config')`) configureVitest("broken.config.mjs") - _, err := framework.NewVitest().DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: "**/*.test.js"}) + _, err := framework.NewVitest(platform.NewJavaScript()).DiscoverTestFiles(ctx, discovery.TestFileSet{Pattern: "**/*.test.js"}) if err == nil || !strings.Contains(err.Error(), "broken.config.mjs") { t.Fatalf("expected configuration error, got %v", err) } - err = framework.NewVitest().RunTests(ctx, []string{"src/endOfYear/test.js"}, nil) + err = runVitest(ctx, []string{"src/endOfYear/test.js"}, nil) if err == nil || !strings.Contains(err.Error(), "exit status 1") { t.Fatalf("expected configuration failure, got %v", err) } @@ -353,11 +361,11 @@ test('records coverage', () => { expect(add(1, 2)).toBe(3) }) t.Run("no matching specification", func(t *testing.T) { configureVitest("vitest.unit.mjs") - err := framework.NewVitest().RunTests(ctx, []string{"src/endOfYear/test.js"}, nil) + err := runVitest(ctx, []string{"src/endOfYear/test.js"}, nil) if err == nil || !strings.Contains(err.Error(), "exit status 1") { t.Fatalf("expected no-test failure, got %v", err) } - if err := framework.NewVitest().RunTests(ctx, []string{"src/endOfYear/test.js"}, map[string]string{"DDTEST_PASS_WITH_NO_TESTS": "true"}); err != nil { + if err := runVitest(ctx, []string{"src/endOfYear/test.js"}, map[string]string{"DDTEST_PASS_WITH_NO_TESTS": "true"}); err != nil { t.Fatalf("passWithNoTests failed: %v", err) } }) @@ -409,9 +417,9 @@ test('ddtest unassigned traced file', () => { throw new Error('unassigned file r t.Chdir(root) t.Setenv("NODE_OPTIONS", "") configureVitest("") - vitest := framework.NewVitest() + settings.Get().Framework = "vitest" tracerRoot := filepath.Join(tracerModules, "dd-trace") - vitest.SetPlatformEnv(map[string]string{ + for key, value := range map[string]string{ "NODE_OPTIONS": "--require " + strconv.Quote(filepath.Join(tracerRoot, "ci/init.js")) + " --import " + strconv.Quote(filepath.Join(tracerRoot, "register.js")), "DD_TRACE_AGENT_URL": agent.URL, @@ -423,7 +431,14 @@ test('ddtest unassigned traced file', () => { throw new Error('unassigned file r "DD_GIT_METADATA_ENABLED": "false", "DD_INSTRUMENTATION_TELEMETRY_ENABLED": "false", "DD_TRACE_STARTUP_LOGS": "false", - }) + } { + t.Setenv(key, value) + } + p := platform.NewJavaScript() + vitest, err := p.DetectFramework() + if err != nil { + t.Fatal(err) + } for _, tc := range []struct { name string failure bool diff --git a/internal/framework/command_args_test.go b/internal/framework/command_args_test.go index 50c4ceac..4d9d70ee 100644 --- a/internal/framework/command_args_test.go +++ b/internal/framework/command_args_test.go @@ -44,8 +44,8 @@ func TestRubyAndPythonSelectedFileArguments(t *testing.T) { runner Framework want []string }{ - {&RSpec{executor: executor, commandOverride: []string{"bundle", "exec", "rspec", "--tag", "smoke", "--", "old.rb"}}, []string{"exec", "rspec", "--tag", "smoke", "--format", "progress", "--", "selected"}}, - {&PyTest{executor: executor, commandOverride: []string{"python", "-m", "pytest", "-k", "smoke", "--", "old.py"}}, []string{"-m", "pytest", "-k", "smoke", "--", "selected"}}, + {&RSpec{platform: &testPlatform{}, executor: executor, commandOverride: []string{"bundle", "exec", "rspec", "--tag", "smoke", "--", "old.rb"}}, []string{"exec", "rspec", "--tag", "smoke", "--format", "progress", "--", "selected"}}, + {&PyTest{platform: &testPlatform{}, executor: executor, commandOverride: []string{"python", "-m", "pytest", "-k", "smoke", "--", "old.py"}}, []string{"-m", "pytest", "-k", "smoke", "--", "selected"}}, } { if err := tc.runner.RunTests(t.Context(), []string{"selected"}, nil); err != nil { t.Fatal(err) diff --git a/internal/framework/command_override_test.go b/internal/framework/command_override_test.go index 8cb6fa61..bd9447c8 100644 --- a/internal/framework/command_override_test.go +++ b/internal/framework/command_override_test.go @@ -172,7 +172,7 @@ func TestLoadCommandOverride_Integration(t *testing.T) { switch tt.frameworkType { case "rspec": - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) command, args := rspec.Command() if command != tt.expectedCommand { @@ -190,7 +190,7 @@ func TestLoadCommandOverride_Integration(t *testing.T) { } } case "minitest": - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) command, args, _ := minitest.getMinitestCommand(context.Background()) if command != tt.expectedCommand { @@ -223,15 +223,15 @@ func TestFrameworkCommand(t *testing.T) { command string args []string }{ - {NewJest(), "npx", []string{"jest"}}, - {NewMocha(), "npx", []string{"mocha"}}, - {NewCucumber(), "npx", []string{"cucumber-js"}}, - {NewVitest(), "node", nil}, - {NewPlaywright(), "npx", []string{"playwright", "test"}}, - {NewCypress(), "npx", []string{"cypress", "run"}}, - {NewPytest(), "python", []string{"-m", "pytest"}}, - {NewRSpec(), "bundle", []string{"exec", "rspec"}}, - {NewMinitest(), "bundle", []string{"exec", "rake", "test"}}, + {NewJest(&testPlatform{}), "npx", []string{"jest"}}, + {NewMocha(&testPlatform{}), "npx", []string{"mocha"}}, + {NewCucumber(&testPlatform{}), "npx", []string{"cucumber-js"}}, + {NewVitest(&testPlatform{}), "node", nil}, + {NewPlaywright(&testPlatform{}), "npx", []string{"playwright", "test"}}, + {NewCypress(&testPlatform{}), "npx", []string{"cypress", "run"}}, + {NewPytest(&testPlatform{}), "python", []string{"-m", "pytest"}}, + {NewRSpec(&testPlatform{}), "bundle", []string{"exec", "rspec"}}, + {NewMinitest(&testPlatform{}), "bundle", []string{"exec", "rake", "test"}}, } { t.Run(tc.framework.Name(), func(t *testing.T) { command, args := tc.framework.Command() @@ -245,8 +245,8 @@ func TestFrameworkCommand(t *testing.T) { path string args []string }{ - {NewJest(), binJestPath, nil}, - {NewRSpec(), binRSpecPath, nil}, + {NewJest(&testPlatform{}), binJestPath, nil}, + {NewRSpec(&testPlatform{}), binRSpecPath, nil}, } { if err := os.MkdirAll(filepath.Dir(tc.path), 0755); err != nil { t.Fatal(err) @@ -266,7 +266,7 @@ func TestFrameworkCommandPreservesOverride(t *testing.T) { viper.Reset() viper.Set("command", `npm test -- --runInBand "path with spaces"`) settings.Init() - for _, f := range []Framework{NewJest(), NewMocha(), NewCucumber(), NewPlaywright(), NewCypress(), NewPytest(), NewRSpec(), NewMinitest()} { + for _, f := range []Framework{NewJest(&testPlatform{}), NewMocha(&testPlatform{}), NewCucumber(&testPlatform{}), NewPlaywright(&testPlatform{}), NewCypress(&testPlatform{}), NewPytest(&testPlatform{}), NewRSpec(&testPlatform{}), NewMinitest(&testPlatform{})} { command, args := f.Command() if command != "npm" || !slices.Equal(args, []string{"test", "--", "--runInBand", "path with spaces"}) { t.Fatalf("%s: %s %q", f.Name(), command, args) diff --git a/internal/framework/cucumber.go b/internal/framework/cucumber.go index 8b80c2c1..319da2b0 100644 --- a/internal/framework/cucumber.go +++ b/internal/framework/cucumber.go @@ -6,7 +6,6 @@ import ( "fmt" "io" "log/slog" - "maps" "os" "path/filepath" "slices" @@ -62,7 +61,7 @@ var cucumberValueOptions = map[string]bool{ type Cucumber struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } type cucumberEnvelope struct { @@ -75,18 +74,18 @@ type cucumberEnvelope struct { } `json:"testCase"` } -func NewCucumber() *Cucumber { +func NewCucumber(p PlatformEnvironment) *Cucumber { return &Cucumber{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (c *Cucumber) SetPlatformEnv(platformEnv map[string]string) { c.platformEnv = platformEnv } -func (c *Cucumber) GetPlatformEnv() map[string]string { return c.platformEnv } -func (c *Cucumber) Name() string { return "cucumber" } -func (c *Cucumber) SupportsFullTestDiscovery() bool { return false } +func (c *Cucumber) Platform() PlatformEnvironment { return c.platform } + +func (c *Cucumber) Name() string { return "cucumber" } +func (c *Cucumber) SupportsFullTestDiscovery() bool { return false } func (c *Cucumber) SourceFileForSuite(suite string) (string, bool) { suite = utils.NormalizePath(strings.TrimSpace(suite)) @@ -116,6 +115,16 @@ func (c *Cucumber) DiscoverTests(context.Context, discovery.TestFileSet) ([]test // that survived profile, tag, name and path filtering; their Pickle envelopes // carry the feature file URI. func (c *Cucumber) DiscoverTestFiles(ctx context.Context, selectedFiles discovery.TestFileSet) ([]string, error) { + envMap, err := c.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{Env: map[string]string{cucumberPublishEnabled: "false"}}) + if err != nil { + return nil, err + } + + // Cucumber discovery always supplies NODE_OPTIONS, even when it is empty. + if _, found := envMap["NODE_OPTIONS"]; !found { + envMap["NODE_OPTIONS"] = "" + } + command, baseArgs := c.Command() if _, err := cucumberCLIArgs(command, baseArgs); err != nil { return nil, err @@ -139,7 +148,7 @@ func (c *Cucumber) DiscoverTestFiles(ctx context.Context, selectedFiles discover "--format", "message:"+filepath.Base(messagePath), ) slog.Info("Discovering Cucumber test files with command", "command", command, "args", redactCucumberArgs(args)) - output, err := c.executor.CombinedOutput(ctx, command, args, c.discoveryEnv()) + output, err := c.executor.CombinedOutput(ctx, command, args, envMap) if err != nil { message := strings.TrimSpace(string(output)) if message == "" { @@ -171,22 +180,11 @@ func (c *Cucumber) RunTests(ctx context.Context, testFiles []string, envMap map[ args = append(args, testFiles...) slog.Info("Running Cucumber tests with command", "command", command, "args", redactCucumberArgs(args)) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, c.platformEnv) - maps.Copy(mergedEnv, envMap) - return c.executor.Run(ctx, command, args, mergedEnv) -} - -func (c *Cucumber) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(c.platformEnv)+2) - maps.Copy(envMap, c.platformEnv) - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - nodeOptions, _ = os.LookupEnv(nodeOptionsEnvVar) + mergedEnv, err := c.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err } - envMap[nodeOptionsEnvVar] = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - envMap[cucumberPublishEnabled] = "false" - return envMap + return c.executor.Run(ctx, command, args, mergedEnv) } func (c *Cucumber) Command() (string, []string) { diff --git a/internal/framework/cucumber_test.go b/internal/framework/cucumber_test.go index 83a19b23..bf1cc017 100644 --- a/internal/framework/cucumber_test.go +++ b/internal/framework/cucumber_test.go @@ -81,7 +81,7 @@ func cucumberTestCaseEnvelope(pickleID string) cucumberEnvelope { } func TestCucumberBasics(t *testing.T) { - cucumber := NewCucumber() + cucumber := NewCucumber(&testPlatform{}) if cucumber.Name() != "cucumber" { t.Fatalf("Name() = %q, want cucumber", cucumber.Name()) } @@ -107,7 +107,7 @@ func TestCucumberHasUnskippableMarker(t *testing.T) { if err := os.WriteFile(marked, []byte("@datadog:unskippable\nFeature: guarded\n"), 0644); err != nil { t.Fatal(err) } - if !NewCucumber().HasUnskippableMarker(marked) { + if !NewCucumber(&testPlatform{}).HasUnskippableMarker(marked) { t.Fatal("expected @datadog:unskippable feature to be guarded") } } @@ -226,10 +226,10 @@ func TestCucumberDiscoverTestFilesUsesSelectedTestCases(t *testing.T) { cucumber := &Cucumber{ executor: executor, commandOverride: []string{"pnpm", "exec", "cucumber-js", "features/**/*.feature", "--tags", "@smoke"}, - platformEnv: map[string]string{ - "NODE_OPTIONS": "-r dd-trace/ci/init --max-old-space-size=4096", + platform: &testPlatform{env: map[string]string{ + "NODE_OPTIONS": "--max-old-space-size=4096", "CUSTOM": "value", - }, + }}, } files, err := cucumber.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: cucumber.TestPattern()}) @@ -281,7 +281,7 @@ func TestCucumberDiscoverTestFilesFiltersLocationAndExclude(t *testing.T) { cucumberPickleEnvelope("excluded", "features/excluded.feature"), cucumberTestCaseEnvelope("excluded"), cucumberPickleEnvelope("outside", "other/outside.feature"), cucumberTestCaseEnvelope("outside"), }} - cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platformEnv: map[string]string{}} + cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platform: &testPlatform{env: map[string]string{}}} files, err := cucumber.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: cucumber.TestPattern()}) if err != nil { t.Fatal(err) @@ -312,7 +312,7 @@ func TestCucumberDiscoverTestFilesEmptyGlobCandidatesStillUsesCucumber(t *testin executor := &cucumberCommandExecutor{messages: []cucumberEnvelope{ cucumberPickleEnvelope("configured", "custom/from-config.feature"), cucumberTestCaseEnvelope("configured"), }} - cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platformEnv: map[string]string{}} + cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platform: &testPlatform{env: map[string]string{}}} files, err := cucumber.DiscoverTestFiles(context.Background(), discovery.TestFileSet{ Pattern: cucumber.TestPattern(), ExplicitFiles: []string{}, @@ -330,7 +330,7 @@ func TestCucumberDiscoverTestFilesEmptyGlobCandidatesStillUsesCucumber(t *testin func TestCucumberDiscoverTestFilesReportsCommandError(t *testing.T) { executor := &cucumberCommandExecutor{output: []byte("invalid profile"), combinedErr: errors.New("exit status 1")} - cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platformEnv: map[string]string{}} + cucumber := &Cucumber{executor: executor, commandOverride: []string{"cucumber-js"}, platform: &testPlatform{env: map[string]string{}}} _, err := cucumber.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: cucumber.TestPattern()}) if err == nil || !strings.Contains(err.Error(), "invalid profile") { t.Fatalf("error = %v", err) @@ -344,7 +344,7 @@ func TestCucumberRunTestsReplacesConfiguredPaths(t *testing.T) { commandOverride: []string{ "pnpm", "exec", "cucumber-js", "features/v1/*.feature", "--profile", "ci", "--tags", "not @slow", }, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "SHARED": "platform"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "SHARED": "platform"}}, } err := cucumber.RunTests(context.Background(), []string{"features/a.feature", "features/b.feature"}, map[string]string{"SHARED": "worker"}) if err != nil { @@ -377,7 +377,7 @@ func TestParseCucumberMessagesRejectsMalformedLine(t *testing.T) { func TestCucumberRunPreservesWrapperSeparatorAndTags(t *testing.T) { executor := &cucumberCommandExecutor{} - c := &Cucumber{executor: executor, commandOverride: []string{"npx", "--", "cucumber-js", "--tags", "@smoke", "--", "old.feature"}} + c := &Cucumber{platform: &testPlatform{}, executor: executor, commandOverride: []string{"npx", "--", "cucumber-js", "--tags", "@smoke", "--", "old.feature"}} if err := c.RunTests(t.Context(), []string{"selected.feature"}, nil); err != nil { t.Fatal(err) } @@ -390,3 +390,32 @@ func TestCucumberRunPreservesWrapperSeparatorAndTags(t *testing.T) { t.Fatal(args) } } + +func TestCucumberDiscoveryAppliesNodeOptionsDefault(t *testing.T) { + for _, tc := range []struct { + name string + env map[string]string + want string + }{ + {name: "missing", env: map[string]string{}, want: ""}, + {name: "prepared by platform", env: map[string]string{"NODE_OPTIONS": "--require project-loader.cjs"}, want: "--require project-loader.cjs"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Chdir(t.TempDir()) + executor := &cucumberCommandExecutor{messages: []cucumberEnvelope{}} + cucumber := &Cucumber{ + executor: executor, + commandOverride: []string{"cucumber-js"}, + platform: &testPlatform{env: tc.env}, + } + _, err := cucumber.DiscoverTestFiles(t.Context(), discovery.TestFileSet{Pattern: "features/**/*.feature"}) + if err != nil { + t.Fatal(err) + } + value, found := executor.capturedEnv["NODE_OPTIONS"] + if !found || value != tc.want { + t.Fatalf("NODE_OPTIONS = %q (present=%t), want %q", value, found, tc.want) + } + }) + } +} diff --git a/internal/framework/cypress.go b/internal/framework/cypress.go index ff39465b..c28ec8db 100644 --- a/internal/framework/cypress.go +++ b/internal/framework/cypress.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "log/slog" - "maps" "os" "path/filepath" "slices" @@ -42,7 +41,7 @@ var cypressDiscoveryConfigScript string type Cypress struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } type cypressDiscoveryConfig struct { @@ -51,18 +50,18 @@ type cypressDiscoveryConfig struct { SpecFiles []string `json:"specFiles"` } -func NewCypress() *Cypress { +func NewCypress(p PlatformEnvironment) *Cypress { return &Cypress{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (c *Cypress) SetPlatformEnv(platformEnv map[string]string) { c.platformEnv = platformEnv } -func (c *Cypress) GetPlatformEnv() map[string]string { return c.platformEnv } -func (c *Cypress) Name() string { return "cypress" } -func (c *Cypress) SupportsFullTestDiscovery() bool { return false } +func (c *Cypress) Platform() PlatformEnvironment { return c.platform } + +func (c *Cypress) Name() string { return "cypress" } +func (c *Cypress) SupportsFullTestDiscovery() bool { return false } func (c *Cypress) SourceFileForSuite(suite string) (string, bool) { suite = strings.TrimSpace(suite) @@ -108,6 +107,11 @@ func (c *Cypress) DiscoverTests(context.Context, discovery.TestFileSet) ([]testo } func (c *Cypress) DiscoverTestFiles(ctx context.Context, selectedFiles discovery.TestFileSet) ([]string, error) { + envMap, err := c.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err + } + command, baseArgs := c.Command() cliArgs, err := cypressCLIArgs(command, baseArgs) if err != nil { @@ -129,7 +133,7 @@ func (c *Cypress) DiscoverTestFiles(ctx context.Context, selectedFiles discovery args := cypressDiscoveryArgs(command, baseArgs, discoveryConfigPath) slog.Info("Discovering Cypress test files with command", "command", command, "args", args) - output, err := c.executor.CombinedOutput(ctx, command, args, c.discoveryEnv()) + output, err := c.executor.CombinedOutput(ctx, command, args, envMap) config, configErr := parseCypressDiscoveryOutput(output) if err != nil && configErr != nil { message := strings.TrimSpace(string(output)) @@ -170,25 +174,11 @@ func (c *Cypress) RunTests(ctx context.Context, testFiles []string, envMap map[s args := cypressRunArgs(command, baseArgs, projectTestFiles) slog.Info("Running Cypress tests", "command", command, "args", args, "testFiles", testFiles) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, c.platformEnv) - maps.Copy(mergedEnv, envMap) - return c.executor.Run(ctx, command, args, mergedEnv) -} - -func (c *Cypress) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(c.platformEnv)+1) - maps.Copy(envMap, c.platformEnv) - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - var found bool - nodeOptions, found = os.LookupEnv(nodeOptionsEnvVar) - if !found { - return envMap - } + mergedEnv, err := c.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err } - envMap[nodeOptionsEnvVar] = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - return envMap + return c.executor.Run(ctx, command, args, mergedEnv) } func (c *Cypress) Command() (string, []string) { diff --git a/internal/framework/cypress_test.go b/internal/framework/cypress_test.go index ef5a68fb..47cf9d5d 100644 --- a/internal/framework/cypress_test.go +++ b/internal/framework/cypress_test.go @@ -45,7 +45,7 @@ func (e *cypressCommandExecutor) capture(name string, args []string, env map[str } func TestCypressFrameworkMetadata(t *testing.T) { - cypress := NewCypress() + cypress := NewCypress(&testPlatform{}) if cypress.Name() != "cypress" { t.Fatalf("Name() = %q, want cypress", cypress.Name()) } @@ -196,7 +196,7 @@ func TestCypressDiscoverTestFilesLoadsConfigAndFilters(t *testing.T) { commandOverride: []string{ "pnpm", "exec", "cypress", "run", "--project", "web", "--config-file", "custom.config.ts", "--record", }, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init --max-old-space-size=2048", "CUSTOM": "value"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "--max-old-space-size=2048", "CUSTOM": "value"}}, } files, err := cypress.DiscoverTestFiles(context.Background(), discovery.TestFileSet{}) @@ -261,7 +261,7 @@ func TestCypressRunTests(t *testing.T) { commandOverride: []string{ "npx", "cypress", "run", "--browser", "chrome", "--spec", "configured.cy.ts", }, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "BASE": "base"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "BASE": "base"}}, } if err := cypress.RunTests(context.Background(), []string{"a.cy.ts", "b.cy.ts"}, map[string]string{"WORKER": "1"}); err != nil { t.Fatal(err) @@ -292,7 +292,7 @@ func TestCypressRunTestsUsesPathsRelativeToSelectedProject(t *testing.T) { commandOverride: []string{ "npx", "cypress", "run", "--project", "apps/web", }, - platformEnv: make(map[string]string), + platform: &testPlatform{env: make(map[string]string)}, } testFiles := []string{ @@ -322,7 +322,7 @@ func TestCypressUnskippableMarker(t *testing.T) { if err := os.WriteFile(file, []byte("// @datadog unskippable\n"), 0644); err != nil { t.Fatal(err) } - if !NewCypress().HasUnskippableMarker(file) { + if !NewCypress(&testPlatform{}).HasUnskippableMarker(file) { t.Fatal("expected marker") } } diff --git a/internal/framework/framework.go b/internal/framework/framework.go index 468f06ae..12a01702 100644 --- a/internal/framework/framework.go +++ b/internal/framework/framework.go @@ -8,15 +8,14 @@ import ( ) type Framework interface { - // Command returns the effective test-suite command, preserving all override arguments. + // Command identifies the executable and base arguments used by the framework. Command() (string, []string) Name() string TestPattern() string DiscoverTestFiles(ctx context.Context, testFiles discovery.TestFileSet) ([]string, error) DiscoverTests(ctx context.Context, testFiles discovery.TestFileSet) ([]testoptimization.Test, error) RunTests(ctx context.Context, testFiles []string, envMap map[string]string) error - SetPlatformEnv(platformEnv map[string]string) - GetPlatformEnv() map[string]string + Platform() PlatformEnvironment SupportsFullTestDiscovery() bool SourceFileForSuite(suite string) (string, bool) HasUnskippableMarker(testFile string) bool diff --git a/internal/framework/jest.go b/internal/framework/jest.go index 35a29a98..20e0f452 100644 --- a/internal/framework/jest.go +++ b/internal/framework/jest.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "log/slog" - "maps" "os" "path/filepath" "slices" @@ -21,9 +20,7 @@ import ( ) const ( - binJestPath = "node_modules/.bin/jest" - nodeOptionsEnvVar = "NODE_OPTIONS" - ddTraceCIInitModule = "dd-trace/ci/init" + binJestPath = "node_modules/.bin/jest" ) var ErrFullTestDiscoveryUnsupported = errors.New("full test discovery is not supported") @@ -33,24 +30,18 @@ var jestTestFileExtensions = []string{"js", "jsx", "ts", "tsx", "mjs", "cjs"} type Jest struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } -func NewJest() *Jest { +func NewJest(p PlatformEnvironment) *Jest { return &Jest{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (j *Jest) SetPlatformEnv(platformEnv map[string]string) { - j.platformEnv = platformEnv -} - -func (j *Jest) GetPlatformEnv() map[string]string { - return j.platformEnv -} +func (j *Jest) Platform() PlatformEnvironment { return j.platform } func (j *Jest) Name() string { return "jest" @@ -96,12 +87,17 @@ func (j *Jest) DiscoverTestFiles(ctx context.Context, testFiles discovery.TestFi return slices.Clone(testFiles.ExplicitFiles), nil } + envMap, err := j.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err + } + command, baseArgs := j.Command() args := slices.Clone(baseArgs) args = withFrameworkOptions(command, args, "jest", "--listTests", "--json") slog.Info("Discovering Jest test files with command", "command", command, "args", args) - output, err := j.executor.CombinedOutput(ctx, command, args, j.discoveryEnv()) + output, err := j.executor.CombinedOutput(ctx, command, args, envMap) if err != nil { message := strings.TrimSpace(string(output)) if message == "" { @@ -129,27 +125,11 @@ func (j *Jest) RunTests(ctx context.Context, testFiles []string, envMap map[stri slog.Info("Running tests with command", "command", command, "args", args) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, j.platformEnv) - maps.Copy(mergedEnv, envMap) - return j.executor.Run(ctx, command, args, mergedEnv) -} - -func (j *Jest) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(j.platformEnv)+1) - maps.Copy(envMap, j.platformEnv) - - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - var found bool - nodeOptions, found = os.LookupEnv(nodeOptionsEnvVar) - if !found { - return envMap - } + mergedEnv, err := j.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err } - - envMap[nodeOptionsEnvVar] = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - return envMap + return j.executor.Run(ctx, command, args, mergedEnv) } // Decide between user custom command, local jest binary and npx jest @@ -196,10 +176,6 @@ func filterJestTestFiles(testFiles []string, selectedTestFiles discovery.TestFil return slices.Compact(filteredFiles), nil } -func stripNodeOptionsRequire(nodeOptions string, module string) string { - return utils.NodeOptionsWithoutRequire(nodeOptions, module) -} - // Jest's --listTests --json writes an array of absolute paths. Preloads and // package managers may log before or after it, even without a newline. Accept // exactly one such array; missing or ambiguous output must not become an empty diff --git a/internal/framework/jest_test.go b/internal/framework/jest_test.go index 78c8128d..7509e432 100644 --- a/internal/framework/jest_test.go +++ b/internal/framework/jest_test.go @@ -37,24 +37,24 @@ func (m *jestCommandExecutor) Run(ctx context.Context, name string, args []strin } func TestNewJest(t *testing.T) { - jest := NewJest() + jest := NewJest(&testPlatform{}) if jest == nil { - t.Fatal("NewJest() returned nil") + t.Fatal("NewJest(&testPlatform{}) returned nil") } if jest.executor == nil { - t.Error("NewJest() created Jest with nil executor") + t.Error("NewJest(&testPlatform{}) created Jest with nil executor") } } func TestJest_Name(t *testing.T) { - jest := NewJest() + jest := NewJest(&testPlatform{}) if jest.Name() != "jest" { t.Errorf("expected %q, got %q", "jest", jest.Name()) } } func TestJest_DiscoverTests_Unsupported(t *testing.T) { - jest := NewJest() + jest := NewJest(&testPlatform{}) tests, err := jest.DiscoverTests(context.Background(), discovery.TestFileSet{}) if tests != nil { @@ -69,7 +69,7 @@ func TestJest_DiscoverTests_Unsupported(t *testing.T) { } func TestJest_SourceFileForSuite(t *testing.T) { - jest := NewJest() + jest := NewJest(&testPlatform{}) sourceFile, ok := jest.SourceFileForSuite("src/example.test.js") if !ok || sourceFile != "src/example.test.js" { @@ -92,7 +92,7 @@ func TestJest_HasUnskippableMarker(t *testing.T) { t.Fatal(err) } - jest := NewJest() + jest := NewJest(&testPlatform{}) if !jest.HasUnskippableMarker(markedFile) { t.Fatal("expected marker when @datadog and unskippable are present") } @@ -141,8 +141,8 @@ func TestJest_DiscoverTestFiles_UsesLocalJestListTests(t *testing.T) { }, } jest := &Jest{ - executor: mockExecutor, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init --max-old-space-size=4096", "CUSTOM_ENV": "value"}, + executor: mockExecutor, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "--max-old-space-size=4096", "CUSTOM_ENV": "value"}}, } files, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err != nil { @@ -169,7 +169,7 @@ func TestJest_DiscoverTestFiles_UsesLocalJestListTests(t *testing.T) { } } -func TestJest_DiscoverTestFiles_StripsInheritedNodeOptions(t *testing.T) { +func TestJest_DiscoverTestFiles_UsesPlatformDiscoveryEnvironment(t *testing.T) { t.Setenv("NODE_OPTIONS", "--require dd-trace/ci/init --max-old-space-size=4096") tempDir := t.TempDir() @@ -188,7 +188,7 @@ func TestJest_DiscoverTestFiles_StripsInheritedNodeOptions(t *testing.T) { mockExecutor := &jestCommandExecutor{ output: jestListOutput(filepath.Join(tempDir, "src", "a.test.js")), } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "--max-old-space-size=4096"}}} files, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err != nil { @@ -236,7 +236,7 @@ func TestJest_DiscoverTestFiles_WithTestsLocationFiltersListTestsOutput(t *testi capturedArgs = slices.Clone(args) }, } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: make(map[string]string)}} files, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -262,7 +262,7 @@ func TestJest_DiscoverTestFiles_WithTestsLocationReturnsInvalidPatternError(t *t mockExecutor := &jestCommandExecutor{ output: []byte("[]"), } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: make(map[string]string)}} _, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err == nil { @@ -299,7 +299,7 @@ func TestJest_DiscoverTestFiles_WithTestsExcludePatternFiltersListTestsOutput(t capturedArgs = slices.Clone(args) }, } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: make(map[string]string)}} files, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -342,7 +342,7 @@ func TestJest_DiscoverTestFiles_WithOverride(t *testing.T) { jest := &Jest{ executor: mockExecutor, commandOverride: []string{"pnpm", "jest", "--runInBand"}, - platformEnv: make(map[string]string), + platform: &testPlatform{env: make(map[string]string)}, } if _, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}); err != nil { @@ -363,7 +363,7 @@ func TestJest_DiscoverTestFiles_CommandError(t *testing.T) { output: []byte("invalid jest config"), err: errors.New("exit status 1"), } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: make(map[string]string)}} _, err := jest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: jest.TestPattern()}) if err == nil { @@ -398,8 +398,8 @@ func TestJest_RunTests_UsesLocalJestBinary(t *testing.T) { }, } jest := &Jest{ - executor: mockExecutor, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "SHARED": "platform"}, + executor: mockExecutor, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "SHARED": "platform"}}, } err := jest.RunTests(context.Background(), []string{"src/a.test.js", "src/b.test.ts"}, map[string]string{"SHARED": "worker", "DD_ENV": "ci"}) @@ -441,7 +441,7 @@ func TestJest_RunTests_UsesNpxFallback(t *testing.T) { capturedArgs = slices.Clone(args) }, } - jest := &Jest{executor: mockExecutor, platformEnv: make(map[string]string)} + jest := &Jest{executor: mockExecutor, platform: &testPlatform{env: make(map[string]string)}} if err := jest.RunTests(context.Background(), []string{"src/a.test.js"}, nil); err != nil { t.Fatalf("RunTests failed: %v", err) @@ -456,20 +456,6 @@ func TestJest_RunTests_UsesNpxFallback(t *testing.T) { } } -func TestJest_SetPlatformEnv(t *testing.T) { - jest := NewJest() - env := map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "FOO": "bar"} - jest.SetPlatformEnv(env) - - got := jest.GetPlatformEnv() - if got["NODE_OPTIONS"] != "-r dd-trace/ci/init" { - t.Errorf("expected NODE_OPTIONS %q, got %q", "-r dd-trace/ci/init", got["NODE_OPTIONS"]) - } - if got["FOO"] != "bar" { - t.Errorf("expected FOO %q, got %q", "bar", got["FOO"]) - } -} - func TestJest_RunTests_WithOverride(t *testing.T) { var capturedName string var capturedArgs []string @@ -482,7 +468,7 @@ func TestJest_RunTests_WithOverride(t *testing.T) { jest := &Jest{ executor: mockExecutor, commandOverride: []string{"pnpm", "jest", "--runInBand"}, - platformEnv: make(map[string]string), + platform: &testPlatform{env: make(map[string]string)}, } if err := jest.RunTests(context.Background(), []string{"src/a.test.js"}, nil); err != nil { @@ -504,7 +490,7 @@ func TestJestSeparatorPreservesOptionsAndReplacesSelection(t *testing.T) { {"npx", "--", "jest", "--runInBand", "--", "old.test.js"}, } { var got []string - j := &Jest{commandOverride: override, executor: &jestCommandExecutor{output: []byte("[]"), onExecution: func(_ string, args []string) { got = slices.Clone(args) }}} + j := &Jest{platform: &testPlatform{}, commandOverride: override, executor: &jestCommandExecutor{output: []byte("[]"), onExecution: func(_ string, args []string) { got = slices.Clone(args) }}} if err := j.RunTests(t.Context(), []string{"selected.test.js"}, nil); err != nil { t.Fatal(err) } @@ -566,7 +552,7 @@ func TestJestDiscoveryWithNoisyOutput(t *testing.T) { {name: "missing file", output: string(jestListOutput(filepath.Join(root, "missing.test.js"))), wantError: "invalid Jest test file"}, } { t.Run(tc.name, func(t *testing.T) { - jest := &Jest{executor: &jestCommandExecutor{output: []byte(tc.output)}} + jest := &Jest{platform: &testPlatform{}, executor: &jestCommandExecutor{output: []byte(tc.output)}} files, err := jest.DiscoverTestFiles(t.Context(), discovery.TestFileSet{Pattern: "**/*.test.js"}) if tc.wantError != "" { if err == nil || !strings.Contains(err.Error(), tc.wantError) { diff --git a/internal/framework/minitest.go b/internal/framework/minitest.go index 818ee1b7..3266f434 100644 --- a/internal/framework/minitest.go +++ b/internal/framework/minitest.go @@ -2,9 +2,7 @@ package framework import ( "context" - "fmt" "log/slog" - "maps" "os" "path/filepath" "strings" @@ -25,24 +23,18 @@ const ( type Minitest struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } -func NewMinitest() *Minitest { +func NewMinitest(p PlatformEnvironment) *Minitest { return &Minitest{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (m *Minitest) SetPlatformEnv(platformEnv map[string]string) { - m.platformEnv = platformEnv -} - -func (m *Minitest) GetPlatformEnv() map[string]string { - return m.platformEnv -} +func (m *Minitest) Platform() PlatformEnvironment { return m.platform } func (m *Minitest) Name() string { return "minitest" @@ -54,14 +46,14 @@ func (m *Minitest) DiscoverTests(ctx context.Context, testFiles discovery.TestFi if testFiles.Empty() { return []testoptimization.Test{}, nil } - if err := utils.CheckRubyTracer(ctx, m.executor); err != nil { - return nil, fmt.Errorf("full test discovery requires datadog-ci: %w", err) + + envMap, err := m.platform.DiscoveryEnv(ctx, FullDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err } executable, args, isRails := m.getMinitestCommand(ctx) - envMap := make(map[string]string) - maps.Copy(envMap, m.platformEnv) if isRails { if testFiles.UseExplicitFiles() { args = withFrameworkFiles(executable, args, "rails", testFiles.ExplicitFiles) @@ -113,9 +105,10 @@ func (m *Minitest) RunTests(ctx context.Context, testFiles []string, envMap map[ } } - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, m.platformEnv) - maps.Copy(mergedEnv, envMap) + mergedEnv, err := m.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err + } return m.executor.Run(ctx, command, args, mergedEnv) } diff --git a/internal/framework/minitest_test.go b/internal/framework/minitest_test.go index c40ea8c2..03148d7c 100644 --- a/internal/framework/minitest_test.go +++ b/internal/framework/minitest_test.go @@ -26,9 +26,6 @@ type mockRailsCommandExecutor struct { } func (m *mockRailsCommandExecutor) CombinedOutput(ctx context.Context, name string, args []string, envMap map[string]string) ([]byte, error) { - if name == "bundle" && slices.Equal(args, []string{"info", "datadog-ci"}) { - return []byte(" * datadog-ci (1.31.0)"), nil - } // Capture env for assertions m.capturedEnvMap = envMap @@ -94,19 +91,19 @@ func (m *countingCommandExecutor) Run(ctx context.Context, name string, args []s } func TestNewMinitest(t *testing.T) { - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) if minitest == nil { - t.Error("NewMinitest() returned nil") + t.Error("NewMinitest(&testPlatform{}) returned nil") return } if minitest.executor == nil { - t.Error("NewMinitest() created Minitest with nil executor") + t.Error("NewMinitest(&testPlatform{}) created Minitest with nil executor") return } } func TestMinitest_Name(t *testing.T) { - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) expected := "minitest" actual := minitest.Name() @@ -680,7 +677,7 @@ func TestMinitest_DiscoverTestFiles(t *testing.T) { } } - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) discoveredFiles, err := discovery.DiscoverTestFiles(minitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { @@ -753,7 +750,7 @@ func TestMinitest_DiscoverTestFiles_WithTestsLocation(t *testing.T) { setTestsLocation(t, filepath.Join("custom", "test", "**", "*_test.rb")) - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) files, err := discovery.DiscoverTestFiles(minitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -807,7 +804,7 @@ func TestMinitest_DiscoverTestFiles_WithTestsExcludePattern(t *testing.T) { setTestsExcludePattern(t, filepath.Join("test", "system", "**", "*_test.rb")) - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) files, err := discovery.DiscoverTestFiles(minitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -843,7 +840,7 @@ func TestMinitest_DiscoverTestFiles_NoTestDirectory(t *testing.T) { _ = os.Chdir(originalDir) }() - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) discoveredFiles, err := discovery.DiscoverTestFiles(minitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { @@ -1531,19 +1528,6 @@ func TestMinitest_getMinitestCommand_RailsApplication_WithBinRails(t *testing.T) } } -func TestMinitest_SetPlatformEnv(t *testing.T) { - minitest := NewMinitest() - - platformEnv := map[string]string{ - "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", - } - minitest.SetPlatformEnv(platformEnv) - - if minitest.GetPlatformEnv()["RUBYOPT"] != platformEnv["RUBYOPT"] { - t.Errorf("expected platformEnv to be set, got %v", minitest.GetPlatformEnv()) - } -} - func TestMinitest_RunTests_UsesPlatformEnv(t *testing.T) { testFiles := []string{"test/models/user_test.rb"} @@ -1557,7 +1541,7 @@ func TestMinitest_RunTests_UsesPlatformEnv(t *testing.T) { platformEnv := map[string]string{ "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", } - minitest.SetPlatformEnv(platformEnv) + minitest.platform = &testPlatform{env: platformEnv} err := minitest.RunTests(context.Background(), testFiles, nil) if err != nil { @@ -1584,7 +1568,7 @@ func TestMinitest_RunTests_MergesPlatformEnvWithPassedEnv(t *testing.T) { "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", "PLATFORM_VAR": "platform_value", } - minitest.SetPlatformEnv(platformEnv) + minitest.platform = &testPlatform{env: platformEnv} // Pass additional env vars additionalEnv := map[string]string{ @@ -1629,7 +1613,7 @@ func TestMinitest_RunTests_AdditionalEnvOverridesPlatformEnv(t *testing.T) { "SHARED_VAR": "platform_value", "ANOTHER_VAR": "platform_another", } - minitest.SetPlatformEnv(platformEnv) + minitest.platform = &testPlatform{env: platformEnv} // Pass additional env that overrides SHARED_VAR additionalEnv := map[string]string{ @@ -1704,7 +1688,7 @@ func TestMinitest_DiscoverTests_UsesPlatformEnv(t *testing.T) { platformEnv := map[string]string{ "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", } - minitest.SetPlatformEnv(platformEnv) + minitest.platform = &testPlatform{env: platformEnv} _, err := discoverAndParseTests(t, minitest, resolveTestFilesForFramework(t, minitest.TestPattern())) if err != nil { @@ -1746,7 +1730,7 @@ func TestMinitest_RunTests_RailsApplication_UsesPlatformEnv(t *testing.T) { platformEnv := map[string]string{ "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", } - minitest.SetPlatformEnv(platformEnv) + minitest.platform = &testPlatform{env: platformEnv} err := minitest.RunTests(context.Background(), testFiles, nil) if err != nil { @@ -1764,7 +1748,7 @@ func TestMinitest_DefaultTestPattern(t *testing.T) { settings.Init() t.Cleanup(settings.Init) - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) expected := filepath.Join(minitestRootDir, "**", minitestTestFilePattern) if got := minitest.TestPattern(); got != expected { t.Errorf("expected default test pattern %q, got %q", expected, got) @@ -1772,14 +1756,14 @@ func TestMinitest_DefaultTestPattern(t *testing.T) { } func TestMinitest_SupportsFullTestDiscovery(t *testing.T) { - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) if !minitest.SupportsFullTestDiscovery() { t.Error("expected Minitest to support full test discovery") } } func TestMinitest_SourceFileForSuite(t *testing.T) { - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) sourceFile, ok := minitest.SourceFileForSuite("UserTest at test/models/user_test.rb") if !ok || sourceFile != "test/models/user_test.rb" { @@ -1794,7 +1778,7 @@ func TestMinitest_HasUnskippableMarker(t *testing.T) { t.Fatal(err) } - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) if !minitest.HasUnskippableMarker(markedFile) { t.Fatal("expected Ruby unskippable marker") } @@ -1807,7 +1791,7 @@ func TestMinitestCommandSharesRailsResolution(t *testing.T) { for _, rails := range []bool{false, true} { t.Run(fmt.Sprint(rails), func(t *testing.T) { t.Chdir(t.TempDir()) - m := &Minitest{executor: &mockRailsCommandExecutor{isRails: rails}} + m := &Minitest{platform: &testPlatform{}, executor: &mockRailsCommandExecutor{isRails: rails}} command, args := m.Command() runnerCommand, runnerArgs, isRails := m.getMinitestCommand(t.Context()) if command != runnerCommand || !slices.Equal(args, runnerArgs) || isRails != rails { diff --git a/internal/framework/mocha.go b/internal/framework/mocha.go index be2b9a41..61b0afd7 100644 --- a/internal/framework/mocha.go +++ b/internal/framework/mocha.go @@ -10,7 +10,6 @@ import ( "os" "path/filepath" "slices" - "strconv" "strings" "github.com/DataDog/ddtest/internal/discovery" @@ -32,21 +31,21 @@ var mochaAdapterScript string type Mocha struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } -func NewMocha() *Mocha { +func NewMocha(p PlatformEnvironment) *Mocha { return &Mocha{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (m *Mocha) SetPlatformEnv(platformEnv map[string]string) { m.platformEnv = platformEnv } -func (m *Mocha) GetPlatformEnv() map[string]string { return m.platformEnv } -func (m *Mocha) Name() string { return "mocha" } -func (m *Mocha) SupportsFullTestDiscovery() bool { return false } +func (m *Mocha) Platform() PlatformEnvironment { return m.platform } + +func (m *Mocha) Name() string { return "mocha" } +func (m *Mocha) SupportsFullTestDiscovery() bool { return false } func (m *Mocha) SourceFileForSuite(suite string) (string, bool) { suite = strings.TrimSpace(suite) @@ -94,11 +93,15 @@ func (m *Mocha) DiscoverTestFiles(ctx context.Context, testFiles discovery.TestF if err != nil { return nil, fmt.Errorf("failed to encode Mocha discovery request: %w", err) } - adapterPath, adapterEnv, err := prepareMochaAdapter(m.discoveryEnv(), request) + adapterPath, adapterEnv, err := prepareMochaAdapter(request) if err != nil { return nil, err } defer func() { _ = os.Remove(adapterPath) }() + adapterEnv, err = m.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{Env: adapterEnv, PreloadFiles: []string{adapterPath}}) + if err != nil { + return nil, err + } slog.Info("Discovering Mocha test files", "command", command, "args", baseArgs) output, err := m.executor.CombinedOutput(ctx, command, baseArgs, adapterEnv) @@ -132,18 +135,22 @@ func (m *Mocha) RunTests(ctx context.Context, testFiles []string, envMap map[str } slog.Info("Running Mocha tests", "command", command, "args", baseArgs, "testFiles", testFiles) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, m.platformEnv) - maps.Copy(mergedEnv, envMap) - adapterPath, adapterEnv, err := prepareMochaAdapter(mergedEnv, request) + adapterPath, adapterEnv, err := prepareMochaAdapter(request) if err != nil { return err } defer func() { _ = os.Remove(adapterPath) }() + maps.Copy(adapterEnv, envMap) + adapterEnv[mochaRequestEnvVar] = string(request) + adapterEnv, err = m.platform.RunEnv(RuntimeOptions{Env: adapterEnv, PreloadFiles: []string{adapterPath}}) + if err != nil { + return err + } + return m.executor.Run(ctx, command, baseArgs, adapterEnv) } -func prepareMochaAdapter(baseEnv map[string]string, request []byte) (string, map[string]string, error) { +func prepareMochaAdapter(request []byte) (string, map[string]string, error) { adapterFile, err := os.CreateTemp("", "ddtest-mocha-adapter-*.js") if err != nil { return "", nil, fmt.Errorf("failed to create Mocha adapter: %w", err) @@ -160,32 +167,10 @@ func prepareMochaAdapter(baseEnv map[string]string, request []byte) (string, map return "", nil, fmt.Errorf("failed to close Mocha adapter: %w", err) } - adapterEnv := make(map[string]string, len(baseEnv)+2) - maps.Copy(adapterEnv, baseEnv) - nodeOptions, ok := adapterEnv[nodeOptionsEnvVar] - if !ok { - nodeOptions = os.Getenv(nodeOptionsEnvVar) - } - adapterEnv[nodeOptionsEnvVar] = strings.TrimSpace(nodeOptions + " --require " + strconv.Quote(adapterPath)) - adapterEnv[mochaRequestEnvVar] = string(request) + adapterEnv := map[string]string{mochaRequestEnvVar: string(request)} return adapterPath, adapterEnv, nil } -func (m *Mocha) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(m.platformEnv)+1) - maps.Copy(envMap, m.platformEnv) - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - var found bool - nodeOptions, found = os.LookupEnv(nodeOptionsEnvVar) - if !found { - return envMap - } - } - envMap[nodeOptionsEnvVar] = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - return envMap -} - func (m *Mocha) Command() (string, []string) { if len(m.commandOverride) > 0 { return m.commandOverride[0], m.commandOverride[1:] diff --git a/internal/framework/mocha_test.go b/internal/framework/mocha_test.go index ad17422c..07b1040b 100644 --- a/internal/framework/mocha_test.go +++ b/internal/framework/mocha_test.go @@ -42,7 +42,7 @@ func (m *mochaCommandExecutor) capture(name string, args []string, envMap map[st } func TestMochaBasics(t *testing.T) { - mocha := NewMocha() + mocha := NewMocha(&testPlatform{}) if mocha.Name() != "mocha" { t.Fatalf("Name() = %q, want mocha", mocha.Name()) } @@ -109,7 +109,7 @@ func TestMochaDiscoverTestFiles(t *testing.T) { mocha := &Mocha{ executor: executor, commandOverride: []string{"pnpm", "exec", "mocha", "--parallel"}, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init --max-old-space-size=4096", "CUSTOM": "value"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "--max-old-space-size=4096", "CUSTOM": "value"}}, } files, err := mocha.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: mocha.TestPattern()}) if err != nil { @@ -146,7 +146,7 @@ func TestMochaDiscoverTestFilesPassesCustomLocation(t *testing.T) { mocha := &Mocha{ executor: executor, commandOverride: []string{"mocha"}, - platformEnv: make(map[string]string), + platform: &testPlatform{env: make(map[string]string)}, } files, err := mocha.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: mocha.TestPattern()}) @@ -172,7 +172,7 @@ func TestMochaRunTests(t *testing.T) { mocha := &Mocha{ executor: executor, commandOverride: []string{"npx", "mocha", "--parallel"}, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "BASE": "base"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init", "BASE": "base"}}, } files := []string{"test/a.spec.js"} if err := mocha.RunTests(context.Background(), files, map[string]string{"WORKER": "1"}); err != nil { @@ -209,7 +209,7 @@ func TestMochaUnskippableMarker(t *testing.T) { if err := os.WriteFile(file, []byte("// @datadog unskippable\n"), 0644); err != nil { t.Fatal(err) } - if !NewMocha().HasUnskippableMarker(file) { + if !NewMocha(&testPlatform{}).HasUnskippableMarker(file) { t.Fatal("expected marker") } } @@ -234,7 +234,7 @@ func TestMochaDiscoveryFailureIncludesOutput(t *testing.T) { mocha := &Mocha{ executor: &mochaCommandExecutor{output: []byte("bad config"), combinedErr: errors.New("exit 1")}, commandOverride: []string{"mocha"}, - platformEnv: make(map[string]string), + platform: &testPlatform{env: make(map[string]string)}, } _, err := mocha.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: mocha.TestPattern()}) if err == nil || !strings.Contains(err.Error(), "bad config") { diff --git a/internal/framework/platform_environment.go b/internal/framework/platform_environment.go new file mode 100644 index 00000000..13195f2a --- /dev/null +++ b/internal/framework/platform_environment.go @@ -0,0 +1,32 @@ +package framework + +import "context" + +// PlatformEnvironment prepares runtime environments and checks platform prerequisites for frameworks. +// Implementations live in package platform; this consumer interface avoids an import cycle. +type PlatformEnvironment interface { + Name() string + SanityCheck(context.Context) error + // RunEnv prepares execution; the command layer calls SanityCheck before running. + RunEnv(RuntimeOptions) (map[string]string, error) + // DiscoveryEnv validates any tracer prerequisite before preparing discovery. + DiscoveryEnv(context.Context, DiscoveryKind, RuntimeOptions) (map[string]string, error) +} + +// RuntimeOptions describes the framework's environment and additional startup files. +// Env overrides inherited values, including explicitly empty values. Methods return +// a fresh map and never modify Env. PreloadFiles are appended after project loaders. +type RuntimeOptions struct { + // ESM requests ESM instrumentation for execution and its removal for discovery. + ESM bool + Env map[string]string + PreloadFiles []string +} + +// DiscoveryKind distinguishes file discovery from full test discovery. +type DiscoveryKind uint8 + +const ( + FileDiscovery DiscoveryKind = iota + FullDiscovery +) diff --git a/internal/framework/platform_environment_test.go b/internal/framework/platform_environment_test.go new file mode 100644 index 00000000..a0b11a51 --- /dev/null +++ b/internal/framework/platform_environment_test.go @@ -0,0 +1,135 @@ +package framework + +import ( + "context" + "errors" + "maps" + "strconv" + "strings" + "testing" + + "github.com/DataDog/ddtest/internal/discovery" + "github.com/stretchr/testify/require" +) + +// testPlatform supplies prepared environments; runtime policy is tested in platform. +type testPlatform struct { + env map[string]string + discoveryErr error + runErr error + runOptions RuntimeOptions + discoveryOptions RuntimeOptions +} + +func (p *testPlatform) Name() string { return "test" } +func (p *testPlatform) SanityCheck(context.Context) error { return nil } +func (p *testPlatform) RunEnv(options RuntimeOptions) (map[string]string, error) { + p.runOptions = options + env := make(map[string]string) + maps.Copy(env, p.env) + maps.Copy(env, options.Env) + // Emulate a platform accepting the framework's startup-file request. + for _, file := range options.PreloadFiles { + env["NODE_OPTIONS"] = strings.TrimSpace(env["NODE_OPTIONS"] + " --require " + strconv.Quote(file)) + } + return env, p.runErr +} +func (p *testPlatform) DiscoveryEnv(_ context.Context, _ DiscoveryKind, options RuntimeOptions) (map[string]string, error) { + env, _ := p.RunEnv(options) + p.discoveryOptions = options + return env, p.discoveryErr +} + +const ( + nodeOptionsEnvVar = "NODE_OPTIONS" + ddTraceCIInitModule = "dd-trace/ci/init" +) + +// platformBoundaryExecutor records whether an environment error leaked into a command. +type platformBoundaryExecutor struct{ runs, probes int } + +func (e *platformBoundaryExecutor) Run(context.Context, string, []string, map[string]string) error { + e.runs++ + return nil +} +func (e *platformBoundaryExecutor) CombinedOutput(context.Context, string, []string, map[string]string) ([]byte, error) { + e.probes++ + return nil, errors.New("not a Rails application") +} +func (e *platformBoundaryExecutor) Output(ctx context.Context, name string, args []string, env map[string]string) ([]byte, []byte, error) { + out, err := e.CombinedOutput(ctx, name, args, env) + return out, nil, err +} + +func TestFrameworksPropagatePlatformErrors(t *testing.T) { + constructors := []func(PlatformEnvironment, *platformBoundaryExecutor) Framework{ + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewRSpec(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewMinitest(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewPytest(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewJest(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewMocha(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewVitest(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewPlaywright(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewCypress(p) + f.executor = e + return f + }, + func(p PlatformEnvironment, e *platformBoundaryExecutor) Framework { + f := NewCucumber(p) + f.executor = e + return f + }, + } + for _, newFramework := range constructors { + failure := errors.New("platform could not prepare the environment") + p := &testPlatform{discoveryErr: failure, runErr: failure} + e := &platformBoundaryExecutor{} + fw := newFramework(p, e) + t.Run(fw.Name(), func(t *testing.T) { + t.Chdir(t.TempDir()) + require.Same(t, p, fw.Platform()) + files := discovery.TestFileSet{Pattern: "**/*"} + var err error + if fw.SupportsFullTestDiscovery() { + _, err = fw.DiscoverTests(t.Context(), files) + } else { + _, err = fw.DiscoverTestFiles(t.Context(), files) + } + require.ErrorIs(t, err, failure) + require.Equal(t, fw.Name() == "vitest", p.discoveryOptions.ESM) + require.Zero(t, e.probes, "discovery must stop before executing commands") + require.ErrorIs(t, fw.RunTests(t.Context(), []string{"example_test.rb"}, nil), failure) + require.Equal(t, fw.Name() == "vitest", p.runOptions.ESM) + require.Zero(t, e.runs, "test execution must stop when environment preparation fails") + }) + } +} diff --git a/internal/framework/playwright.go b/internal/framework/playwright.go index a24df71c..24025b84 100644 --- a/internal/framework/playwright.go +++ b/internal/framework/playwright.go @@ -7,7 +7,6 @@ import ( "errors" "fmt" "log/slog" - "maps" "os" "path/filepath" "regexp" @@ -35,7 +34,7 @@ var playwrightDiscoveryReporterScript string type Playwright struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment discoveryRoot string } @@ -48,18 +47,18 @@ type playwrightDiscoveryError struct { Message string `json:"message"` } -func NewPlaywright() *Playwright { +func NewPlaywright(p PlatformEnvironment) *Playwright { return &Playwright{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (p *Playwright) SetPlatformEnv(platformEnv map[string]string) { p.platformEnv = platformEnv } -func (p *Playwright) GetPlatformEnv() map[string]string { return p.platformEnv } -func (p *Playwright) Name() string { return "playwright" } -func (p *Playwright) SupportsFullTestDiscovery() bool { return false } +func (p *Playwright) Platform() PlatformEnvironment { return p.platform } + +func (p *Playwright) Name() string { return "playwright" } +func (p *Playwright) SupportsFullTestDiscovery() bool { return false } func (p *Playwright) SourceFileForSuite(suite string) (string, bool) { suite = strings.TrimSpace(suite) @@ -108,6 +107,11 @@ func (p *Playwright) DiscoverTests(context.Context, discovery.TestFileSet) ([]te } func (p *Playwright) DiscoverTestFiles(ctx context.Context, selectedFiles discovery.TestFileSet) ([]string, error) { + envMap, err := p.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err + } + command, baseArgs := p.Command() if _, err := playwrightCLIArgs(command, baseArgs); err != nil { return nil, err @@ -120,7 +124,7 @@ func (p *Playwright) DiscoverTestFiles(ctx context.Context, selectedFiles discov args := playwrightDiscoveryArgs(command, baseArgs, reporterPath) slog.Info("Discovering Playwright test files with command", "command", command, "args", args) - output, commandErr := p.executor.CombinedOutput(ctx, command, args, p.discoveryEnv()) + output, commandErr := p.executor.CombinedOutput(ctx, command, args, envMap) discoveryResult, parseErr := parsePlaywrightDiscoveryOutput(output) if commandErr != nil && (!isPlaywrightNoTestsExit(output, commandErr) || len(discoveryResult.Files) > 0) { message := strings.TrimSpace(string(output)) @@ -159,25 +163,11 @@ func (p *Playwright) RunTests(ctx context.Context, testFiles []string, envMap ma } args := playwrightRunArgs(command, baseArgs, testFiles) slog.Info("Running Playwright tests", "command", command, "args", args, "testFiles", testFiles) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, p.platformEnv) - maps.Copy(mergedEnv, envMap) - return p.executor.Run(ctx, command, args, mergedEnv) -} - -func (p *Playwright) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(p.platformEnv)+1) - maps.Copy(envMap, p.platformEnv) - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - var found bool - nodeOptions, found = os.LookupEnv(nodeOptionsEnvVar) - if !found { - return envMap - } + mergedEnv, err := p.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err } - envMap[nodeOptionsEnvVar] = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - return envMap + return p.executor.Run(ctx, command, args, mergedEnv) } func (p *Playwright) Command() (string, []string) { diff --git a/internal/framework/playwright_test.go b/internal/framework/playwright_test.go index dfd8e809..6ae6c50a 100644 --- a/internal/framework/playwright_test.go +++ b/internal/framework/playwright_test.go @@ -51,7 +51,7 @@ func (e *playwrightCommandExecutor) capture(name string, args []string, env map[ } func TestPlaywrightFrameworkMetadata(t *testing.T) { - playwright := NewPlaywright() + playwright := NewPlaywright(&testPlatform{}) if playwright.Name() != "playwright" || playwright.SupportsFullTestDiscovery() { t.Fatalf("unexpected metadata: %q, full=%v", playwright.Name(), playwright.SupportsFullTestDiscovery()) } @@ -193,7 +193,7 @@ func TestPlaywrightDiscoverTestFilesUsesNativeListAndNormalizes(t *testing.T) { commandOverride: []string{ "pnpm", "exec", "playwright", "test", "--config", "apps/web/playwright.config.ts", "--project", "chromium", "--reporter", "html", }, - platformEnv: map[string]string{"NODE_OPTIONS": "-r dd-trace/ci/init --max-old-space-size=2048", "CUSTOM": "value"}, + platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "--max-old-space-size=2048", "CUSTOM": "value"}}, } discovered, err := playwright.DiscoverTestFiles(context.Background(), discovery.TestFileSet{}) if err != nil { @@ -230,7 +230,7 @@ func TestPlaywrightDiscoveryAcceptsOnlyTheNoTestsExit(t *testing.T) { playwrightErrorMarker + `{"message":"Error: No tests found"}`), err: playwrightCommandExitError{code: 1}, } - playwright := &Playwright{executor: executor, commandOverride: []string{"playwright", "test"}, platformEnv: map[string]string{}} + playwright := &Playwright{executor: executor, commandOverride: []string{"playwright", "test"}, platform: &testPlatform{env: map[string]string{}}} files, err := playwright.DiscoverTestFiles(context.Background(), discovery.TestFileSet{}) if err != nil || len(files) != 0 { t.Fatalf("empty discovery = %v, %v", files, err) @@ -301,7 +301,7 @@ func TestPlaywrightRunTestsMergesEnvironmentAndSkipsEmptyAssignments(t *testing. playwright := &Playwright{ executor: executor, commandOverride: []string{"playwright", "test", "old.spec.ts", "--shard", "1/3"}, - platformEnv: map[string]string{"SHARED": "platform", "PLATFORM": "yes"}, + platform: &testPlatform{env: map[string]string{"SHARED": "platform", "PLATFORM": "yes"}}, } if err := playwright.RunTests(context.Background(), nil, nil); err != nil || executor.runCalls != 0 { t.Fatalf("empty RunTests() = %v, calls = %d", err, executor.runCalls) @@ -326,7 +326,7 @@ func TestPlaywrightSourceFileForSuiteUsesConfigDirectory(t *testing.T) { if err := os.WriteFile("apps/web/playwright.config.ts", []byte("export default {}\n"), 0644); err != nil { t.Fatal(err) } - playwright := &Playwright{commandOverride: []string{"playwright", "test", "--config", "apps/web/playwright.config.ts"}} + playwright := &Playwright{platform: &testPlatform{}, commandOverride: []string{"playwright", "test", "--config", "apps/web/playwright.config.ts"}} if source, ok := playwright.SourceFileForSuite("tests/a.spec.ts"); !ok || source != "apps/web/tests/a.spec.ts" { t.Fatalf("SourceFileForSuite() = %q, %v", source, ok) } diff --git a/internal/framework/pytest.go b/internal/framework/pytest.go index 556b72b0..b877160f 100644 --- a/internal/framework/pytest.go +++ b/internal/framework/pytest.go @@ -3,7 +3,6 @@ package framework import ( "context" "log/slog" - "maps" "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/ext" @@ -21,24 +20,18 @@ const ( type PyTest struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } -func NewPytest() *PyTest { +func NewPytest(p PlatformEnvironment) *PyTest { return &PyTest{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (p *PyTest) SetPlatformEnv(platformEnv map[string]string) { - p.platformEnv = platformEnv -} - -func (p *PyTest) GetPlatformEnv() map[string]string { - return p.platformEnv -} +func (p *PyTest) Platform() PlatformEnvironment { return p.platform } func (p *PyTest) Name() string { return "pytest" @@ -74,6 +67,11 @@ func (p *PyTest) DiscoverTests(ctx context.Context, testFiles discovery.TestFile return []testoptimization.Test{}, nil } + envMap, err := p.platform.DiscoveryEnv(ctx, FullDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err + } + command, args := p.Command() if testFiles.UseExplicitFiles() { @@ -91,7 +89,7 @@ func (p *PyTest) DiscoverTests(ctx context.Context, testFiles discovery.TestFile args = withFrameworkFiles(command, args, "pytest", files) } - return discovery.DiscoverTests(ctx, p.executor, command, args, p.platformEnv) + return discovery.DiscoverTests(ctx, p.executor, command, args, envMap) } func (p *PyTest) DiscoverTestFiles(ctx context.Context, testFiles discovery.TestFileSet) ([]string, error) { @@ -121,9 +119,10 @@ func (p *PyTest) RunTests(ctx context.Context, testFiles []string, envMap map[st slog.Info("Running tests with command", "command", command, "args", args) args = withFrameworkFiles(command, args, "pytest", testFiles) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, p.platformEnv) - maps.Copy(mergedEnv, envMap) + mergedEnv, err := p.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err + } return p.executor.Run(ctx, command, args, mergedEnv) } diff --git a/internal/framework/pytest_config_test.go b/internal/framework/pytest_config_test.go index efc5f460..efc5b371 100644 --- a/internal/framework/pytest_config_test.go +++ b/internal/framework/pytest_config_test.go @@ -154,7 +154,7 @@ func TestParsePyprojectToml_InvalidToml(t *testing.T) { } func TestPyTest_testPattern_DefaultWhenNoConfig(t *testing.T) { - pytest := &PyTest{platformEnv: map[string]string{}} + pytest := &PyTest{platform: &testPlatform{env: map[string]string{}}} if got := pytest.TestPattern(); got != pytestDefaultPattern { t.Errorf("expected default pattern %q, got %q", pytestDefaultPattern, got) } @@ -162,7 +162,7 @@ func TestPyTest_testPattern_DefaultWhenNoConfig(t *testing.T) { func TestPyTest_testPattern_ExplicitTestsLocationOverridesConfig(t *testing.T) { setTestsLocation(t, "mydir/**/*_test.py") - pytest := &PyTest{platformEnv: map[string]string{}} + pytest := &PyTest{platform: &testPlatform{env: map[string]string{}}} if got := pytest.TestPattern(); got != "mydir/**/*_test.py" { t.Errorf("expected explicit location %q, got %q", "mydir/**/*_test.py", got) } @@ -174,7 +174,7 @@ func TestPyTest_testPattern_MultipleTestpaths(t *testing.T) { if err := os.WriteFile("pytest.ini", []byte("[pytest]\ntestpaths = tests src\n"), 0644); err != nil { t.Fatal(err) } - pytest := &PyTest{platformEnv: map[string]string{}} + pytest := &PyTest{platform: &testPlatform{env: map[string]string{}}} if got, want := pytest.TestPattern(), "{tests,src}/**/{test_*,*_test}.py"; got != want { t.Errorf("TestPattern() = %q, want %q", got, want) } @@ -186,7 +186,7 @@ func TestPyTest_testPattern_MultipleFilePatterns(t *testing.T) { if err := os.WriteFile("pytest.ini", []byte("[pytest]\npython_files = test_*.py *_test.py check_*.py\n"), 0644); err != nil { t.Fatal(err) } - pytest := &PyTest{platformEnv: map[string]string{}} + pytest := &PyTest{platform: &testPlatform{env: map[string]string{}}} if got, want := pytest.TestPattern(), "**/{test_*.py,*_test.py,check_*.py}"; got != want { t.Errorf("TestPattern() = %q, want %q", got, want) } diff --git a/internal/framework/pytest_test.go b/internal/framework/pytest_test.go index 18679039..2ed02121 100644 --- a/internal/framework/pytest_test.go +++ b/internal/framework/pytest_test.go @@ -28,7 +28,7 @@ func TestPyTest_DiscoverTests_WithExplicitFiles(t *testing.T) { }, } - pytest := &PyTest{executor: mockExecutor, platformEnv: map[string]string{}} + pytest := &PyTest{executor: mockExecutor, platform: &testPlatform{env: map[string]string{}}} testFiles := discovery.TestFileSet{ExplicitFiles: explicitFiles} _, _ = pytest.DiscoverTests(context.Background(), testFiles) @@ -70,7 +70,7 @@ func TestPyTest_DiscoverTests_WithPatternGlobsFiles(t *testing.T) { }, } - pytest := &PyTest{executor: mockExecutor, platformEnv: map[string]string{}} + pytest := &PyTest{executor: mockExecutor, platform: &testPlatform{env: map[string]string{}}} // Pattern-based (no explicit files): pytest.go will glob the pattern pattern := filepath.Join(tmpDir, "test_*.py") testFiles := discovery.TestFileSet{Pattern: pattern} @@ -96,7 +96,7 @@ func TestPyTest_DiscoverTests_EmptyFileSet(t *testing.T) { }, } - pytest := &PyTest{executor: mockExecutor, platformEnv: map[string]string{}} + pytest := &PyTest{executor: mockExecutor, platform: &testPlatform{env: map[string]string{}}} tests, err := pytest.DiscoverTests(context.Background(), discovery.TestFileSet{ExplicitFiles: []string{}}) if err != nil { @@ -150,7 +150,7 @@ func TestPyTest_DiscoverTests_Success(t *testing.T) { }, } - pytest := &PyTest{executor: mockExecutor, platformEnv: map[string]string{}} + pytest := &PyTest{executor: mockExecutor, platform: &testPlatform{env: map[string]string{}}} testFiles := discovery.TestFileSet{ExplicitFiles: []string{"tests/test_user.py", "tests/test_auth.py"}} tests, err := pytest.DiscoverTests(context.Background(), testFiles) if err != nil { @@ -178,8 +178,8 @@ func TestPyTest_DiscoverTests_Success(t *testing.T) { } } -func TestPyTest_MetadataAndPlatformEnv(t *testing.T) { - pytest := NewPytest() +func TestPyTest_Metadata(t *testing.T) { + pytest := NewPytest(&testPlatform{}) if pytest.Name() != "pytest" { t.Errorf("expected framework name pytest, got %q", pytest.Name()) } @@ -187,15 +187,10 @@ func TestPyTest_MetadataAndPlatformEnv(t *testing.T) { t.Error("expected PyTest to support full test discovery") } - platformEnv := map[string]string{"PYTEST_ADDOPTS": "--ddtrace"} - pytest.SetPlatformEnv(platformEnv) - if got := pytest.GetPlatformEnv(); got["PYTEST_ADDOPTS"] != platformEnv["PYTEST_ADDOPTS"] { - t.Errorf("expected platform env to be retained, got %v", got) - } } func TestPyTest_SupportsFullTestDiscovery(t *testing.T) { - pytest := NewPytest() + pytest := NewPytest(&testPlatform{}) if !pytest.SupportsFullTestDiscovery() { t.Error("expected PyTest to support full test discovery") } @@ -219,7 +214,7 @@ func TestPyTest_RunTests_WithCommandOverride(t *testing.T) { pytest := &PyTest{ executor: mockExecutor, - platformEnv: map[string]string{}, + platform: &testPlatform{env: map[string]string{}}, commandOverride: []string{"pytest"}, } if err := pytest.RunTests(context.Background(), testFiles, nil); err != nil { @@ -255,7 +250,7 @@ func TestPyTest_DiscoverTests_WithCommandOverride(t *testing.T) { pytest := &PyTest{ executor: mockExecutor, - platformEnv: map[string]string{}, + platform: &testPlatform{env: map[string]string{}}, commandOverride: []string{"pytest"}, } testFiles := discovery.TestFileSet{ExplicitFiles: explicitFiles} @@ -294,10 +289,10 @@ func TestPyTest_RunTests(t *testing.T) { pytest := &PyTest{ executor: mockExecutor, - platformEnv: map[string]string{ + platform: &testPlatform{env: map[string]string{ "PYTEST_ADDOPTS": "--ddtrace", "SHARED_VAR": "platform", - }, + }}, } if err := pytest.RunTests(context.Background(), testFiles, envMap); err != nil { t.Fatalf("RunTests failed: %v", err) diff --git a/internal/framework/rspec.go b/internal/framework/rspec.go index d33b7b4a..e11968d6 100644 --- a/internal/framework/rspec.go +++ b/internal/framework/rspec.go @@ -2,9 +2,7 @@ package framework import ( "context" - "fmt" "log/slog" - "maps" "os" "path/filepath" "strings" @@ -27,24 +25,18 @@ const ( type RSpec struct { executor ext.CommandExecutor commandOverride []string - platformEnv map[string]string + platform PlatformEnvironment } -func NewRSpec() *RSpec { +func NewRSpec(p PlatformEnvironment) *RSpec { return &RSpec{ executor: &ext.DefaultCommandExecutor{}, commandOverride: loadCommandOverride(), - platformEnv: make(map[string]string), + platform: p, } } -func (r *RSpec) SetPlatformEnv(platformEnv map[string]string) { - r.platformEnv = platformEnv -} - -func (r *RSpec) GetPlatformEnv() map[string]string { - return r.platformEnv -} +func (r *RSpec) Platform() PlatformEnvironment { return r.platform } func (r *RSpec) Name() string { return "rspec" @@ -56,8 +48,10 @@ func (r *RSpec) DiscoverTests(ctx context.Context, testFiles discovery.TestFileS if testFiles.Empty() { return []testoptimization.Test{}, nil } - if err := utils.CheckRubyTracer(ctx, r.executor); err != nil { - return nil, fmt.Errorf("full test discovery requires datadog-ci: %w", err) + + envMap, err := r.platform.DiscoveryEnv(ctx, FullDiscovery, RuntimeOptions{}) + if err != nil { + return nil, err } executable, baseArgs := r.Command() @@ -69,7 +63,7 @@ func (r *RSpec) DiscoverTests(ctx context.Context, testFiles discovery.TestFileS args = withFrameworkOptions(executable, args, "rspec", "--pattern", testFiles.Pattern) } - return discovery.DiscoverTests(ctx, r.executor, executable, args, r.platformEnv) + return discovery.DiscoverTests(ctx, r.executor, executable, args, envMap) } func (r *RSpec) TestPattern() string { @@ -95,9 +89,10 @@ func (r *RSpec) RunTests(ctx context.Context, testFiles []string, envMap map[str slog.Info("Running tests with command", "command", command, "args", args) args = withFrameworkFiles(command, args, "rspec", testFiles) - mergedEnv := make(map[string]string) - maps.Copy(mergedEnv, r.GetPlatformEnv()) - maps.Copy(mergedEnv, envMap) + mergedEnv, err := r.platform.RunEnv(RuntimeOptions{Env: envMap}) + if err != nil { + return err + } return r.executor.Run(ctx, command, args, mergedEnv) } diff --git a/internal/framework/rspec_test.go b/internal/framework/rspec_test.go index 17ecb012..e498963a 100644 --- a/internal/framework/rspec_test.go +++ b/internal/framework/rspec_test.go @@ -89,9 +89,6 @@ type mockCommandExecutor struct { } func (m *mockCommandExecutor) CombinedOutput(ctx context.Context, name string, args []string, envMap map[string]string) ([]byte, error) { - if name == "bundle" && slices.Equal(args, []string{"info", "datadog-ci"}) { - return []byte(" * datadog-ci (1.31.0)"), nil - } if m.onExecution != nil { m.onExecution(name, args) } @@ -108,13 +105,13 @@ func (m *mockCommandExecutor) Run(ctx context.Context, name string, args []strin } func newTestRSpecWithExecutor(executor ext.CommandExecutor) *RSpec { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) rspec.executor = executor return rspec } func newTestRSpecWithOverride(commandOverride []string) *RSpec { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) rspec.commandOverride = commandOverride return rspec } @@ -126,7 +123,7 @@ func newTestRSpecWithExecutorAndOverride(executor ext.CommandExecutor, commandOv } func newTestMinitestWithExecutor(executor ext.CommandExecutor) *Minitest { - minitest := NewMinitest() + minitest := NewMinitest(&testPlatform{}) minitest.executor = executor return minitest } @@ -138,24 +135,24 @@ func newTestMinitestWithExecutorAndOverride(executor ext.CommandExecutor, comman } func TestNewRSpec(t *testing.T) { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) if rspec == nil { - t.Error("NewRSpec() returned nil") + t.Error("NewRSpec(&testPlatform{}) returned nil") return } if rspec.executor == nil { - t.Error("NewRSpec() created RSpec with nil executor") + t.Error("NewRSpec(&testPlatform{}) created RSpec with nil executor") return } // Verify it's using the default executor if _, ok := rspec.executor.(*ext.DefaultCommandExecutor); !ok { - t.Error("NewRSpec() should use DefaultCommandExecutor") + t.Error("NewRSpec(&testPlatform{}) should use DefaultCommandExecutor") } } func TestRSpec_Name(t *testing.T) { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) expected := "rspec" actual := rspec.Name() @@ -178,7 +175,7 @@ func TestRSpec_getRSpecCommand_WithBinRSpec(t *testing.T) { t.Fatalf("failed to create bin/rspec: %v", err) } - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) command, baseArgs := rspec.Command() if command != "bin/rspec" { @@ -203,7 +200,7 @@ func TestRSpec_getRSpecCommand_WithNonExecutableBinRSpec(t *testing.T) { t.Fatalf("failed to create bin/rspec: %v", err) } - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) command, baseArgs := rspec.Command() if command != "bundle" { @@ -224,7 +221,7 @@ func TestRSpec_getRSpecCommand_WithoutBinRSpec(t *testing.T) { // Ensure bin/rspec doesn't exist _ = os.RemoveAll("bin") - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) command, baseArgs := rspec.Command() if command != "bundle" { @@ -676,7 +673,7 @@ func TestRSpec_DiscoverTestFiles(t *testing.T) { } } - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) discoveredFiles, err := discovery.DiscoverTestFiles(rspec.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { @@ -750,7 +747,7 @@ func TestRSpec_DiscoverTestFiles_WithTestsLocation(t *testing.T) { setTestsLocation(t, filepath.Join("custom", "spec", "**", "*_spec.rb")) - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) files, err := discovery.DiscoverTestFiles(rspec.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -804,7 +801,7 @@ func TestRSpec_DiscoverTestFiles_WithTestsExcludePattern(t *testing.T) { setTestsExcludePattern(t, filepath.Join("spec", "system", "**", "*_spec.rb")) - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) files, err := discovery.DiscoverTestFiles(rspec.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { t.Fatalf("DiscoverTestFiles failed: %v", err) @@ -1130,7 +1127,7 @@ func TestRSpec_DiscoverTestFiles_NoSpecDirectory(t *testing.T) { _ = os.Chdir(originalDir) }() - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) discoveredFiles, err := discovery.DiscoverTestFiles(rspec.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { @@ -1143,19 +1140,6 @@ func TestRSpec_DiscoverTestFiles_NoSpecDirectory(t *testing.T) { } } -func TestRSpec_SetPlatformEnv(t *testing.T) { - rspec := NewRSpec() - - platformEnv := map[string]string{ - "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", - } - rspec.SetPlatformEnv(platformEnv) - - if rspec.GetPlatformEnv()["RUBYOPT"] != platformEnv["RUBYOPT"] { - t.Errorf("expected platformEnv to be set, got %v", rspec.GetPlatformEnv()) - } -} - func TestRSpec_RunTests_UsesPlatformEnv(t *testing.T) { _ = os.RemoveAll("bin") @@ -1171,7 +1155,7 @@ func TestRSpec_RunTests_UsesPlatformEnv(t *testing.T) { platformEnv := map[string]string{ "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", } - rspec.SetPlatformEnv(platformEnv) + rspec.platform = &testPlatform{env: platformEnv} err := rspec.RunTests(context.Background(), testFiles, nil) if err != nil { @@ -1200,7 +1184,7 @@ func TestRSpec_RunTests_MergesPlatformEnvWithPassedEnv(t *testing.T) { "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", "PLATFORM_VAR": "platform_value", } - rspec.SetPlatformEnv(platformEnv) + rspec.platform = &testPlatform{env: platformEnv} // Pass additional env vars additionalEnv := map[string]string{ @@ -1247,7 +1231,7 @@ func TestRSpec_RunTests_AdditionalEnvOverridesPlatformEnv(t *testing.T) { "SHARED_VAR": "platform_value", "ANOTHER_VAR": "platform_another", } - rspec.SetPlatformEnv(platformEnv) + rspec.platform = &testPlatform{env: platformEnv} // Pass additional env that overrides SHARED_VAR additionalEnv := map[string]string{ @@ -1283,9 +1267,6 @@ type mockCommandExecutorWithEnvCapture struct { } func (m *mockCommandExecutorWithEnvCapture) CombinedOutput(ctx context.Context, name string, args []string, envMap map[string]string) ([]byte, error) { - if name == "bundle" && slices.Equal(args, []string{"info", "datadog-ci"}) { - return []byte(" * datadog-ci (1.31.0)"), nil - } m.combinedOutputEnvMap = envMap if m.onExecution != nil { m.onExecution(name, args) @@ -1344,7 +1325,7 @@ func TestRSpec_DiscoverTests_UsesPlatformEnv(t *testing.T) { platformEnv := map[string]string{ "RUBYOPT": "-rbundler/setup -rdatadog/ci/auto_instrument", } - rspec.SetPlatformEnv(platformEnv) + rspec.platform = &testPlatform{env: platformEnv} _, err := discoverAndParseTests(t, rspec, resolveTestFilesForFramework(t, rspec.TestPattern())) if err != nil { @@ -1378,7 +1359,7 @@ func TestRSpec_DefaultTestPattern(t *testing.T) { settings.Init() t.Cleanup(settings.Init) - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) expected := filepath.Join(rspecRootDir, "**", rspecTestFilePattern) if got := rspec.TestPattern(); got != expected { t.Errorf("expected default test pattern %q, got %q", expected, got) @@ -1386,14 +1367,14 @@ func TestRSpec_DefaultTestPattern(t *testing.T) { } func TestRSpec_SupportsFullTestDiscovery(t *testing.T) { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) if !rspec.SupportsFullTestDiscovery() { t.Error("expected RSpec to support full test discovery") } } func TestRSpec_SourceFileForSuite(t *testing.T) { - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) sourceFile, ok := rspec.SourceFileForSuite("User model at spec/models/user_spec.rb") if !ok || sourceFile != "spec/models/user_spec.rb" { @@ -1421,7 +1402,7 @@ func TestRSpec_HasUnskippableMarker(t *testing.T) { t.Fatal(err) } - rspec := NewRSpec() + rspec := NewRSpec(&testPlatform{}) if !rspec.HasUnskippableMarker(markedFile) { t.Fatal("expected Ruby unskippable marker") } diff --git a/internal/framework/vitest.go b/internal/framework/vitest.go index 08399341..6c14112e 100644 --- a/internal/framework/vitest.go +++ b/internal/framework/vitest.go @@ -6,7 +6,6 @@ import ( "encoding/json" "fmt" "log/slog" - "maps" "os" "path/filepath" "slices" @@ -19,8 +18,6 @@ import ( "github.com/DataDog/ddtest/internal/utils" ) -const ddTraceRegisterPath = "dd-trace/register.js" - //go:embed scripts/vitest.mjs var vitestScript string @@ -36,25 +33,19 @@ type Vitest struct { executor ext.CommandExecutor configFile string customCommand string - platformEnv map[string]string + platform PlatformEnvironment } -func NewVitest() *Vitest { +func NewVitest(p PlatformEnvironment) *Vitest { return &Vitest{ executor: &ext.DefaultCommandExecutor{}, configFile: settings.GetVitestConfig(), customCommand: settings.GetCommand(), - platformEnv: make(map[string]string), + platform: p, } } -func (v *Vitest) SetPlatformEnv(platformEnv map[string]string) { - v.platformEnv = platformEnv -} - -func (v *Vitest) GetPlatformEnv() map[string]string { - return v.platformEnv -} +func (v *Vitest) Platform() PlatformEnvironment { return v.platform } func (v *Vitest) Name() string { return "vitest" @@ -106,12 +97,16 @@ func (v *Vitest) DiscoverTestFiles(ctx context.Context, testFiles discovery.Test } } + env, err := v.platform.DiscoveryEnv(ctx, FileDiscovery, RuntimeOptions{ESM: true}) + if err != nil { + return nil, err + } dir, err := prepareVitestAdapter(vitestRequest{Config: v.configFile, Discover: true}) if err != nil { return nil, err } defer func() { _ = os.RemoveAll(dir) }() - output, err := v.executor.CombinedOutput(ctx, "node", []string{filepath.Join(dir, "vitest.mjs")}, v.discoveryEnv()) + output, err := v.executor.CombinedOutput(ctx, "node", []string{filepath.Join(dir, "vitest.mjs")}, env) if err != nil { return nil, fmt.Errorf("failed to discover Vitest test files: %s: %w", strings.TrimSpace(string(output)), err) } @@ -147,9 +142,10 @@ func (v *Vitest) RunTests(ctx context.Context, testFiles []string, envMap map[st } defer func() { _ = os.RemoveAll(dir) }() - env := make(map[string]string) - maps.Copy(env, v.platformEnv) - maps.Copy(env, envMap) + env, err := v.platform.RunEnv(RuntimeOptions{ESM: true, Env: envMap}) + if err != nil { + return err + } slog.Info("Running assigned Vitest files with Node API", "config", v.configFile, "files", testFiles) return v.executor.Run(ctx, "node", []string{filepath.Join(dir, "vitest.mjs")}, env) } @@ -196,25 +192,3 @@ func prepareVitestAdapter(request vitestRequest) (string, error) { } return dir, nil } - -func (v *Vitest) discoveryEnv() map[string]string { - envMap := make(map[string]string, len(v.platformEnv)+1) - maps.Copy(envMap, v.platformEnv) - - nodeOptions, ok := envMap[nodeOptionsEnvVar] - if !ok { - var found bool - nodeOptions, found = os.LookupEnv(nodeOptionsEnvVar) - if !found { - return envMap - } - } - - nodeOptions = stripNodeOptionsRequire(nodeOptions, ddTraceCIInitModule) - envMap[nodeOptionsEnvVar] = stripNodeOptionsImport(nodeOptions, ddTraceRegisterPath) - return envMap -} - -func stripNodeOptionsImport(nodeOptions string, module string) string { - return utils.NodeOptionsWithoutImport(nodeOptions, module) -} diff --git a/internal/framework/vitest_test.go b/internal/framework/vitest_test.go index 4d3abb70..e07b1141 100644 --- a/internal/framework/vitest_test.go +++ b/internal/framework/vitest_test.go @@ -66,7 +66,7 @@ func (m *vitestCommandExecutor) Run(_ context.Context, name string, args []strin } func TestVitest_FrameworkMetadata(t *testing.T) { - vitest := NewVitest() + vitest := NewVitest(&testPlatform{}) if vitest.Name() != "vitest" { t.Fatalf("Name() = %q, want vitest", vitest.Name()) } @@ -93,7 +93,7 @@ func TestVitest_HasUnskippableMarker(t *testing.T) { t.Fatal(err) } - vitest := NewVitest() + vitest := NewVitest(&testPlatform{}) if !vitest.HasUnskippableMarker(markedFile) { t.Fatal("expected unskippable marker") } @@ -128,9 +128,9 @@ func TestVitest_DiscoverTestFiles_WithConfig(t *testing.T) { vitest := &Vitest{ executor: executor, configFile: "vitest.unit.ts", - platformEnv: map[string]string{ - "NODE_OPTIONS": "--import dd-trace/register.js -r dd-trace/ci/init --max-old-space-size=4096", - }, + platform: &testPlatform{env: map[string]string{ + "NODE_OPTIONS": "--max-old-space-size=4096", + }}, } files, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: vitest.TestPattern()}) @@ -157,7 +157,7 @@ func TestVitest_DiscoverTestFiles_WithConfig(t *testing.T) { func TestVitest_DiscoverTestFiles_ExplicitFiles(t *testing.T) { executor := &vitestCommandExecutor{err: errors.New("should not execute")} - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} want := []string{"src/a.test.ts"} files, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{ExplicitFiles: want}) if err != nil { @@ -190,7 +190,7 @@ func TestVitest_DiscoverTestFiles_ExcludeStillUsesVitestDiscovery(t *testing.T) "custom.check.ts", ), }, - platformEnv: make(map[string]string), + platform: &testPlatform{}, } resolvedTestFiles, err := discovery.ResolveTestFiles(vitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { @@ -230,7 +230,7 @@ func TestVitest_DiscoverTestFiles_ExcludeWithEmptyCandidatesStillUsesVitestDisco "excluded.test.ts", "custom.check.ts", )} - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} resolvedTestFiles, err := discovery.ResolveTestFiles(vitest.TestPattern(), settings.GetTestsExcludePattern()) if err != nil { t.Fatal(err) @@ -253,7 +253,7 @@ func TestVitest_DiscoverTestFiles_ExcludeWithEmptyCandidatesStillUsesVitestDisco func TestVitest_DiscoverTestFiles_ErrorIncludesOutput(t *testing.T) { executor := &vitestCommandExecutor{output: []byte("invalid Vitest config"), err: errors.New("exit status 1")} - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} _, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: vitest.TestPattern()}) if err == nil || !strings.Contains(err.Error(), "invalid Vitest config") { t.Fatalf("unexpected error: %v", err) @@ -262,7 +262,7 @@ func TestVitest_DiscoverTestFiles_ErrorIncludesOutput(t *testing.T) { func TestVitest_DiscoverTestFiles_InvalidJSON(t *testing.T) { executor := &vitestCommandExecutor{output: []byte("not JSON")} - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} _, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: vitest.TestPattern()}) if err == nil || !strings.Contains(err.Error(), "failed to parse Vitest test file list") { t.Fatalf("unexpected error: %v", err) @@ -285,7 +285,7 @@ func TestVitest_DiscoverTestFiles_IgnoresStdoutAndStderrNoise(t *testing.T) { stdout: []byte("Vitest config log\n"), stderr: []byte("Vite deprecation warning\n"), } - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} files, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: vitest.TestPattern()}) if err != nil { t.Fatal(err) @@ -316,7 +316,7 @@ func TestVitest_DiscoverTestFiles_FiltersCustomLocation(t *testing.T) { "src/b.test.ts", "custom/a.check.ts", )} - vitest := &Vitest{executor: executor, platformEnv: make(map[string]string)} + vitest := &Vitest{executor: executor, platform: &testPlatform{}} files, err := vitest.DiscoverTestFiles(context.Background(), discovery.TestFileSet{Pattern: vitest.TestPattern()}) if err != nil { t.Fatal(err) @@ -346,7 +346,7 @@ func TestVitest_ConfigFromEnvironment(t *testing.T) { t.Setenv("DD_TEST_OPTIMIZATION_RUNNER_VITEST_CONFIG", "config with spaces/vitest.ts") viper.Reset() settings.Init() - if got := NewVitest().configFile; got != "config with spaces/vitest.ts" { + if got := NewVitest(&testPlatform{}).configFile; got != "config with spaces/vitest.ts" { t.Fatalf("config = %q", got) } } @@ -382,7 +382,7 @@ func TestVitest_RunTests_ExactSelectionEnvironmentAndCleanup(t *testing.T) { } return runErr }} - vitest := &Vitest{executor: executor, configFile: "vitest.unit.ts", platformEnv: map[string]string{"NODE_OPTIONS": "platform-options", "SHARED": "platform"}} + vitest := &Vitest{executor: executor, configFile: "vitest.unit.ts", platform: &testPlatform{env: map[string]string{"NODE_OPTIONS": "platform-options", "SHARED": "platform"}}} if err := vitest.RunTests(t.Context(), []string{"src/endOfYear/test.ts"}, workerEnv); !errors.Is(err, runErr) { t.Fatalf("got %v, want %v", err, runErr) } @@ -401,15 +401,7 @@ func TestVitest_RunTests_EmptyBatch(t *testing.T) { t.Fatal("empty batch must not invoke Vitest") return nil }} - if err := (&Vitest{executor: executor}).RunTests(t.Context(), nil, nil); err != nil { + if err := (&Vitest{executor: executor, platform: &testPlatform{}}).RunTests(t.Context(), nil, nil); err != nil { t.Fatal(err) } } - -func TestStripNodeOptionsImport(t *testing.T) { - input := "--import dd-trace/register.js --import=other/register.js --max-old-space-size=4096" - want := "--import=other/register.js --max-old-space-size=4096" - if got := stripNodeOptionsImport(input, ddTraceRegisterPath); got != want { - t.Fatalf("got %q, want %q", got, want) - } -} diff --git a/internal/planner/planner_test.go b/internal/planner/planner_test.go index 3769912f..dd9ae089 100644 --- a/internal/planner/planner_test.go +++ b/internal/planner/planner_test.go @@ -356,13 +356,7 @@ func (m *MockFramework) RunTests(ctx context.Context, testFiles []string, envMap return m.Err } -func (m *MockFramework) SetPlatformEnv(platformEnv map[string]string) { - // No-op for mock -} - -func (m *MockFramework) GetPlatformEnv() map[string]string { - return nil -} +func (m *MockFramework) Platform() framework.PlatformEnvironment { return nil } func (m *MockFramework) SupportsFullTestDiscovery() bool { return !m.FullDiscoveryUnsupported diff --git a/internal/planner/positional_selection_test.go b/internal/planner/positional_selection_test.go index c1f9b75b..3a3251a3 100644 --- a/internal/planner/positional_selection_test.go +++ b/internal/planner/positional_selection_test.go @@ -10,6 +10,7 @@ import ( "github.com/DataDog/ddtest/internal/discovery" "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/platform" "github.com/DataDog/ddtest/internal/settings" "github.com/DataDog/ddtest/internal/testoptimization" "github.com/DataDog/ddtest/internal/testoptimization/api" @@ -25,15 +26,15 @@ func TestPositionalSelectionNarrowsFrameworkDiscovery(t *testing.T) { exclude string want []string }{ - {name: "RSpec directory", framework: framework.NewRSpec(), args: []string{"spec/models"}, want: []string{"spec/models/user_spec.rb"}}, - {name: "RSpec broad glob", framework: framework.NewRSpec(), args: []string{"spec/**/*"}, want: []string{"spec/models/user_spec.rb", "spec/requests/api_spec.rb"}}, - {name: "pytest directory", framework: framework.NewPytest(), args: []string{"tests"}, want: []string{"tests/test_user.py"}}, - {name: "multiple overlapping arguments", framework: framework.NewRSpec(), args: []string{"spec/models", "spec/requests/api_spec.rb", "spec/models/*"}, want: []string{"spec/models/user_spec.rb", "spec/requests/api_spec.rb"}}, - {name: "helper is not a test", framework: framework.NewRSpec(), args: []string{"spec/models/helper.rb"}}, - {name: "outside default roots", framework: framework.NewRSpec(), args: []string{"custom_specs"}}, - {name: "custom discovery root", framework: framework.NewRSpec(), args: []string{"custom_specs"}, location: "custom_specs/**/*_spec.rb", want: []string{"custom_specs/extra_spec.rb"}}, - {name: "exclude still applies", framework: framework.NewRSpec(), args: []string{"spec"}, exclude: "spec/requests/**", want: []string{"spec/models/user_spec.rb"}}, - {name: "unmatched glob", framework: framework.NewRSpec(), args: []string{"missing/**"}}, + {name: "RSpec directory", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"spec/models"}, want: []string{"spec/models/user_spec.rb"}}, + {name: "RSpec broad glob", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"spec/**/*"}, want: []string{"spec/models/user_spec.rb", "spec/requests/api_spec.rb"}}, + {name: "pytest directory", framework: framework.NewPytest(platform.NewPython()), args: []string{"tests"}, want: []string{"tests/test_user.py"}}, + {name: "multiple overlapping arguments", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"spec/models", "spec/requests/api_spec.rb", "spec/models/*"}, want: []string{"spec/models/user_spec.rb", "spec/requests/api_spec.rb"}}, + {name: "helper is not a test", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"spec/models/helper.rb"}}, + {name: "outside default roots", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"custom_specs"}}, + {name: "custom discovery root", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"custom_specs"}, location: "custom_specs/**/*_spec.rb", want: []string{"custom_specs/extra_spec.rb"}}, + {name: "exclude still applies", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"spec"}, exclude: "spec/requests/**", want: []string{"spec/models/user_spec.rb"}}, + {name: "unmatched glob", framework: framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), args: []string{"missing/**"}}, } { t.Run(tt.name, func(t *testing.T) { t.Chdir(t.TempDir()) diff --git a/internal/platform/framework_env_test.go b/internal/platform/framework_env_test.go new file mode 100644 index 00000000..5363d566 --- /dev/null +++ b/internal/platform/framework_env_test.go @@ -0,0 +1,164 @@ +package platform + +import ( + "fmt" + "os" + "testing" + + "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/settings" + "github.com/stretchr/testify/require" +) + +func frameworkRunEnv(t *testing.T, f framework.Framework) map[string]string { + t.Helper() + env, err := f.Platform().RunEnv(framework.RuntimeOptions{ESM: f.Name() == "vitest"}) + if err != nil { + t.Fatal(err) + } + return env +} + +func TestEveryFrameworkRetainsItsPlatform(t *testing.T) { + for _, tc := range []struct { + platform Platform + frameworks []string + }{ + {NewRuby(settings.TestSkippingLevelTest), []string{"rspec", "minitest"}}, + {NewPython(), []string{"pytest"}}, + {NewJavaScript(), []string{"jest", "mocha", "vitest", "playwright", "cypress", "cucumber"}}, + } { + for _, name := range tc.frameworks { + t.Run(name, func(t *testing.T) { + resetDetectionSettings(t) + settings.Get().Framework = name + t.Setenv("PATH", t.TempDir()) // Construction must not probe runtimes or tracers. + fw, err := tc.platform.DetectFramework() + require.NoError(t, err) + require.Same(t, tc.platform, fw.Platform()) + }) + } + } +} + +func TestJavaScriptDiscoveryEnvironment(t *testing.T) { + inherited := `--require "/project with spaces/.pnp.cjs" --require="/external/dd-trace/ci/init.js" --import=/external/dd-trace/register.js --max-old-space-size=4096` + t.Setenv("NODE_OPTIONS", inherited) + for _, esm := range []bool{false, true} { + t.Run(fmt.Sprintf("ESM=%t", esm), func(t *testing.T) { + input := map[string]string{"CUSTOM": "value"} + p := NewJavaScript() + env, err := p.DiscoveryEnv(t.Context(), framework.FileDiscovery, framework.RuntimeOptions{ESM: esm, Env: input, PreloadFiles: []string{"/adapter with spaces/entry.js"}}) + require.NoError(t, err) + want := `--require "/project with spaces/.pnp.cjs" --import=/external/dd-trace/register.js --max-old-space-size=4096 --require "/adapter with spaces/entry.js"` + if esm { + want = `--require "/project with spaces/.pnp.cjs" --max-old-space-size=4096 --require "/adapter with spaces/entry.js"` + } + require.Equal(t, want, env["NODE_OPTIONS"]) + require.Equal(t, map[string]string{"CUSTOM": "value"}, input) + require.Equal(t, "value", env["CUSTOM"]) + require.Equal(t, inherited, os.Getenv("NODE_OPTIONS")) + }) + } +} + +func TestPlatformEnvironmentOverridesAndIsolation(t *testing.T) { + t.Setenv("NODE_OPTIONS", "--require project-loader.cjs") + t.Setenv("RUBYOPT", "-rproject_setup") + t.Setenv("PYTEST_ADDOPTS", "-q") + for _, tc := range []struct { + p Platform + name, key string + }{ + {NewJavaScript(), "vitest", "NODE_OPTIONS"}, + {NewRuby(settings.TestSkippingLevelTest), "rspec", "RUBYOPT"}, + {NewPython(), "pytest", "PYTEST_ADDOPTS"}, + } { + t.Run(tc.p.Name(), func(t *testing.T) { + for _, value := range []string{"", "explicit override"} { + input := map[string]string{tc.key: value, "CUSTOM": "input"} + env, err := tc.p.RunEnv(framework.RuntimeOptions{ESM: tc.name == "vitest", Env: input}) + require.NoError(t, err) + require.Equal(t, input, env, "explicit values, including empty ones, override defaults") + env["CUSTOM"] = "changed" + require.Equal(t, "input", input["CUSTOM"]) + again, err := tc.p.RunEnv(framework.RuntimeOptions{ESM: tc.name == "vitest", Env: input}) + require.NoError(t, err) + require.Equal(t, "input", again["CUSTOM"], "returned maps must not share state") + } + }) + } +} + +func TestDiscoveryUsesOverridesBeforeRemovingPreloads(t *testing.T) { + t.Setenv("NODE_OPTIONS", "--require inherited-loader.cjs -r dd-trace/ci/init") + p := NewJavaScript() + for _, tc := range []struct{ value, want string }{ + {"", ""}, + {`--require "override loader.cjs" -r dd-trace/ci/init`, `--require "override loader.cjs"`}, + } { + env, err := p.DiscoveryEnv(t.Context(), framework.FileDiscovery, framework.RuntimeOptions{Env: map[string]string{"NODE_OPTIONS": tc.value}}) + require.NoError(t, err) + require.Equal(t, tc.want, env["NODE_OPTIONS"]) + } +} + +func TestFileDiscoveryDoesNotCheckTracer(t *testing.T) { + for _, p := range []Platform{ + &Ruby{executor: nil}, &Python{executor: nil}, &JavaScript{executor: nil}, + } { + t.Run(p.Name(), func(t *testing.T) { + env, err := p.DiscoveryEnv(t.Context(), framework.FileDiscovery, framework.RuntimeOptions{Env: map[string]string{"CUSTOM": "value"}}) + require.NoError(t, err) + require.Equal(t, "value", env["CUSTOM"]) + }) + } +} + +func TestPythonFullDiscoveryDoesNotCheckTracer(t *testing.T) { + p := &Python{executor: nil} + env, err := p.DiscoveryEnv(t.Context(), framework.FullDiscovery, framework.RuntimeOptions{}) + require.NoError(t, err) + require.Contains(t, env["PYTEST_ADDOPTS"], "--ddtrace") +} + +func TestSelectedFrameworkKeepsCapturedEnvironment(t *testing.T) { + for _, tc := range []struct { + p Platform + framework, key, initial string + }{ + {NewJavaScript(), "mocha", "NODE_OPTIONS", "--require original-loader.cjs"}, + {NewJavaScript(), "vitest", "NODE_OPTIONS", "--require original-loader.cjs"}, + {NewRuby(settings.TestSkippingLevelTest), "rspec", "RUBYOPT", "-roriginal_setup"}, + {NewPython(), "pytest", "PYTEST_ADDOPTS", "-q"}, + } { + t.Run(tc.framework, func(t *testing.T) { + resetDetectionSettings(t) + settings.Get().Framework = tc.framework + t.Setenv(tc.key, tc.initial) + if tc.p.Name() == "ruby" { + require.NoError(t, os.Unsetenv(tc.key)) + } + t.Setenv("DD_TRACE_PACKAGE", "/original/dd-trace/ci/init.js") + t.Setenv("DD_TRACE_ESM_IMPORT", "/original/dd-trace/register.js") + fw, err := tc.p.DetectFramework() + require.NoError(t, err) + before := frameworkRunEnv(t, fw) + if tc.p.Name() == "ruby" { + require.Equal(t, rubyOptDefaultValue, before[tc.key]) + } else { + require.Contains(t, before[tc.key], tc.initial) + } + + t.Setenv(tc.key, "changed after detection") + t.Setenv("DD_TRACE_PACKAGE", "/changed/dd-trace/ci/init.js") + t.Setenv("DD_TRACE_ESM_IMPORT", "/changed/dd-trace/register.js") + require.Equal(t, before, frameworkRunEnv(t, fw)) + if tc.p.Name() == "javascript" { + env, err := tc.p.DiscoveryEnv(t.Context(), framework.FileDiscovery, framework.RuntimeOptions{ESM: tc.framework == "vitest"}) + require.NoError(t, err) + require.Equal(t, tc.initial, env[tc.key]) + } + }) + } +} diff --git a/internal/platform/javascript.go b/internal/platform/javascript.go index aaf3dcd5..e0afffa9 100644 --- a/internal/platform/javascript.go +++ b/internal/platform/javascript.go @@ -17,7 +17,6 @@ import ( "github.com/DataDog/ddtest/internal/ext" "github.com/DataDog/ddtest/internal/framework" "github.com/DataDog/ddtest/internal/settings" - "github.com/DataDog/ddtest/internal/utils" "github.com/kballard/go-shellquote" ) @@ -33,7 +32,9 @@ const ( ) type JavaScript struct { - executor commandExecutor + frameworkEnv map[string]string + esmEnv map[string]string + executor commandExecutor } func NewJavaScript() *JavaScript { @@ -53,7 +54,7 @@ func (j *JavaScript) Detect(repositoryRoot string) (bool, error) { func (j *JavaScript) DetectFramework() (framework.Framework, error) { root := "." hint := settings.GetFramework() - candidates := []framework.Framework{framework.NewJest(), framework.NewMocha(), framework.NewCypress(), framework.NewPlaywright(), framework.NewCucumber(), framework.NewVitest()} + candidates := []framework.Framework{framework.NewJest(j), framework.NewMocha(j), framework.NewCypress(j), framework.NewPlaywright(j), framework.NewCucumber(j), framework.NewVitest(j)} if hint == "" { manifest, found, err := readPackageManifest(root) if err != nil { @@ -78,11 +79,8 @@ func (j *JavaScript) DetectFramework() (framework.Framework, error) { if err != nil { return nil, err } - env := j.GetPlatformEnv() - if fw.Name() == "vitest" { - env = addNodeImport(env, ddTraceRegisterModule) - } - fw.SetPlatformEnv(env) + j.frameworkEnv = j.baseEnv() + j.esmEnv = addNodeImport(maps.Clone(j.frameworkEnv), ddTraceRegisterModule) return fw, nil } @@ -90,12 +88,12 @@ func (j *JavaScript) TestSkippingLevel() settings.TestSkippingLevel { return settings.TestSkippingLevelSuite } -// GetPlatformEnv returns environment variables required for JS commands. -func (j *JavaScript) GetPlatformEnv() map[string]string { +// baseEnv returns environment variables required for JS commands. +func (j *JavaScript) baseEnv() map[string]string { // Jest and Vitest need CI initialization. // Add the preload only when missing and preserve existing NODE_OPTIONS. currentValue, _ := os.LookupEnv(nodeOptionsEnvVar) - if utils.NodeOptionsHasRequire(currentValue, ddTraceCIInitModule) { + if nodeOptionsHasRequire(currentValue, ddTraceCIInitModule) { return map[string]string{} } @@ -120,7 +118,7 @@ func addNodeImport(platformEnv map[string]string, module string) map[string]stri if !ok { nodeOptions, _ = os.LookupEnv(nodeOptionsEnvVar) } - if utils.NodeOptionsHasImport(nodeOptions, module) { + if nodeOptionsHasImport(nodeOptions, module) { return platformEnv } @@ -129,7 +127,7 @@ func addNodeImport(platformEnv map[string]string, module string) map[string]stri } else { // An explicit external CI preload also identifies its register module, // even when the action's optional ESM variable is unavailable. - preload := utils.NodeOptionsRequire(nodeOptions, ddTraceCIInitModule) + preload := nodeOptionsRequire(nodeOptions, ddTraceCIInitModule) if filepath.IsAbs(preload) { module = strconv.Quote(filepath.Join(filepath.Dir(filepath.Dir(preload)), "register.js")) } @@ -147,8 +145,8 @@ func javascriptProbeEnv() map[string]string { if !found || current == "" { return nil } - cleaned := utils.NodeOptionsWithoutRequire(current, ddTraceCIInitModule) - cleaned = utils.NodeOptionsWithoutImport(cleaned, ddTraceRegisterModule) + cleaned := nodeOptionsWithoutRequire(current, ddTraceCIInitModule) + cleaned = nodeOptionsWithoutImport(cleaned, ddTraceRegisterModule) if cleaned == current { return nil } @@ -282,7 +280,7 @@ func (j *JavaScript) DetectTracer(ctx context.Context, _ TracerOptions) (string, // Preserve other project loaders, including Yarn PnP, while ensuring that // the resolution probe itself never starts Test Optimization. probeEnv := javascriptProbeEnv() - preload := utils.NodeOptionsRequire(os.Getenv(nodeOptionsEnvVar), ddTraceCIInitModule) + preload := nodeOptionsRequire(os.Getenv(nodeOptionsEnvVar), ddTraceCIInitModule) if preload == "" { preload = os.Getenv("DD_TRACE_PACKAGE") } @@ -353,3 +351,180 @@ func (j *JavaScript) TracerInstallCommand(options TracerOptions) (string, []stri } return "npm", installArgs, nil } + +func (j *JavaScript) RunEnv(options framework.RuntimeOptions) (map[string]string, error) { + env := j.executionEnv(options.ESM) + maps.Copy(env, options.Env) + appendNodePreloads(env, options.PreloadFiles) + return env, nil +} + +func (j *JavaScript) DiscoveryEnv(_ context.Context, kind framework.DiscoveryKind, options framework.RuntimeOptions) (map[string]string, error) { + if kind != framework.FileDiscovery { + return nil, fmt.Errorf("JavaScript full test discovery is not supported") + } + env := j.executionEnv(options.ESM) + maps.Copy(env, options.Env) + current, found := env[nodeOptionsEnvVar] + if !found { + current, found = os.LookupEnv(nodeOptionsEnvVar) + } + if found { + current = nodeOptionsWithoutRequire(current, ddTraceCIInitModule) + if options.ESM { + current = nodeOptionsWithoutImport(current, ddTraceRegisterModule) + } + env[nodeOptionsEnvVar] = current + } + appendNodePreloads(env, options.PreloadFiles) + return env, nil +} + +func appendNodePreloads(env map[string]string, files []string) { + if len(files) == 0 { + return + } + current, found := env[nodeOptionsEnvVar] + if !found { + current = os.Getenv(nodeOptionsEnvVar) + } + for _, file := range files { + current = strings.TrimSpace(current + " --require " + strconv.Quote(file)) + } + env[nodeOptionsEnvVar] = current +} + +// executionEnv preserves the environment captured when the framework was selected. +func (j *JavaScript) executionEnv(esm bool) map[string]string { + if j.frameworkEnv != nil { + if esm { + return maps.Clone(j.esmEnv) + } + return maps.Clone(j.frameworkEnv) + } + env := j.baseEnv() + if esm { + env = addNodeImport(env, ddTraceRegisterModule) + } + return env +} + +type nodeOptionsToken struct { + raw string + value string +} + +// Node uses double quotes, rather than shell quoting, in NODE_OPTIONS. Keep +// each original token so removing a preload does not change other options or +// quoted project loader paths. +func splitnodeOptions(value string) []nodeOptionsToken { + var tokens []nodeOptionsToken + for index := 0; index < len(value); { + for index < len(value) && isnodeOptionsSpace(value[index]) { + index++ + } + if index == len(value) { + break + } + start := index + var decoded strings.Builder + quoted := false + for index < len(value) { + current := value[index] + if current == '"' { + quoted = !quoted + index++ + continue + } + if current == '\\' && index+1 < len(value) && value[index+1] == '"' { + decoded.WriteByte('"') + index += 2 + continue + } + if !quoted && isnodeOptionsSpace(current) { + break + } + decoded.WriteByte(current) + index++ + } + tokens = append(tokens, nodeOptionsToken{raw: value[start:index], value: decoded.String()}) + } + return tokens +} + +func isnodeOptionsSpace(value byte) bool { + return value == ' ' || value == '\t' || value == '\n' || value == '\r' +} + +func nodeOptionsOptionValue(tokens []nodeOptionsToken, index int, option string) (string, int, bool) { + value := tokens[index].value + if value == option || (option == "--require" && value == "-r") { + if index+1 < len(tokens) { + return tokens[index+1].value, 2, true + } + return "", 1, false + } + if strings.HasPrefix(value, option+"=") { + return strings.TrimPrefix(value, option+"="), 1, true + } + if option == "--require" && strings.HasPrefix(value, "-r") && len(value) > 2 { + return strings.TrimPrefix(value, "-r"), 1, true + } + return "", 1, false +} + +func nodeOptionsMatchesModule(value, module string) bool { + if value == module { + return true + } + normalized := strings.ReplaceAll(value, "\\", "/") + return strings.HasSuffix(normalized, "/"+module) || + (module == "dd-trace/ci/init" && (value == module+".js" || strings.HasSuffix(normalized, "/"+module+".js"))) +} + +func findNodeOption(value, option, module string) string { + tokens := splitnodeOptions(value) + for index := 0; index < len(tokens); index++ { + candidate, width, found := nodeOptionsOptionValue(tokens, index, option) + if found && nodeOptionsMatchesModule(candidate, module) { + return candidate + } + index += width - 1 + } + return "" +} + +func nodeOptionsHasRequire(value, module string) bool { + return nodeOptionsRequire(value, module) != "" +} + +func nodeOptionsRequire(value, module string) string { + return findNodeOption(value, "--require", module) +} + +func nodeOptionsHasImport(value, module string) bool { + return findNodeOption(value, "--import", module) != "" +} + +func withoutNodeOption(value, option, module string) string { + tokens := splitnodeOptions(value) + kept := make([]string, 0, len(tokens)) + for index := 0; index < len(tokens); { + candidate, width, found := nodeOptionsOptionValue(tokens, index, option) + if found && nodeOptionsMatchesModule(candidate, module) { + index += width + continue + } + kept = append(kept, tokens[index].raw) + index++ + } + return strings.Join(kept, " ") +} + +func nodeOptionsWithoutRequire(value, module string) string { + return withoutNodeOption(value, "--require", module) +} + +func nodeOptionsWithoutImport(value, module string) string { + return withoutNodeOption(value, "--import", module) +} diff --git a/internal/platform/javascript_test.go b/internal/platform/javascript_test.go index 07c25d65..d4f7a55d 100644 --- a/internal/platform/javascript_test.go +++ b/internal/platform/javascript_test.go @@ -54,22 +54,22 @@ func TestJavaScript_TestSkippingLevel(t *testing.T) { } } -func TestJavaScript_GetPlatformEnv_SetsNODEOPTIONS(t *testing.T) { +func TestJavaScript_baseEnv_SetsNODEOPTIONS(t *testing.T) { t.Setenv(nodeOptionsEnvVar, "") javascript := NewJavaScript() - envMap := javascript.GetPlatformEnv() + envMap := javascript.baseEnv() if envMap[nodeOptionsEnvVar] != nodeOptionsDDTraceCIArg { t.Errorf("expected NODE_OPTIONS to be %q, got %q", nodeOptionsDDTraceCIArg, envMap[nodeOptionsEnvVar]) } } -func TestJavaScript_GetPlatformEnv_PreservesExistingNODEOPTIONS(t *testing.T) { +func TestJavaScript_baseEnv_PreservesExistingNODEOPTIONS(t *testing.T) { t.Setenv(nodeOptionsEnvVar, "--max-old-space-size=4096") javascript := NewJavaScript() - envMap := javascript.GetPlatformEnv() + envMap := javascript.baseEnv() expected := "--max-old-space-size=4096 " + nodeOptionsDDTraceCIArg if envMap[nodeOptionsEnvVar] != expected { @@ -77,11 +77,11 @@ func TestJavaScript_GetPlatformEnv_PreservesExistingNODEOPTIONS(t *testing.T) { } } -func TestJavaScript_GetPlatformEnv_DoesNotDuplicateDDTraceInit(t *testing.T) { +func TestJavaScript_baseEnv_DoesNotDuplicateDDTraceInit(t *testing.T) { t.Setenv(nodeOptionsEnvVar, "-r dd-trace/ci/init --max-old-space-size=4096") javascript := NewJavaScript() - envMap := javascript.GetPlatformEnv() + envMap := javascript.baseEnv() if len(envMap) != 0 { t.Errorf("expected empty env map when dd-trace init is already present, got %v", envMap) @@ -103,7 +103,7 @@ func TestJavaScript_ActionEnvironment(t *testing.T) { require.Len(t, executor.commands, 1) require.Contains(t, executor.commands[0].args, preload) - env := javascript.GetPlatformEnv() + env := javascript.baseEnv() require.Equal(t, os.Getenv(nodeOptionsEnvVar)+" -r "+strconv.Quote(preload), env[nodeOptionsEnvVar]) addNodeImport(env, ddTraceRegisterModule) require.Equal(t, "--import "+strconv.Quote(register)+" "+os.Getenv(nodeOptionsEnvVar)+" -r "+strconv.Quote(preload), env[nodeOptionsEnvVar]) @@ -114,7 +114,7 @@ func TestJavaScript_ExplicitPreloadsTakePrecedence(t *testing.T) { t.Setenv(nodeOptionsEnvVar, options) t.Setenv("DD_TRACE_PACKAGE", "/action/dd-trace/ci/init.js") t.Setenv("DD_TRACE_ESM_IMPORT", "/action/dd-trace/register.js") - env := NewJavaScript().GetPlatformEnv() + env := NewJavaScript().baseEnv() addNodeImport(env, ddTraceRegisterModule) require.Empty(t, env, "workers must inherit the customer's options unchanged") } @@ -122,7 +122,7 @@ func TestJavaScript_ExplicitPreloadsTakePrecedence(t *testing.T) { func TestJavaScript_RegisterUsesExternalPreloadInstallation(t *testing.T) { t.Setenv(nodeOptionsEnvVar, `-r "/customer install/dd-trace/ci/init.js"`) t.Setenv("DD_TRACE_ESM_IMPORT", "") - env := NewJavaScript().GetPlatformEnv() + env := NewJavaScript().baseEnv() addNodeImport(env, ddTraceRegisterModule) require.Equal(t, `--import "/customer install/dd-trace/register.js" `+os.Getenv(nodeOptionsEnvVar), env[nodeOptionsEnvVar]) } @@ -149,7 +149,7 @@ func TestJavaScript_DetectTracer_PrefersExplicitActionPreload(t *testing.T) { require.Len(t, executor.commands, 1, "validate only the explicitly selected preload") require.Contains(t, executor.commands[0].args, "/external/dd-trace/ci/init.js") require.Equal(t, map[string]string{nodeOptionsEnvVar: ""}, executor.envs[0]) - require.Empty(t, javascript.GetPlatformEnv(), "the absolute preload must not be duplicated for workers") + require.Empty(t, javascript.baseEnv(), "the absolute preload must not be duplicated for workers") } func TestJavaScript_DetectTracer_RejectsInvalidActionPreload(t *testing.T) { @@ -166,7 +166,7 @@ func TestJavaScript_DetectTracer_RejectsInvalidActionPreload(t *testing.T) { require.ErrorContains(t, err, preload) } -func TestJavaScript_GetPlatformEnv_DoesNotDependOnFramework(t *testing.T) { +func TestJavaScript_baseEnv_DoesNotDependOnFramework(t *testing.T) { t.Setenv(nodeOptionsEnvVar, "") viper.Reset() viper.Set("framework", "vitest") @@ -176,7 +176,7 @@ func TestJavaScript_GetPlatformEnv_DoesNotDependOnFramework(t *testing.T) { settings.Init() }() - if got := NewJavaScript().GetPlatformEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { + if got := NewJavaScript().baseEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { t.Fatalf("NODE_OPTIONS = %q, want %q", got, nodeOptionsDDTraceCIArg) } } @@ -329,7 +329,7 @@ func TestJavaScript_DetectFramework_Mocha(t *testing.T) { if fw.Name() != "mocha" { t.Fatalf("framework name = %q, want mocha", fw.Name()) } - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { t.Fatalf("NODE_OPTIONS = %q, want %q", got, nodeOptionsDDTraceCIArg) } } @@ -351,7 +351,7 @@ func TestJavaScript_DetectFramework_Cypress(t *testing.T) { if fw.Name() != "cypress" { t.Fatalf("framework name = %q, want cypress", fw.Name()) } - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { t.Fatalf("NODE_OPTIONS = %q, want %q", got, nodeOptionsDDTraceCIArg) } } @@ -373,7 +373,7 @@ func TestJavaScript_DetectFramework_Playwright(t *testing.T) { if fw.Name() != "playwright" { t.Fatalf("framework name = %q, want playwright", fw.Name()) } - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { t.Fatalf("NODE_OPTIONS = %q, want %q", got, nodeOptionsDDTraceCIArg) } } @@ -395,7 +395,7 @@ func TestJavaScript_DetectFramework_Cucumber(t *testing.T) { if fw.Name() != "cucumber" { t.Fatalf("framework name = %q, want cucumber", fw.Name()) } - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != nodeOptionsDDTraceCIArg { t.Fatalf("NODE_OPTIONS = %q, want %q", got, nodeOptionsDDTraceCIArg) } } @@ -418,7 +418,7 @@ func TestJavaScript_DetectFramework_Vitest(t *testing.T) { t.Fatalf("framework name = %q, want vitest", fw.Name()) } wantNodeOptions := nodeImportArg + " " + ddTraceRegisterModule + " " + nodeOptionsDDTraceCIArg - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != wantNodeOptions { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != wantNodeOptions { t.Fatalf("NODE_OPTIONS = %q, want %q", got, wantNodeOptions) } } @@ -439,7 +439,7 @@ func TestJavaScript_DetectFramework_VitestPreservesExistingOptions(t *testing.T) } want := nodeImportArg + " " + ddTraceRegisterModule + " " + nodeOptionsDDTraceCIArg + " --max-old-space-size=4096" - if got := fw.GetPlatformEnv()[nodeOptionsEnvVar]; got != want { + if got := frameworkRunEnv(t, fw)[nodeOptionsEnvVar]; got != want { t.Fatalf("NODE_OPTIONS = %q, want %q", got, want) } } @@ -459,7 +459,7 @@ func TestJavaScript_DetectFramework_VitestDoesNotDuplicateRegister(t *testing.T) t.Fatalf("DetectFramework failed: %v", err) } - nodeOptions := fw.GetPlatformEnv()[nodeOptionsEnvVar] + nodeOptions := frameworkRunEnv(t, fw)[nodeOptionsEnvVar] if strings.Count(nodeOptions, ddTraceRegisterModule) != 1 { t.Fatalf("NODE_OPTIONS contains duplicate registration: %q", nodeOptions) } @@ -591,13 +591,13 @@ func TestJavaScript_DetectFramework_SetsPlatformEnv(t *testing.T) { t.Fatal("expected framework to be non-nil") } - frameworkPlatformEnv := fw.GetPlatformEnv() + frameworkPlatformEnv := frameworkRunEnv(t, fw) if frameworkPlatformEnv[nodeOptionsEnvVar] != nodeOptionsDDTraceCIArg { t.Errorf("expected framework platformEnv %s=%q, got %q", nodeOptionsEnvVar, nodeOptionsDDTraceCIArg, frameworkPlatformEnv[nodeOptionsEnvVar]) } } -func TestJavaScript_GetPlatformEnv_UnsetNODEOPTIONS(t *testing.T) { +func TestJavaScript_baseEnv_UnsetNODEOPTIONS(t *testing.T) { // When NODE_OPTIONS is completely unset (not just empty), we should still // set it to the dd-trace init argument. if err := os.Unsetenv(nodeOptionsEnvVar); err != nil { @@ -605,7 +605,7 @@ func TestJavaScript_GetPlatformEnv_UnsetNODEOPTIONS(t *testing.T) { } javascript := NewJavaScript() - envMap := javascript.GetPlatformEnv() + envMap := javascript.baseEnv() if envMap[nodeOptionsEnvVar] != nodeOptionsDDTraceCIArg { t.Errorf("expected NODE_OPTIONS to be %q, got %q", nodeOptionsDDTraceCIArg, envMap[nodeOptionsEnvVar]) diff --git a/internal/utils/node_options_test.go b/internal/platform/node_options_test.go similarity index 73% rename from internal/utils/node_options_test.go rename to internal/platform/node_options_test.go index 0f48ff3e..25085352 100644 --- a/internal/utils/node_options_test.go +++ b/internal/platform/node_options_test.go @@ -1,4 +1,4 @@ -package utils +package platform import "testing" @@ -18,13 +18,13 @@ func TestCIRequireOptions(t *testing.T) { {name: "similar package", options: "-r dd-trace/ci/initializer", without: "-r dd-trace/ci/initializer"}, } { t.Run(test.name, func(t *testing.T) { - if got := NodeOptionsHasRequire(test.options, "dd-trace/ci/init"); got != test.has { + if got := nodeOptionsHasRequire(test.options, "dd-trace/ci/init"); got != test.has { t.Fatalf("HasRequire() = %t, want %t", got, test.has) } - if got := NodeOptionsRequire(test.options, "dd-trace/ci/init"); got != test.require { + if got := nodeOptionsRequire(test.options, "dd-trace/ci/init"); got != test.require { t.Fatalf("Require() = %q, want %q", got, test.require) } - if got := NodeOptionsWithoutRequire(test.options, "dd-trace/ci/init"); got != test.without { + if got := nodeOptionsWithoutRequire(test.options, "dd-trace/ci/init"); got != test.without { t.Fatalf("WithoutRequire() = %q, want %q", got, test.without) } }) @@ -33,10 +33,18 @@ func TestCIRequireOptions(t *testing.T) { func TestRegisterImportOptions(t *testing.T) { options := `--import="/tmp/action install/dd-trace/register.js" --require "/tmp/loader.cjs"` - if !NodeOptionsHasImport(options, "dd-trace/register.js") { + if !nodeOptionsHasImport(options, "dd-trace/register.js") { t.Fatal("absolute register import was not found") } - if got := NodeOptionsWithoutImport(options, "dd-trace/register.js"); got != `--require "/tmp/loader.cjs"` { + if got := nodeOptionsWithoutImport(options, "dd-trace/register.js"); got != `--require "/tmp/loader.cjs"` { t.Fatalf("WithoutImport() = %q", got) } } + +func TestStripNodeOptionsImport(t *testing.T) { + input := "--import dd-trace/register.js --import=other/register.js --max-old-space-size=4096" + want := "--import=other/register.js --max-old-space-size=4096" + if got := nodeOptionsWithoutImport(input, ddTraceRegisterModule); got != want { + t.Fatalf("got %q, want %q", got, want) + } +} diff --git a/internal/platform/platform.go b/internal/platform/platform.go index 1746d305..3b6aceb0 100644 --- a/internal/platform/platform.go +++ b/internal/platform/platform.go @@ -13,11 +13,10 @@ import ( ) type Platform interface { - Name() string + framework.PlatformEnvironment Detect(repositoryRoot string) (bool, error) CreateTagsMap(ctx context.Context) (map[string]string, error) DetectFramework() (framework.Framework, error) - SanityCheck(ctx context.Context) error DetectTracer(ctx context.Context, options TracerOptions) (string, error) TracerInstallCommand(options TracerOptions) (string, []string, error) InstallTestdriveTracer(ctx context.Context, options TracerOptions) (TracerInstallation, error) diff --git a/internal/platform/platform_test.go b/internal/platform/platform_test.go index 6437a3c3..ebdf099f 100644 --- a/internal/platform/platform_test.go +++ b/internal/platform/platform_test.go @@ -197,7 +197,7 @@ func TestAutomaticPlatformAndFrameworkSelection(t *testing.T) { fw, err := p.DetectFramework() require.NoError(t, err) require.Equal(t, tc.runner, fw.Name()) - require.Contains(t, fw.GetPlatformEnv(), tc.env) + require.Contains(t, frameworkRunEnv(t, fw), tc.env) lang, readOnly, err := detectFixture(t, root, "") require.NoError(t, err) require.Equal(t, p.Name(), lang) diff --git a/internal/platform/python.go b/internal/platform/python.go index f61eb448..40342eda 100644 --- a/internal/platform/python.go +++ b/internal/platform/python.go @@ -42,7 +42,8 @@ const ( ) type Python struct { - executor commandExecutor + frameworkEnv map[string]string + executor commandExecutor } func NewPython() *Python { @@ -82,11 +83,11 @@ func (p *Python) Detect(root string) (bool, error) { // Pytest is the only supported Python framework and is the platform default. func (p *Python) DetectFramework() (framework.Framework, error) { hint := settings.GetFramework() - fw, err := selectFramework(p.Name(), hint, []framework.Framework{framework.NewPytest()}) + fw, err := selectFramework(p.Name(), hint, []framework.Framework{framework.NewPytest(p)}) if err != nil { return nil, err } - fw.SetPlatformEnv(p.GetPlatformEnv()) + p.frameworkEnv = p.baseEnv() return fw, nil } @@ -94,9 +95,9 @@ func (p *Python) TestSkippingLevel() settings.TestSkippingLevel { return settings.TestSkippingLevelTest } -// GetPlatformEnv returns environment variables required for Python commands. +// baseEnv returns environment variables required for Python commands. // It appends --ddtrace to PYTEST_ADDOPTS to load the ddtrace pytest plugin. -func (p *Python) GetPlatformEnv() map[string]string { +func (p *Python) baseEnv() map[string]string { envMap := make(map[string]string) // Get existing PYTEST_ADDOPTS if set, then append --ddtrace @@ -262,3 +263,26 @@ func (p *Python) TracerInstallCommand(options TracerOptions) (string, []string, args := append(append([]string{}, prefixArgs...), "-m", "pip", "install", "--disable-pip-version-check", "--target", target, packageName) return command, args, nil } + +func (p *Python) RunEnv(options framework.RuntimeOptions) (map[string]string, error) { + if len(options.PreloadFiles) != 0 { + return nil, fmt.Errorf("Python framework preloads are not supported") + } + env := maps.Clone(p.frameworkEnv) + if env == nil { + env = p.baseEnv() + } + maps.Copy(env, options.Env) + return env, nil +} + +func (p *Python) DiscoveryEnv(_ context.Context, kind framework.DiscoveryKind, options framework.RuntimeOptions) (map[string]string, error) { + switch kind { + case framework.FileDiscovery: + return maps.Clone(options.Env), nil + case framework.FullDiscovery: + return p.RunEnv(options) + default: + return nil, fmt.Errorf("unknown discovery kind: %d", kind) + } +} diff --git a/internal/platform/python_test.go b/internal/platform/python_test.go index cf191fdf..9bddbae3 100644 --- a/internal/platform/python_test.go +++ b/internal/platform/python_test.go @@ -136,7 +136,7 @@ func TestPython_SanityCheck_InvalidVersion(t *testing.T) { } } -func TestPython_GetPlatformEnv_SetsWhenNotSet(t *testing.T) { +func TestPython_baseEnv_SetsWhenNotSet(t *testing.T) { original, existed := os.LookupEnv(pytestAddOptsEnvVar) if existed { _ = os.Unsetenv(pytestAddOptsEnvVar) @@ -144,14 +144,14 @@ func TestPython_GetPlatformEnv_SetsWhenNotSet(t *testing.T) { } python := NewPython() - envMap := python.GetPlatformEnv() + envMap := python.baseEnv() if envMap[pytestAddOptsEnvVar] != pytestDefaultAddOpts { t.Errorf("expected %s=%q, got %q", pytestAddOptsEnvVar, pytestDefaultAddOpts, envMap[pytestAddOptsEnvVar]) } } -func TestPython_GetPlatformEnv_AppendsWhenAlreadySet(t *testing.T) { +func TestPython_baseEnv_AppendsWhenAlreadySet(t *testing.T) { original, existed := os.LookupEnv(pytestAddOptsEnvVar) existingValue := "-v --tb=short" _ = os.Setenv(pytestAddOptsEnvVar, existingValue) @@ -164,7 +164,7 @@ func TestPython_GetPlatformEnv_AppendsWhenAlreadySet(t *testing.T) { }() python := NewPython() - envMap := python.GetPlatformEnv() + envMap := python.baseEnv() expected := existingValue + " " + pytestDefaultAddOpts if envMap[pytestAddOptsEnvVar] != expected { @@ -304,7 +304,7 @@ func TestPython_DetectFramework_Pytest(t *testing.T) { settings.Init() }() - // Ensure PYTEST_ADDOPTS is unset so GetPlatformEnv produces a deterministic value + // Ensure PYTEST_ADDOPTS is unset so baseEnv produces a deterministic value original, existed := os.LookupEnv(pytestAddOptsEnvVar) if existed { _ = os.Unsetenv(pytestAddOptsEnvVar) @@ -324,7 +324,7 @@ func TestPython_DetectFramework_Pytest(t *testing.T) { t.Errorf("expected framework name 'pytest', got %q", fw.Name()) } - frameworkEnv := fw.GetPlatformEnv() + frameworkEnv := frameworkRunEnv(t, fw) if frameworkEnv[pytestAddOptsEnvVar] != pytestDefaultAddOpts { t.Errorf("expected framework platformEnv %s=%q, got %q", pytestAddOptsEnvVar, pytestDefaultAddOpts, frameworkEnv[pytestAddOptsEnvVar]) diff --git a/internal/platform/ruby.go b/internal/platform/ruby.go index 78df3278..34e6bf31 100644 --- a/internal/platform/ruby.go +++ b/internal/platform/ruby.go @@ -9,24 +9,28 @@ import ( "maps" "os" "path/filepath" + "regexp" "strings" "github.com/DataDog/ddtest/internal/constants" "github.com/DataDog/ddtest/internal/ext" "github.com/DataDog/ddtest/internal/framework" "github.com/DataDog/ddtest/internal/settings" - "github.com/DataDog/ddtest/internal/utils" + "github.com/DataDog/ddtest/internal/version" ) //go:embed scripts/ruby_env.rb var rubyEnvScript string const ( - rubyOptEnvVar = "RUBYOPT" - rubyOptDefaultValue = "-rbundler/setup -rdatadog/ci/auto_instrument" + requiredGemName = "datadog-ci" + requiredGemMinVersion = "1.31.0" + rubyOptEnvVar = "RUBYOPT" + rubyOptDefaultValue = "-rbundler/setup -rdatadog/ci/auto_instrument" ) type Ruby struct { + frameworkEnv map[string]string executor commandExecutor testSkippingLevel settings.TestSkippingLevel } @@ -49,7 +53,7 @@ func (r *Ruby) Detect(repositoryRoot string) (bool, error) { func (r *Ruby) DetectFramework() (framework.Framework, error) { root := "." hint := settings.GetFramework() - candidates := []framework.Framework{framework.NewRSpec(), framework.NewMinitest()} + candidates := []framework.Framework{framework.NewRSpec(r), framework.NewMinitest(r)} if hint == "" { candidates = nil gemfile, err := os.ReadFile(filepath.Join(root, "Gemfile")) @@ -61,21 +65,21 @@ func (r *Ruby) DetectFramework() (framework.Framework, error) { return nil, err } if rspec || strings.Contains(string(gemfile), "rspec") { - candidates = append(candidates, framework.NewRSpec()) + candidates = append(candidates, framework.NewRSpec(r)) } tests, err := os.Stat(filepath.Join(root, "test")) if err != nil && !os.IsNotExist(err) { return nil, err } if err == nil && tests.IsDir() || strings.Contains(string(gemfile), "minitest") { - candidates = append(candidates, framework.NewMinitest()) + candidates = append(candidates, framework.NewMinitest(r)) } } fw, err := selectFramework(r.Name(), hint, candidates) if err != nil { return nil, err } - fw.SetPlatformEnv(r.GetPlatformEnv()) + r.frameworkEnv = r.baseEnv() return fw, nil } @@ -83,9 +87,9 @@ func (r *Ruby) TestSkippingLevel() settings.TestSkippingLevel { return r.testSkippingLevel } -// GetPlatformEnv returns environment variables required for Ruby commands. +// baseEnv returns environment variables required for Ruby commands. // It sets RUBYOPT to auto-instrument with datadog-ci if not already set. -func (r *Ruby) GetPlatformEnv() map[string]string { +func (r *Ruby) baseEnv() map[string]string { envMap := make(map[string]string) // Check if RUBYOPT is already set in the environment @@ -135,17 +139,13 @@ func (r *Ruby) CreateTagsMap(ctx context.Context) (map[string]string, error) { return tags, nil } -func (r *Ruby) SanityCheck(ctx context.Context) error { - return utils.CheckRubyTracer(ctx, r.executor) -} - // DetectTracer reads the project tracer's bundle information. func (r *Ruby) DetectTracer(ctx context.Context, _ TracerOptions) (string, error) { - gemVersion, err := utils.DetectRubyTracer(ctx, r.executor) + gemVersion, err := r.detectTracerVersion(ctx) if err != nil { return "", err } - return fmt.Sprintf(" * %s (%s)", utils.RubyTracerGemName, gemVersion.String()), nil + return fmt.Sprintf(" * %s (%s)", requiredGemName, gemVersion.String()), nil } func (r *Ruby) InstallTestdriveTracer(ctx context.Context, options TracerOptions) (TracerInstallation, error) { @@ -163,7 +163,7 @@ func (r *Ruby) InstallTestdriveTracer(ctx context.Context, options TracerOptions } func (r *Ruby) TracerInstallCommand(options TracerOptions) (string, []string, error) { - args := []string{"add", utils.RubyTracerGemName} + args := []string{"add", requiredGemName} if ref, ok := strings.CutPrefix(options.Version, "git:"); ok { if ref == "" { return "", nil, fmt.Errorf("tracer git ref must not be empty") @@ -174,3 +174,83 @@ func (r *Ruby) TracerInstallCommand(options TracerOptions) (string, []string, er } return "bundle", args, nil } + +func (r *Ruby) RunEnv(options framework.RuntimeOptions) (map[string]string, error) { + if len(options.PreloadFiles) != 0 { + return nil, fmt.Errorf("Ruby framework preloads are not supported") + } + env := maps.Clone(r.frameworkEnv) + if env == nil { + env = r.baseEnv() + } + maps.Copy(env, options.Env) + return env, nil +} + +func (r *Ruby) DiscoveryEnv(ctx context.Context, kind framework.DiscoveryKind, options framework.RuntimeOptions) (map[string]string, error) { + switch kind { + case framework.FileDiscovery: + return maps.Clone(options.Env), nil + case framework.FullDiscovery: + if err := r.SanityCheck(ctx); err != nil { + return nil, fmt.Errorf("full test discovery requires datadog-ci: %w", err) + } + return r.RunEnv(options) + default: + return nil, fmt.Errorf("unknown discovery kind: %d", kind) + } +} + +// SanityCheck checks the prerequisite shared by Ruby execution and full discovery. +func (r *Ruby) SanityCheck(ctx context.Context) error { + gemVersion, err := r.detectTracerVersion(ctx) + if err != nil { + return err + } + requiredVersion, err := version.Parse(requiredGemMinVersion) + if err != nil { + return err + } + if gemVersion.Compare(requiredVersion) < 0 { + return fmt.Errorf("datadog-ci gem version %s is lower than required >= %s", gemVersion.String(), requiredVersion.String()) + } + return nil +} + +// detectTracerVersion reads the tracer version from the project's bundle. +func (r *Ruby) detectTracerVersion(ctx context.Context) (version.Version, error) { + // Inherit project loaders without adding the instrumentation preload. + output, err := r.executor.CombinedOutput(ctx, "bundle", []string{"info", requiredGemName}, nil) + if err != nil { + return version.Version{}, fmt.Errorf("detect project tracer: %s: %w", strings.TrimSpace(string(output)), err) + } + return parseBundlerInfoVersion(string(output), requiredGemName) +} + +// bundlerInfoRegex matches bundler info output format: " * gem-name (version [hash])" +// Captures: 1=gem-name, 2=version +var bundlerInfoRegex = regexp.MustCompile(`^\s*\*\s+(\S+)\s+\((\d+\.\d+\.\d+)`) + +func parseBundlerInfoVersion(output, gemName string) (version.Version, error) { + for line := range strings.SplitSeq(output, "\n") { + matches := bundlerInfoRegex.FindStringSubmatch(line) + if matches == nil { + continue + } + + matchedGem := matches[1] + if matchedGem != gemName { + continue + } + + versionString := matches[2] + parsed, err := version.Parse(versionString) + if err != nil { + return version.Version{}, fmt.Errorf("failed to parse version from bundle info output: %w", err) + } + + return parsed, nil + } + + return version.Version{}, fmt.Errorf("unable to find datadog-ci gem version in bundle info output") +} diff --git a/internal/framework/ruby_test.go b/internal/platform/ruby_discovery_test.go similarity index 77% rename from internal/framework/ruby_test.go rename to internal/platform/ruby_discovery_test.go index 8a693811..29d57d5d 100644 --- a/internal/framework/ruby_test.go +++ b/internal/platform/ruby_discovery_test.go @@ -1,4 +1,4 @@ -package framework +package platform import ( "context" @@ -6,6 +6,8 @@ import ( "testing" "github.com/DataDog/ddtest/internal/discovery" + "github.com/DataDog/ddtest/internal/framework" + "github.com/DataDog/ddtest/internal/settings" "github.com/stretchr/testify/require" ) @@ -41,9 +43,11 @@ func TestRubyFullDiscoveryRequiresCompatibleTracer(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { executor := &rubyPrerequisiteExecutor{t: t, output: tc.output, err: tc.err} - for _, fw := range []Framework{ - newTestRSpecWithExecutor(executor), - newTestMinitestWithExecutor(executor), + ruby := NewRuby(settings.TestSkippingLevelTest) + ruby.executor = executor + for _, fw := range []framework.Framework{ + framework.NewRSpec(ruby), + framework.NewMinitest(ruby), } { t.Run(fw.Name(), func(t *testing.T) { executor.t = t @@ -58,3 +62,8 @@ func TestRubyFullDiscoveryRequiresCompatibleTracer(t *testing.T) { }) } } + +func (e *rubyPrerequisiteExecutor) Output(ctx context.Context, name string, args []string, env map[string]string) ([]byte, []byte, error) { + output, err := e.CombinedOutput(ctx, name, args, env) + return output, nil, err +} diff --git a/internal/platform/ruby_test.go b/internal/platform/ruby_test.go index a35d9d7f..96e94dee 100644 --- a/internal/platform/ruby_test.go +++ b/internal/platform/ruby_test.go @@ -473,7 +473,7 @@ func TestDetectPlatform_Unsupported(t *testing.T) { } } -func TestRuby_GetPlatformEnv_SetsRUBYOPT_WhenNotSet(t *testing.T) { +func TestRuby_baseEnv_SetsRUBYOPT_WhenNotSet(t *testing.T) { // Ensure RUBYOPT is not set originalValue, existed := os.LookupEnv("RUBYOPT") if existed { @@ -482,7 +482,7 @@ func TestRuby_GetPlatformEnv_SetsRUBYOPT_WhenNotSet(t *testing.T) { } ruby := newTestRuby() - envMap := ruby.GetPlatformEnv() + envMap := ruby.baseEnv() expectedValue := "-rbundler/setup -rdatadog/ci/auto_instrument" if envMap["RUBYOPT"] != expectedValue { @@ -490,7 +490,7 @@ func TestRuby_GetPlatformEnv_SetsRUBYOPT_WhenNotSet(t *testing.T) { } } -func TestRuby_GetPlatformEnv_DoesNotOverride_WhenAlreadySet(t *testing.T) { +func TestRuby_baseEnv_DoesNotOverride_WhenAlreadySet(t *testing.T) { // Set RUBYOPT to a custom value originalValue, existed := os.LookupEnv("RUBYOPT") customValue := "-rbundler/setup -rsome_other_require" @@ -504,9 +504,9 @@ func TestRuby_GetPlatformEnv_DoesNotOverride_WhenAlreadySet(t *testing.T) { }() ruby := newTestRuby() - envMap := ruby.GetPlatformEnv() + envMap := ruby.baseEnv() - // When RUBYOPT is already set, GetPlatformEnv should not include it + // When RUBYOPT is already set, baseEnv should not include it if _, exists := envMap["RUBYOPT"]; exists { t.Error("expected RUBYOPT to not be in envMap when it's already set in environment") } @@ -537,7 +537,7 @@ func TestRuby_DetectFramework_SetsPlatformEnv(t *testing.T) { } // Verify the framework received the correct platform env - frameworkPlatformEnv := fw.GetPlatformEnv() + frameworkPlatformEnv := frameworkRunEnv(t, fw) expectedRubyOpt := "-rbundler/setup -rdatadog/ci/auto_instrument" if frameworkPlatformEnv["RUBYOPT"] != expectedRubyOpt { t.Errorf("expected framework platformEnv RUBYOPT=%q, got %q", expectedRubyOpt, frameworkPlatformEnv["RUBYOPT"]) diff --git a/internal/runner/test_helpers_test.go b/internal/runner/test_helpers_test.go index 6210105b..8dc801e9 100644 --- a/internal/runner/test_helpers_test.go +++ b/internal/runner/test_helpers_test.go @@ -123,12 +123,7 @@ func (m *MockFramework) RunTests(ctx context.Context, testFiles []string, envMap return m.Err } -func (m *MockFramework) SetPlatformEnv(platformEnv map[string]string) { -} - -func (m *MockFramework) GetPlatformEnv() map[string]string { - return nil -} +func (m *MockFramework) Platform() framework.PlatformEnvironment { return nil } func (m *MockFramework) SupportsFullTestDiscovery() bool { return !m.FullDiscoveryUnsupported diff --git a/internal/testdrive/testdrive_test.go b/internal/testdrive/testdrive_test.go index d29caaac..0573987e 100644 --- a/internal/testdrive/testdrive_test.go +++ b/internal/testdrive/testdrive_test.go @@ -21,6 +21,7 @@ import ( "github.com/DataDog/ddtest/internal/framework" "github.com/DataDog/ddtest/internal/platform" + "github.com/DataDog/ddtest/internal/settings" "github.com/DataDog/ddtest/internal/testdrive/intake" ) @@ -121,17 +122,17 @@ func TestPreviewNamesOnlyDiscoveredDependencyFiles(t *testing.T) { files []string want string }{ - {"manifest only", "javascript", framework.NewJest(), []string{"package.json"}, "It will not change package.json."}, - {"npm", "javascript", framework.NewJest(), []string{"package.json", "package-lock.json"}, "It will not change package.json or package-lock.json."}, - {"pnpm with other languages", "javascript", framework.NewJest(), []string{"package.json", "pnpm-lock.yaml", "Gemfile", "pyproject.toml"}, "It will not change package.json or pnpm-lock.yaml."}, - {"yarn", "javascript", framework.NewJest(), []string{"package.json", "yarn.lock"}, "It will not change package.json or yarn.lock."}, - {"bun", "javascript", framework.NewJest(), []string{"package.json", "bun.lock"}, "It will not change package.json or bun.lock."}, - {"uv", "python", framework.NewPytest(), []string{"pyproject.toml", "uv.lock", "package.json"}, "It will not change pyproject.toml or uv.lock."}, - {"pip", "python", framework.NewPytest(), []string{"requirements.txt", "requirements-dev.txt"}, "It will not change requirements-dev.txt or requirements.txt."}, - {"poetry", "python", framework.NewPytest(), []string{"pyproject.toml", "poetry.lock"}, "It will not change pyproject.toml or poetry.lock."}, - {"ruby reused", "ruby", framework.NewRSpec(), []string{"Gemfile", "Gemfile.lock", "package.json"}, "It will not change Gemfile or Gemfile.lock."}, - {"multiple locks", "javascript", framework.NewJest(), []string{"package.json", "package-lock.json", "yarn.lock"}, "It will not change package.json, package-lock.json or yarn.lock."}, - {"no dependency files", "python", framework.NewPytest(), []string{"pytest.ini"}, ""}, + {"manifest only", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json"}, "It will not change package.json."}, + {"npm", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json", "package-lock.json"}, "It will not change package.json or package-lock.json."}, + {"pnpm with other languages", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json", "pnpm-lock.yaml", "Gemfile", "pyproject.toml"}, "It will not change package.json or pnpm-lock.yaml."}, + {"yarn", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json", "yarn.lock"}, "It will not change package.json or yarn.lock."}, + {"bun", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json", "bun.lock"}, "It will not change package.json or bun.lock."}, + {"uv", "python", framework.NewPytest(platform.NewPython()), []string{"pyproject.toml", "uv.lock", "package.json"}, "It will not change pyproject.toml or uv.lock."}, + {"pip", "python", framework.NewPytest(platform.NewPython()), []string{"requirements.txt", "requirements-dev.txt"}, "It will not change requirements-dev.txt or requirements.txt."}, + {"poetry", "python", framework.NewPytest(platform.NewPython()), []string{"pyproject.toml", "poetry.lock"}, "It will not change pyproject.toml or poetry.lock."}, + {"ruby reused", "ruby", framework.NewRSpec(platform.NewRuby(settings.TestSkippingLevelTest)), []string{"Gemfile", "Gemfile.lock", "package.json"}, "It will not change Gemfile or Gemfile.lock."}, + {"multiple locks", "javascript", framework.NewJest(platform.NewJavaScript()), []string{"package.json", "package-lock.json", "yarn.lock"}, "It will not change package.json, package-lock.json or yarn.lock."}, + {"no dependency files", "python", framework.NewPytest(platform.NewPython()), []string{"pytest.ini"}, ""}, } { t.Run(tc.name, func(t *testing.T) { root := filepath.Join(t.TempDir(), "project [space]") @@ -707,7 +708,7 @@ func TestPreviewChoosesTracerBeforeConfirmation(t *testing.T) { func TestEnvironmentAppliesOnlySelectedPlatform(t *testing.T) { for _, language := range []string{"javascript", "python", "ruby"} { - drive := &Testdrive{language: language, framework: framework.NewJest()} + drive := &Testdrive{language: language, framework: framework.NewJest(platform.NewJavaScript())} env := drive.environment("/tracer/init.js", "http://127.0.0.1:1234", "run") if (env["NODE_OPTIONS"] != "") != (language == "javascript") { t.Fatalf("%s NODE_OPTIONS = %q", language, env["NODE_OPTIONS"]) diff --git a/internal/utils/node_options.go b/internal/utils/node_options.go deleted file mode 100644 index 446acb0d..00000000 --- a/internal/utils/node_options.go +++ /dev/null @@ -1,123 +0,0 @@ -package utils - -import "strings" - -type nodeOptionsToken struct { - raw string - value string -} - -// Node uses double quotes, rather than shell quoting, in NODE_OPTIONS. Keep -// each original token so removing a preload does not change other options or -// quoted project loader paths. -func splitNodeOptions(value string) []nodeOptionsToken { - var tokens []nodeOptionsToken - for index := 0; index < len(value); { - for index < len(value) && isNodeOptionsSpace(value[index]) { - index++ - } - if index == len(value) { - break - } - start := index - var decoded strings.Builder - quoted := false - for index < len(value) { - current := value[index] - if current == '"' { - quoted = !quoted - index++ - continue - } - if current == '\\' && index+1 < len(value) && value[index+1] == '"' { - decoded.WriteByte('"') - index += 2 - continue - } - if !quoted && isNodeOptionsSpace(current) { - break - } - decoded.WriteByte(current) - index++ - } - tokens = append(tokens, nodeOptionsToken{raw: value[start:index], value: decoded.String()}) - } - return tokens -} - -func isNodeOptionsSpace(value byte) bool { - return value == ' ' || value == '\t' || value == '\n' || value == '\r' -} - -func nodeOptionsOptionValue(tokens []nodeOptionsToken, index int, option string) (string, int, bool) { - value := tokens[index].value - if value == option || (option == "--require" && value == "-r") { - if index+1 < len(tokens) { - return tokens[index+1].value, 2, true - } - return "", 1, false - } - if strings.HasPrefix(value, option+"=") { - return strings.TrimPrefix(value, option+"="), 1, true - } - if option == "--require" && strings.HasPrefix(value, "-r") && len(value) > 2 { - return strings.TrimPrefix(value, "-r"), 1, true - } - return "", 1, false -} - -func nodeOptionsMatchesModule(value, module string) bool { - if value == module { - return true - } - normalized := strings.ReplaceAll(value, "\\", "/") - return strings.HasSuffix(normalized, "/"+module) || - (module == "dd-trace/ci/init" && (value == module+".js" || strings.HasSuffix(normalized, "/"+module+".js"))) -} - -func findNodeOption(value, option, module string) string { - tokens := splitNodeOptions(value) - for index := 0; index < len(tokens); index++ { - candidate, width, found := nodeOptionsOptionValue(tokens, index, option) - if found && nodeOptionsMatchesModule(candidate, module) { - return candidate - } - index += width - 1 - } - return "" -} - -func NodeOptionsHasRequire(value, module string) bool { - return NodeOptionsRequire(value, module) != "" -} - -func NodeOptionsRequire(value, module string) string { - return findNodeOption(value, "--require", module) -} - -func NodeOptionsHasImport(value, module string) bool { - return findNodeOption(value, "--import", module) != "" -} - -func withoutNodeOption(value, option, module string) string { - tokens := splitNodeOptions(value) - kept := make([]string, 0, len(tokens)) - for index := 0; index < len(tokens); { - candidate, width, found := nodeOptionsOptionValue(tokens, index, option) - if found && nodeOptionsMatchesModule(candidate, module) { - index += width - continue - } - kept = append(kept, tokens[index].raw) - index++ - } - return strings.Join(kept, " ") -} - -func NodeOptionsWithoutRequire(value, module string) string { - return withoutNodeOption(value, "--require", module) -} - -func NodeOptionsWithoutImport(value, module string) string { - return withoutNodeOption(value, "--import", module) -} diff --git a/internal/utils/ruby.go b/internal/utils/ruby.go deleted file mode 100644 index c0b7bb59..00000000 --- a/internal/utils/ruby.go +++ /dev/null @@ -1,70 +0,0 @@ -package utils - -import ( - "context" - "fmt" - "regexp" - "strings" - - "github.com/DataDog/ddtest/internal/ext" - "github.com/DataDog/ddtest/internal/version" -) - -const ( - RubyTracerGemName = "datadog-ci" - rubyTracerMinVersion = "1.31.0" -) - -// CheckRubyTracer checks the prerequisite shared by Ruby execution and full discovery. -func CheckRubyTracer(ctx context.Context, executor ext.CommandExecutor) error { - gemVersion, err := DetectRubyTracer(ctx, executor) - if err != nil { - return err - } - requiredVersion, err := version.Parse(rubyTracerMinVersion) - if err != nil { - return err - } - if gemVersion.Compare(requiredVersion) < 0 { - return fmt.Errorf("datadog-ci gem version %s is lower than required >= %s", gemVersion.String(), requiredVersion.String()) - } - return nil -} - -// DetectRubyTracer reads the tracer version from the project's bundle. -func DetectRubyTracer(ctx context.Context, executor ext.CommandExecutor) (version.Version, error) { - // Inherit project loaders without adding the instrumentation preload. - output, err := executor.CombinedOutput(ctx, "bundle", []string{"info", RubyTracerGemName}, nil) - if err != nil { - return version.Version{}, fmt.Errorf("detect project tracer: %s: %w", strings.TrimSpace(string(output)), err) - } - return parseBundlerInfoVersion(string(output), RubyTracerGemName) -} - -// bundlerInfoRegex matches bundler info output format: " * gem-name (version [hash])" -// Captures: 1=gem-name, 2=version -var bundlerInfoRegex = regexp.MustCompile(`^\s*\*\s+(\S+)\s+\((\d+\.\d+\.\d+)`) - -func parseBundlerInfoVersion(output, gemName string) (version.Version, error) { - for line := range strings.SplitSeq(output, "\n") { - matches := bundlerInfoRegex.FindStringSubmatch(line) - if matches == nil { - continue - } - - matchedGem := matches[1] - if matchedGem != gemName { - continue - } - - versionString := matches[2] - parsed, err := version.Parse(versionString) - if err != nil { - return version.Version{}, fmt.Errorf("failed to parse version from bundle info output: %w", err) - } - - return parsed, nil - } - - return version.Version{}, fmt.Errorf("unable to find datadog-ci gem version in bundle info output") -}