-
Notifications
You must be signed in to change notification settings - Fork 4.8k
TRT-2394: panic when duplicate tests are detected #30498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,6 +158,71 @@ func (o *GinkgoRunSuiteOptions) SetIOStreams(streams genericclioptions.IOStreams | |
| o.IOStreams = streams | ||
| } | ||
|
|
||
| // 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 { | ||
| testOccurrences[spec.Name] = append(testOccurrences[spec.Name], spec) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about checking the length here and anytime it is > 1 add it to map[string][]*extensions.ExtensionTestSpec duplicates to track just the known duplicates. Then in the loop below you can just process the duplicates and not iterate over the full list of tests again? |
||
| } | ||
|
|
||
| // Collect all duplicate tests | ||
| var duplicates []string | ||
| var allDuplicateDetails []string | ||
|
|
||
| for testName, occurrences := range testOccurrences { | ||
| if len(occurrences) <= 1 { | ||
| continue // Not a duplicate | ||
| } | ||
|
|
||
| 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 { | ||
| if a > b { | ||
| return a | ||
|
|
@@ -243,6 +308,8 @@ func (o *GinkgoRunSuiteOptions) Run(suite *TestSuite, clusterConfig *clusterdisc | |
| return fmt.Errorf("no tests to run") | ||
| } | ||
|
|
||
| duplicateTestCases := detectDuplicateTests(specs) | ||
|
|
||
| k8sTestNames := map[string]bool{} | ||
| for _, t := range specs { | ||
| if strings.Contains(t.ExtensionTestSpec.Source, "hyperkube") { | ||
|
|
@@ -583,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")) | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,195 @@ | ||
| 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) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use [sig-trt] annotation here?