Merge pull request #15 from DataDog/anmarchenko/test_discovery_format_changes

anmarchenko · web-flow · commit 8b18a644c10f · 2025-10-15T16:09:28.000+02:00
[SDTEST-2709] Tests discovery: change format of tests.json file
diff --git a/internal/framework/rspec.go b/internal/framework/rspec.go
@@ -46,7 +46,6 @@ func (r *RSpec) createDiscoveryCommand() *exec.Cmd {
 	command, baseArgs := r.getRSpecCommand()
 	args := append(baseArgs, "--format", "progress", "--dry-run")
 
-	// this is a constant, no
 	// no-dd-sa:go-security/command-injection
 	cmd := exec.Command(command, args...)
 	cmd.Env = append(
diff --git a/internal/framework/rspec_test.go b/internal/framework/rspec_test.go
@@ -194,17 +194,17 @@ func TestRSpec_DiscoverTests_Success(t *testing.T) {
 
 	testData := []testoptimization.Test{
 		{
-			FQN:             "spec/models/user_spec.rb[1:1]",
 			Name:            "User should be valid",
 			Suite:           "User",
-			SourceFile:      "spec/models/user_spec.rb",
+			Module:          "rspec",
+			Parameters:      "{a: 1}",
 			SuiteSourceFile: "spec/models/user_spec.rb",
 		},
 		{
-			FQN:             "spec/controllers/users_controller_spec.rb[1:1]",
 			Name:            "UsersController GET index should return success",
 			Suite:           "UsersController",
-			SourceFile:      "spec/controllers/users_controller_spec.rb",
+			Module:          "rspec",
+			Parameters:      "{a: 2}",
 			SuiteSourceFile: "spec/controllers/users_controller_spec.rb",
 		},
 	}
@@ -259,17 +259,20 @@ func TestRSpec_DiscoverTests_Success(t *testing.T) {
 			continue
 		}
 		actual := tests[i]
-		if actual.FQN != expected.FQN {
-			t.Errorf("test[%d].FQN: expected %q, got %q", i, expected.FQN, actual.FQN)
+		if actual.Parameters != expected.Parameters {
+			t.Errorf("test[%d].Parameters: expected %q, got %q", i, expected.Parameters, actual.Parameters)
 		}
 		if actual.Name != expected.Name {
 			t.Errorf("test[%d].Name: expected %q, got %q", i, expected.Name, actual.Name)
 		}
 		if actual.Suite != expected.Suite {
 			t.Errorf("test[%d].Suite: expected %q, got %q", i, expected.Suite, actual.Suite)
 		}
-		if actual.SourceFile != expected.SourceFile {
-			t.Errorf("test[%d].SourceFile: expected %q, got %q", i, expected.SourceFile, actual.SourceFile)
+		if actual.Module != expected.Module {
+			t.Errorf("test[%d].Module: expected %q, got %q", i, expected.Module, actual.Module)
+		}
+		if actual.SuiteSourceFile != expected.SuiteSourceFile {
+			t.Errorf("test[%d].SuiteSourceFile: expected %q, got %q", i, expected.SuiteSourceFile, actual.SuiteSourceFile)
 		}
 	}
 }
diff --git a/internal/runner/dd_test_optimization.go b/internal/runner/dd_test_optimization.go
@@ -66,8 +66,8 @@ func (tr *TestRunner) PrepareTestOptimization(ctx context.Context) error {
 
 	tr.testFiles = make(map[string]int)
 	for _, test := range discoveredTests {
-		if !skippableTests[test.FQN] {
-			slog.Debug("Test is not skipped", "test", test.FQN, "sourceFile", test.SuiteSourceFile)
+		if !skippableTests[test.FQN()] {
+			slog.Debug("Test is not skipped", "test", test.FQN(), "sourceFile", test.SuiteSourceFile)
 			if test.SuiteSourceFile != "" {
 				tr.testFiles[test.SuiteSourceFile]++
 			}
diff --git a/internal/runner/dd_test_optimization_test.go b/internal/runner/dd_test_optimization_test.go
@@ -16,10 +16,10 @@ func TestTestRunner_PrepareTestOptimization_Success(t *testing.T) {
 	mockFramework := &MockFramework{
 		FrameworkName: "rspec",
 		Tests: []testoptimization.Test{
-			{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-			{FQN: "TestSuite1.test2", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-			{FQN: "TestSuite2.test3", SourceFile: "test/file2_test.rb", SuiteSourceFile: "test/file2_test.rb"},
-			{FQN: "TestSuite3.test4", SourceFile: "test/file3_test.rb", SuiteSourceFile: "test/file3_test.rb"},
+			{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite1", Name: "test2", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite2", Name: "test3", Parameters: "", SuiteSourceFile: "test/file2_test.rb"},
+			{Suite: "TestSuite3", Name: "test4", Parameters: "", SuiteSourceFile: "test/file3_test.rb"},
 		},
 	}
 
@@ -38,8 +38,8 @@ func TestTestRunner_PrepareTestOptimization_Success(t *testing.T) {
 
 	mockOptimizationClient := &MockTestOptimizationClient{
 		SkippableTests: map[string]bool{
-			"TestSuite1.test2": true, // Skip test2
-			"TestSuite3.test4": true, // Skip test4
+			"TestSuite1.test2.": true, // Skip test2
+			"TestSuite3.test4.": true, // Skip test4
 		},
 	}
 
@@ -145,7 +145,7 @@ func TestTestRunner_PrepareTestOptimization_OptimizationClientInitError(t *testi
 
 	mockFramework := &MockFramework{
 		Tests: []testoptimization.Test{
-			{FQN: "test1", SourceFile: "file1.rb", SuiteSourceFile: "file1.rb"},
+			{Suite: "", Name: "test1", Parameters: "", SuiteSourceFile: "file1.rb"},
 		},
 	}
 
@@ -274,8 +274,8 @@ func TestTestRunner_PrepareTestOptimization_AllTestsSkipped(t *testing.T) {
 
 	mockFramework := &MockFramework{
 		Tests: []testoptimization.Test{
-			{FQN: "test1", SourceFile: "file1.rb", SuiteSourceFile: "file1.rb"},
-			{FQN: "test2", SourceFile: "file2.rb", SuiteSourceFile: "file2.rb"},
+			{Suite: "", Name: "test1", Parameters: "", SuiteSourceFile: "file1.rb"},
+			{Suite: "", Name: "test2", Parameters: "", SuiteSourceFile: "file2.rb"},
 		},
 	}
 
@@ -287,8 +287,8 @@ func TestTestRunner_PrepareTestOptimization_AllTestsSkipped(t *testing.T) {
 	mockPlatformDetector := &MockPlatformDetector{Platform: mockPlatform}
 	mockOptimizationClient := &MockTestOptimizationClient{
 		SkippableTests: map[string]bool{
-			"test1": true,
-			"test2": true,
+			".test1.": true,
+			".test2.": true,
 		},
 	}
 
diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go
@@ -222,11 +222,11 @@ func TestTestRunner_Setup_WithParallelRunners(t *testing.T) {
 	mockFramework := &MockFramework{
 		FrameworkName: "rspec",
 		Tests: []testoptimization.Test{
-			{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-			{FQN: "TestSuite1.test2", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-			{FQN: "TestSuite2.test3", SourceFile: "test/file2_test.rb", SuiteSourceFile: "test/file2_test.rb"},
-			{FQN: "TestSuite3.test4", SourceFile: "test/file3_test.rb", SuiteSourceFile: "test/file3_test.rb"},
-			{FQN: "TestSuite4.test5", SourceFile: "test/file4_test.rb", SuiteSourceFile: "test/file4_test.rb"},
+			{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite1", Name: "test2", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite2", Name: "test3", Parameters: "", SuiteSourceFile: "test/file2_test.rb"},
+			{Suite: "TestSuite3", Name: "test4", Parameters: "", SuiteSourceFile: "test/file3_test.rb"},
+			{Suite: "TestSuite4", Name: "test5", Parameters: "", SuiteSourceFile: "test/file4_test.rb"},
 		},
 	}
 
@@ -239,8 +239,8 @@ func TestTestRunner_Setup_WithParallelRunners(t *testing.T) {
 	mockPlatformDetector := &MockPlatformDetector{Platform: mockPlatform}
 	mockOptimizationClient := &MockTestOptimizationClient{
 		SkippableTests: map[string]bool{
-			"TestSuite1.test2": true, // Skip test2
-			"TestSuite4.test5": true, // Skip test5
+			"TestSuite1.test2.": true, // Skip test2
+			"TestSuite4.test5.": true, // Skip test5
 		},
 	}
 
@@ -279,8 +279,8 @@ func TestTestRunner_Setup_WithCIProvider(t *testing.T) {
 	mockFramework := &MockFramework{
 		FrameworkName: "rspec",
 		Tests: []testoptimization.Test{
-			{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-			{FQN: "TestSuite2.test2", SourceFile: "test/file2_test.rb", SuiteSourceFile: "test/file2_test.rb"},
+			{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite2", Name: "test2", Parameters: "", SuiteSourceFile: "test/file2_test.rb"},
 		},
 	}
 
@@ -341,7 +341,7 @@ func TestTestRunner_Setup_CIProviderDetectionFailure(t *testing.T) {
 	mockFramework := &MockFramework{
 		FrameworkName: "rspec",
 		Tests: []testoptimization.Test{
-			{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
 		},
 	}
 
@@ -382,7 +382,7 @@ func TestTestRunner_Setup_CIProviderConfigureFailure(t *testing.T) {
 	mockFramework := &MockFramework{
 		FrameworkName: "rspec",
 		Tests: []testoptimization.Test{
-			{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
+			{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
 		},
 	}
 
@@ -435,8 +435,8 @@ func TestTestRunner_Setup_WithTestSplit(t *testing.T) {
 		mockFramework := &MockFramework{
 			FrameworkName: "rspec",
 			Tests: []testoptimization.Test{
-				{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-				{FQN: "TestSuite2.test2", SourceFile: "test/file2_test.rb", SuiteSourceFile: "test/file2_test.rb"},
+				{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+				{Suite: "TestSuite2", Name: "test2", Parameters: "", SuiteSourceFile: "test/file2_test.rb"},
 			},
 		}
 
@@ -509,10 +509,10 @@ func TestTestRunner_Setup_WithTestSplit(t *testing.T) {
 		mockFramework := &MockFramework{
 			FrameworkName: "rspec",
 			Tests: []testoptimization.Test{
-				{FQN: "TestSuite1.test1", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"},
-				{FQN: "TestSuite1.test2", SourceFile: "test/file1_test.rb", SuiteSourceFile: "test/file1_test.rb"}, // 2 tests in file1
-				{FQN: "TestSuite2.test3", SourceFile: "test/file2_test.rb", SuiteSourceFile: "test/file2_test.rb"}, // 1 test in file2
-				{FQN: "TestSuite3.test4", SourceFile: "test/file3_test.rb", SuiteSourceFile: "test/file3_test.rb"}, // 1 test in file3
+				{Suite: "TestSuite1", Name: "test1", Parameters: "", SuiteSourceFile: "test/file1_test.rb"},
+				{Suite: "TestSuite1", Name: "test2", Parameters: "", SuiteSourceFile: "test/file1_test.rb"}, // 2 tests in file1
+				{Suite: "TestSuite2", Name: "test3", Parameters: "", SuiteSourceFile: "test/file2_test.rb"}, // 1 test in file2
+				{Suite: "TestSuite3", Name: "test4", Parameters: "", SuiteSourceFile: "test/file3_test.rb"}, // 1 test in file3
 			},
 		}
 
diff --git a/internal/testoptimization/client.go b/internal/testoptimization/client.go
@@ -119,8 +119,12 @@ func (c *DatadogClient) GetSkippableTests() map[string]bool {
 	for _, suites := range skippableTests {
 		for _, tests := range suites {
 			for _, test := range tests {
-				testFQN := c.buildTestFQN(test.Suite, test.Name, test.Parameters)
-				skippedTests[testFQN] = true
+				t := Test{
+					Name:       test.Name,
+					Suite:      test.Suite,
+					Parameters: test.Parameters,
+				}
+				skippedTests[t.FQN()] = true
 			}
 		}
 	}
@@ -167,7 +171,3 @@ func (c *DatadogClient) StoreCacheAndExit() {
 
 	c.integrations.ExitCiVisibility()
 }
-
-func (c *DatadogClient) buildTestFQN(suite, test, parameters string) string {
-	return fmt.Sprintf("%s.%s.%s", suite, test, parameters)
-}
diff --git a/internal/testoptimization/client_test.go b/internal/testoptimization/client_test.go
@@ -277,9 +277,7 @@ func TestDatadogClient_StoreCacheAndExit_WritesSettingsFile(t *testing.T) {
 	}
 }
 
-func TestDatadogClient_buildTestFQN(t *testing.T) {
-	client := NewDatadogClient()
-
+func TestTest_FQN(t *testing.T) {
 	testCases := []struct {
 		suite      string
 		test       string
@@ -293,9 +291,14 @@ func TestDatadogClient_buildTestFQN(t *testing.T) {
 	}
 
 	for _, tc := range testCases {
-		result := client.buildTestFQN(tc.suite, tc.test, tc.parameters)
+		test := Test{
+			Suite:      tc.suite,
+			Name:       tc.test,
+			Parameters: tc.parameters,
+		}
+		result := test.FQN()
 		if result != tc.expected {
-			t.Errorf("buildTestFQN(%q, %q, %q) = %q, expected %q",
+			t.Errorf("Test{Suite: %q, Name: %q, Parameters: %q}.FQN() = %q, expected %q",
 				tc.suite, tc.test, tc.parameters, result, tc.expected)
 		}
 	}
diff --git a/internal/testoptimization/types.go b/internal/testoptimization/types.go
@@ -1,9 +1,16 @@
 package testoptimization
 
+import "fmt"
+
 type Test struct {
-	FQN             string `json:"fqn"`
 	Name            string `json:"name"`
 	Suite           string `json:"suite"`
-	SourceFile      string `json:"sourceFile"`
+	Module          string `json:"module"`
+	Parameters      string `json:"parameters"`
 	SuiteSourceFile string `json:"suiteSourceFile"`
 }
+
+// FQN returns the fully qualified name of the test
+func (t *Test) FQN() string {
+	return fmt.Sprintf("%s.%s.%s", t.Suite, t.Name, t.Parameters)
+}

Original file line number	Diff line number	Diff line change
`@@ -194,17 +194,17 @@ func TestRSpec_DiscoverTests_Success(t *testing.T) {`
`194`	`194`
`195`	`195`	`testData := []testoptimization.Test{`
`196`	`196`	`{`
`197`		`- FQN: "spec/models/user_spec.rb[1:1]",`
`198`	`197`	`Name: "User should be valid",`
`199`	`198`	`Suite: "User",`
`200`		`- SourceFile: "spec/models/user_spec.rb",`
	`199`	`+ Module: "rspec",`
	`200`	`+ Parameters: "{a: 1}",`
`201`	`201`	`SuiteSourceFile: "spec/models/user_spec.rb",`
`202`	`202`	`},`
`203`	`203`	`{`
`204`		`- FQN: "spec/controllers/users_controller_spec.rb[1:1]",`
`205`	`204`	`Name: "UsersController GET index should return success",`
`206`	`205`	`Suite: "UsersController",`
`207`		`- SourceFile: "spec/controllers/users_controller_spec.rb",`
	`206`	`+ Module: "rspec",`
	`207`	`+ Parameters: "{a: 2}",`
`208`	`208`	`SuiteSourceFile: "spec/controllers/users_controller_spec.rb",`
`209`	`209`	`},`
`210`	`210`	`}`
`@@ -259,17 +259,20 @@ func TestRSpec_DiscoverTests_Success(t *testing.T) {`
`259`	`259`	`continue`
`260`	`260`	`}`
`261`	`261`	`actual := tests[i]`
`262`		`- if actual.FQN != expected.FQN {`
`263`		`- t.Errorf("test[%d].FQN: expected %q, got %q", i, expected.FQN, actual.FQN)`
	`262`	`+ if actual.Parameters != expected.Parameters {`
	`263`	`+ t.Errorf("test[%d].Parameters: expected %q, got %q", i, expected.Parameters, actual.Parameters)`
`264`	`264`	`}`
`265`	`265`	`if actual.Name != expected.Name {`
`266`	`266`	`t.Errorf("test[%d].Name: expected %q, got %q", i, expected.Name, actual.Name)`
`267`	`267`	`}`
`268`	`268`	`if actual.Suite != expected.Suite {`
`269`	`269`	`t.Errorf("test[%d].Suite: expected %q, got %q", i, expected.Suite, actual.Suite)`
`270`	`270`	`}`
`271`		`- if actual.SourceFile != expected.SourceFile {`
`272`		`- t.Errorf("test[%d].SourceFile: expected %q, got %q", i, expected.SourceFile, actual.SourceFile)`
	`271`	`+ if actual.Module != expected.Module {`
	`272`	`+ t.Errorf("test[%d].Module: expected %q, got %q", i, expected.Module, actual.Module)`
	`273`	`+ }`
	`274`	`+ if actual.SuiteSourceFile != expected.SuiteSourceFile {`
	`275`	`+ t.Errorf("test[%d].SuiteSourceFile: expected %q, got %q", i, expected.SuiteSourceFile, actual.SuiteSourceFile)`
`273`	`276`	`}`
`274`	`277`	`}`
`275`	`278`	`}`
Original file line number	Diff line number	Diff line change
`@@ -119,8 +119,12 @@ func (c *DatadogClient) GetSkippableTests() map[string]bool {`
`119`	`119`	`for _, suites := range skippableTests {`
`120`	`120`	`for _, tests := range suites {`
`121`	`121`	`for _, test := range tests {`
`122`		`- testFQN := c.buildTestFQN(test.Suite, test.Name, test.Parameters)`
`123`		`- skippedTests[testFQN] = true`
	`122`	`+ t := Test{`
	`123`	`+ Name: test.Name,`
	`124`	`+ Suite: test.Suite,`
	`125`	`+ Parameters: test.Parameters,`
	`126`	`+ }`
	`127`	`+ skippedTests[t.FQN()] = true`
`124`	`128`	`}`
`125`	`129`	`}`
`126`	`130`	`}`
`@@ -167,7 +171,3 @@ func (c *DatadogClient) StoreCacheAndExit() {`
`167`	`171`
`168`	`172`	`c.integrations.ExitCiVisibility()`
`169`	`173`	`}`
`170`		`-`
`171`		`-func (c *DatadogClient) buildTestFQN(suite, test, parameters string) string {`
`172`		`- return fmt.Sprintf("%s.%s.%s", suite, test, parameters)`
`173`		`-}`
Original file line number	Diff line number	Diff line change
`@@ -277,9 +277,7 @@ func TestDatadogClient_StoreCacheAndExit_WritesSettingsFile(t *testing.T) {`
`277`	`277`	`}`
`278`	`278`	`}`
`279`	`279`
`280`		`-func TestDatadogClient_buildTestFQN(t *testing.T) {`
`281`		`- client := NewDatadogClient()`
`282`		`-`
	`280`	`+func TestTest_FQN(t *testing.T) {`
`283`	`281`	`testCases := []struct {`
`284`	`282`	`suite string`
`285`	`283`	`test string`
`@@ -293,9 +291,14 @@ func TestDatadogClient_buildTestFQN(t *testing.T) {`
`293`	`291`	`}`
`294`	`292`
`295`	`293`	`for _, tc := range testCases {`
`296`		`- result := client.buildTestFQN(tc.suite, tc.test, tc.parameters)`
	`294`	`+ test := Test{`
	`295`	`+ Suite: tc.suite,`
	`296`	`+ Name: tc.test,`
	`297`	`+ Parameters: tc.parameters,`
	`298`	`+ }`
	`299`	`+ result := test.FQN()`
`297`	`300`	`if result != tc.expected {`
`298`		`- t.Errorf("buildTestFQN(%q, %q, %q) = %q, expected %q",`
	`301`	`+ t.Errorf("Test{Suite: %q, Name: %q, Parameters: %q}.FQN() = %q, expected %q",`
`299`	`302`	`tc.suite, tc.test, tc.parameters, result, tc.expected)`
`300`	`303`	`}`
`301`	`304`	`}`