From c902705462f68395a53c34cc0d71845a59feb519 Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Tue, 8 Oct 2024 18:36:44 +0200 Subject: [PATCH] internal/civisibility: fix subtest unique naming on retries. --- .../integrations/gotesting/instrumentation.go | 6 +++ .../integrations/gotesting/reflections.go | 39 +++++++++++++++++++ .../gotesting/testcontroller_test.go | 14 +++---- 3 files changed, 52 insertions(+), 7 deletions(-) diff --git a/internal/civisibility/integrations/gotesting/instrumentation.go b/internal/civisibility/integrations/gotesting/instrumentation.go index 5251306ed8..5f0abf98ea 100644 --- a/internal/civisibility/integrations/gotesting/instrumentation.go +++ b/internal/civisibility/integrations/gotesting/instrumentation.go @@ -516,6 +516,9 @@ func applyAdditionalFeaturesToTestFunc(f func(*testing.T), testInfo *commonInfo) originalExecMeta := getTestMetadata(t) for { + // let's clear the matcher subnames map before any execution to avoid subname tests to be called "parent/subname#NN" due the retries + getTestContextMatcherPrivateFields(t).ClearSubNames() + // increment execution index executionIndex++ @@ -683,6 +686,9 @@ func applyAdditionalFeaturesToTestFunc(f func(*testing.T), testInfo *commonInfo) var suite integrations.DdTestSuite for { + // let's clear the matcher subnames map before any execution to avoid subname tests to be called "parent/subname#NN" due the retries + getTestContextMatcherPrivateFields(t).ClearSubNames() + // increment execution index executionIndex++ diff --git a/internal/civisibility/integrations/gotesting/reflections.go b/internal/civisibility/integrations/gotesting/reflections.go index c01fc4f5f5..745aab1468 100644 --- a/internal/civisibility/integrations/gotesting/reflections.go +++ b/internal/civisibility/integrations/gotesting/reflections.go @@ -157,6 +157,45 @@ func getTestParentPrivateFields(t *testing.T) *commonPrivateFields { return nil } +// contextMatcher is collection of required private fields from testing.context.match +type contextMatcher struct { + mu *sync.RWMutex + subNames *map[string]int32 +} + +// ClearSubNames clears the subname map used for creating unique names for subtests +func (c *contextMatcher) ClearSubNames() { + c.mu.Lock() + defer c.mu.Unlock() + *c.subNames = map[string]int32{} +} + +// getTestContextMatcherPrivateFields is a method to retrieve all required privates field from +// testing.T.context.match, returning a contextMatcher instance +func getTestContextMatcherPrivateFields(t *testing.T) *contextMatcher { + indirectValue := reflect.Indirect(reflect.ValueOf(t)) + contextMember := indirectValue.FieldByName("context") + if !contextMember.IsValid() { + return nil + } + contextMember = contextMember.Elem() + matchMember := contextMember.FieldByName("match") + if !matchMember.IsValid() { + return nil + } + matchMember = matchMember.Elem() + + fields := &contextMatcher{} + if ptr, err := getFieldPointerFromValue(matchMember, "mu"); err == nil { + fields.mu = (*sync.RWMutex)(ptr) + } + if ptr, err := getFieldPointerFromValue(matchMember, "subNames"); err == nil { + fields.subNames = (*map[string]int32)(ptr) + } + + return fields +} + // copyTestWithoutParent tries to copy all private fields except the t.parent from a *testing.T to another func copyTestWithoutParent(source *testing.T, target *testing.T) { // Copy important field values diff --git a/internal/civisibility/integrations/gotesting/testcontroller_test.go b/internal/civisibility/integrations/gotesting/testcontroller_test.go index 48ff206d20..bfcf78c003 100644 --- a/internal/civisibility/integrations/gotesting/testcontroller_test.go +++ b/internal/civisibility/integrations/gotesting/testcontroller_test.go @@ -188,15 +188,15 @@ func runEarlyFlakyTestDetectionTests(m *testing.M) { checkSpansByResourceName(finishedSpans, "testing_test.go", 1) checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest01", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01/sub03", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01/sub03", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/yellow_should_return_color", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/banana_should_return_fruit", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/duck_should_return_animal", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/yellow_should_return_color", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/banana_should_return_fruit", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/duck_should_return_animal", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.TestWithExternalCalls", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.TestWithExternalCalls/default", 11) - // checkSpansByResourceName(finishedSpans, "testing_test.go.TestWithExternalCalls/custom-name", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestWithExternalCalls/default", 11) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestWithExternalCalls/custom-name", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.TestSkip", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithPanic", 11) checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithFail", 11)