From ba665cee1764e231e55fde53b46896117698b1e5 Mon Sep 17 00:00:00 2001 From: xueqzhan Date: Tue, 18 Nov 2025 11:19:42 -0500 Subject: [PATCH 1/3] panic when duplicate tests are detected --- pkg/test/ginkgo/cmd_runsuite.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/pkg/test/ginkgo/cmd_runsuite.go b/pkg/test/ginkgo/cmd_runsuite.go index c9b3a652a027..d830463954c9 100644 --- a/pkg/test/ginkgo/cmd_runsuite.go +++ b/pkg/test/ginkgo/cmd_runsuite.go @@ -158,6 +158,17 @@ func (o *GinkgoRunSuiteOptions) SetIOStreams(streams genericclioptions.IOStreams o.IOStreams = streams } +// detectDuplicateTests panics if any test names appear more than once +func detectDuplicateTests(specs extensions.ExtensionTestSpecs) { + seen := make(map[string]bool) + for _, spec := range specs { + if seen[spec.Name] { + panic(fmt.Sprintf("duplicate test detected: %q", spec.Name)) + } + seen[spec.Name] = true + } +} + func max(a, b int) int { if a > b { return a @@ -243,6 +254,8 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc return fmt.Errorf("no tests to run") } + detectDuplicateTests(specs) + k8sTestNames := map[string]bool{} for _, t := range specs { if strings.Contains(t.ExtensionTestSpec.Source, "hyperkube") { From d4820b0007ebb01b41cdde875b323e0f52a4f244 Mon Sep 17 00:00:00 2001 From: xueqzhan Date: Wed, 19 Nov 2025 14:27:52 -0500 Subject: [PATCH 2/3] Create junit for duplicate tests instead of panic for now --- pkg/test/ginkgo/cmd_runsuite.go | 69 +++++++++- pkg/test/ginkgo/cmd_runsuite_test.go | 196 +++++++++++++++++++++++++++ 2 files changed, 258 insertions(+), 7 deletions(-) create mode 100644 pkg/test/ginkgo/cmd_runsuite_test.go diff --git a/pkg/test/ginkgo/cmd_runsuite.go b/pkg/test/ginkgo/cmd_runsuite.go index d830463954c9..c5aee70b7e82 100644 --- a/pkg/test/ginkgo/cmd_runsuite.go +++ b/pkg/test/ginkgo/cmd_runsuite.go @@ -158,15 +158,69 @@ func (o *GinkgoRunSuiteOptions) SetIOStreams(streams genericclioptions.IOStreams o.IOStreams = streams } -// detectDuplicateTests panics if any test names appear more than once -func detectDuplicateTests(specs extensions.ExtensionTestSpecs) { - seen := make(map[string]bool) +// detectDuplicateTests creates test cases to report on duplicate test detection +// Flake for now. +func detectDuplicateTests(specs extensions.ExtensionTestSpecs) []*junitapi.JUnitTestCase { + const duplicateTestName = "There should not be duplicate tests" + + testOccurrences := make(map[string][]*extensions.ExtensionTestSpec) + + // Track all occurrences of each test name for _, spec := range specs { - if seen[spec.Name] { - panic(fmt.Sprintf("duplicate test detected: %q", spec.Name)) + testOccurrences[spec.Name] = append(testOccurrences[spec.Name], spec) + } + + // Collect all duplicate tests + var duplicates []string + var allDuplicateDetails []string + + for testName, occurrences := range testOccurrences { + if len(occurrences) <= 1 { + continue // Not a duplicate } - seen[spec.Name] = true + + duplicates = append(duplicates, testName) + logrus.Warnf("Duplicate test detected: %q appears %d times", testName, len(occurrences)) + + // Collect source information for all occurrences of this test + testDetails := fmt.Sprintf("\n=== Duplicate Test: %s ===", testName) + testDetails += fmt.Sprintf("\nFound %d occurrences:", len(occurrences)) + + for i, spec := range occurrences { + detail := fmt.Sprintf("\n Occurrence %d:", i+1) + detail += fmt.Sprintf("\n Source: %s", spec.ExtensionTestSpec.Source) + if len(spec.ExtensionTestSpec.CodeLocations) > 0 { + detail += fmt.Sprintf("\n CodeLocations: %s", strings.Join(spec.ExtensionTestSpec.CodeLocations, ", ")) + } + testDetails += detail + logrus.Warnf(" Occurrence %d: Source=%s", i+1, spec.ExtensionTestSpec.Source) + } + + allDuplicateDetails = append(allDuplicateDetails, testDetails) + } + + // Always create a success test case first + testCases := []*junitapi.JUnitTestCase{ + { + Name: duplicateTestName, + }, } + + // If duplicates found, also create a failure case to make it a flake + if len(duplicates) > 0 { + totalDuplicates := len(duplicates) + logrus.Warnf("Found %d duplicate tests total", totalDuplicates) + fullDetails := strings.Join(allDuplicateDetails, "\n") + + testCases = append(testCases, &junitapi.JUnitTestCase{ + Name: duplicateTestName, + FailureOutput: &junitapi.FailureOutput{ + Output: fmt.Sprintf("Found %d duplicate tests:\n%s\n%s\n", totalDuplicates, strings.Join(duplicates, "\n"), fullDetails), + }, + }) + } + + return testCases } func max(a, b int) int { @@ -254,7 +308,7 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc return fmt.Errorf("no tests to run") } - detectDuplicateTests(specs) + duplicateTestCases := detectDuplicateTests(specs) k8sTestNames := map[string]bool{} for _, t := range specs { @@ -596,6 +650,7 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc var syntheticTestResults []*junitapi.JUnitTestCase var syntheticFailure bool syntheticTestResults = append(syntheticTestResults, stableClusterTestResults...) + syntheticTestResults = append(syntheticTestResults, duplicateTestCases...) timeSuffix := fmt.Sprintf("_%s", start.UTC().Format("20060102-150405")) diff --git a/pkg/test/ginkgo/cmd_runsuite_test.go b/pkg/test/ginkgo/cmd_runsuite_test.go new file mode 100644 index 000000000000..d33b3927280e --- /dev/null +++ b/pkg/test/ginkgo/cmd_runsuite_test.go @@ -0,0 +1,196 @@ +package ginkgo + +import ( + "strings" + "testing" + + "github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests" + "github.com/openshift/origin/pkg/test/extensions" +) + +func Test_detectDuplicateTests(t *testing.T) { + tests := []struct { + name string + specs extensions.ExtensionTestSpecs + wantTestCaseCount int + wantSuccess bool + wantFailure bool + wantDuplicateCount int + wantFailureContains []string + }{ + { + name: "no duplicates", + specs: extensions.ExtensionTestSpecs{ + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-1", + Source: "source-1", + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-2", + Source: "source-2", + }, + }, + }, + wantTestCaseCount: 1, + wantSuccess: true, + wantFailure: false, + }, + { + name: "single duplicate test", + specs: extensions.ExtensionTestSpecs{ + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "duplicate-test", + Source: "source-1", + CodeLocations: []string{"file1.go:10"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "duplicate-test", + Source: "source-2", + CodeLocations: []string{"file2.go:20"}, + }, + }, + }, + wantTestCaseCount: 2, + wantSuccess: true, + wantFailure: true, + wantDuplicateCount: 1, + wantFailureContains: []string{ + "Found 1 duplicate tests", + "duplicate-test", + "=== Duplicate Test: duplicate-test ===", + "Found 2 occurrences:", + "Occurrence 1:", + "Source: source-1", + "CodeLocations: file1.go:10", + "Occurrence 2:", + "Source: source-2", + "CodeLocations: file2.go:20", + }, + }, + { + name: "multiple duplicate tests", + specs: extensions.ExtensionTestSpecs{ + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-a", + Source: "source-a1", + CodeLocations: []string{"file-a1.go:100"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-a", + Source: "source-a2", + CodeLocations: []string{"file-a2.go:200"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-b", + Source: "source-b1", + CodeLocations: []string{"file-b1.go:300"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-b", + Source: "source-b2", + CodeLocations: []string{"file-b2.go:400"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "test-b", + Source: "source-b3", + CodeLocations: []string{"file-b3.go:500"}, + }, + }, + { + ExtensionTestSpec: &extensiontests.ExtensionTestSpec{ + Name: "unique-test", + Source: "source-unique", + }, + }, + }, + wantTestCaseCount: 2, + wantSuccess: true, + wantFailure: true, + wantDuplicateCount: 2, + wantFailureContains: []string{ + "Found 2 duplicate tests", + "test-a", + "test-b", + "=== Duplicate Test: test-a ===", + "Found 2 occurrences:", + "=== Duplicate Test: test-b ===", + "Found 3 occurrences:", + "source-a1", + "source-a2", + "source-b1", + "source-b2", + "source-b3", + "file-a1.go:100", + "file-a2.go:200", + "file-b1.go:300", + "file-b2.go:400", + "file-b3.go:500", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testCases := detectDuplicateTests(tt.specs) + + // Check test case count + if len(testCases) != tt.wantTestCaseCount { + t.Errorf("detectDuplicateTests() returned %d test cases, want %d", len(testCases), tt.wantTestCaseCount) + } + + // Check for success test case + hasSuccess := false + hasFailure := false + var failureOutput string + + for _, tc := range testCases { + if tc.Name != "There should not be duplicate tests" { + t.Errorf("test case has wrong name: %q", tc.Name) + } + + if tc.FailureOutput == nil { + hasSuccess = true + } else { + hasFailure = true + failureOutput = tc.FailureOutput.Output + } + } + + if hasSuccess != tt.wantSuccess { + t.Errorf("detectDuplicateTests() hasSuccess = %v, want %v", hasSuccess, tt.wantSuccess) + } + + if hasFailure != tt.wantFailure { + t.Errorf("detectDuplicateTests() hasFailure = %v, want %v", hasFailure, tt.wantFailure) + } + + // If we expect failure, check the failure output content + if tt.wantFailure && hasFailure { + for _, want := range tt.wantFailureContains { + if !strings.Contains(failureOutput, want) { + t.Errorf("failure output missing expected string %q\nGot:\n%s", want, failureOutput) + } + } + + // Print the actual failure output for review + t.Logf("Failure output:\n%s", failureOutput) + } + }) + } +} + From e54583b2cb4e06615e9cf45466a196178af07bf8 Mon Sep 17 00:00:00 2001 From: xueqzhan Date: Wed, 19 Nov 2025 15:33:29 -0500 Subject: [PATCH 3/3] gofmt error --- pkg/test/ginkgo/cmd_runsuite_test.go | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/pkg/test/ginkgo/cmd_runsuite_test.go b/pkg/test/ginkgo/cmd_runsuite_test.go index d33b3927280e..bccd4b6ea36b 100644 --- a/pkg/test/ginkgo/cmd_runsuite_test.go +++ b/pkg/test/ginkgo/cmd_runsuite_test.go @@ -10,13 +10,13 @@ import ( func Test_detectDuplicateTests(t *testing.T) { tests := []struct { - name string - specs extensions.ExtensionTestSpecs - wantTestCaseCount int - wantSuccess bool - wantFailure bool - wantDuplicateCount int - wantFailureContains []string + name string + specs extensions.ExtensionTestSpecs + wantTestCaseCount int + wantSuccess bool + wantFailure bool + wantDuplicateCount int + wantFailureContains []string }{ { name: "no duplicates", @@ -193,4 +193,3 @@ func Test_detectDuplicateTests(t *testing.T) { }) } } -