From 3cb8ab2a4c172bc5af3542608595198cb4cef3df Mon Sep 17 00:00:00 2001 From: Tony Redondo Date: Thu, 31 Oct 2024 12:14:49 +0100 Subject: [PATCH] internal/civisibility: add support for unskippable tests and suites (#2957) --- internal/civisibility/constants/test_tags.go | 6 +++ .../gotesting/reflections_test.go | 2 + .../gotesting/testcontroller_test.go | 35 ++++++++++------- .../integrations/gotesting/testing.go | 19 +++++---- .../integrations/gotesting/testing_test.go | 2 + .../integrations/manual_api_ddtest.go | 39 ++++++++++++++++++- 6 files changed, 82 insertions(+), 21 deletions(-) diff --git a/internal/civisibility/constants/test_tags.go b/internal/civisibility/constants/test_tags.go index bbfc94a563..d3b30d461b 100644 --- a/internal/civisibility/constants/test_tags.go +++ b/internal/civisibility/constants/test_tags.go @@ -101,6 +101,12 @@ const ( // CodeCoverageEnabled indicates that code coverage is enabled CodeCoverageEnabled = "test.code_coverage.enabled" + + // TestUnskippable indicates that the test is unskippable + TestUnskippable = "test.itr.unskippable" + + // TestForcedToRun indicates that the test is forced to run because is unskippable + TestForcedToRun = "test.itr.forced_run" ) // Define valid test status types. diff --git a/internal/civisibility/integrations/gotesting/reflections_test.go b/internal/civisibility/integrations/gotesting/reflections_test.go index 453cd7a0e9..4ed25a6d02 100644 --- a/internal/civisibility/integrations/gotesting/reflections_test.go +++ b/internal/civisibility/integrations/gotesting/reflections_test.go @@ -12,6 +12,8 @@ import ( "github.com/stretchr/testify/assert" ) +//dd:suite.unskippable + // TestGetFieldPointerFrom tests the getFieldPointerFrom function. func TestGetFieldPointerFrom(t *testing.T) { // Create a mock struct with a private field diff --git a/internal/civisibility/integrations/gotesting/testcontroller_test.go b/internal/civisibility/integrations/gotesting/testcontroller_test.go index 6bd2a10544..bdae400e68 100644 --- a/internal/civisibility/integrations/gotesting/testcontroller_test.go +++ b/internal/civisibility/integrations/gotesting/testcontroller_test.go @@ -339,6 +339,10 @@ func runIntelligentTestRunnerTests(m *testing.M) { Suite: "testing_test.go", Name: "TestRetryAlwaysFail", }, + { + Suite: "testing_test.go", + Name: "TestNormalPassingAfterRetryAlwaysFail", + }, }) defer server.Close() @@ -373,29 +377,34 @@ func runIntelligentTestRunnerTests(m *testing.M) { checkSpansByResourceName(finishedSpans, "gopkg.in/DataDog/dd-trace-go.v1/internal/civisibility/integrations/gotesting", 1) checkSpansByResourceName(finishedSpans, "reflections_test.go", 1) checkSpansByResourceName(finishedSpans, "testing_test.go", 1) - itrTest01 := checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest01", 1) - itrTest02 := checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02", 1) + checkSpansByResourceName(finishedSpans, "reflections_test.go.TestGetFieldPointerFrom", 1) + checkSpansByResourceName(finishedSpans, "reflections_test.go.TestGetInternalTestArray", 1) + checkSpansByResourceName(finishedSpans, "reflections_test.go.TestGetInternalBenchmarkArray", 1) + checkSpansByResourceName(finishedSpans, "reflections_test.go.TestCommonPrivateFields_AddLevel", 1) + checkSpansByResourceName(finishedSpans, "reflections_test.go.TestGetBenchmarkPrivateFields", 1) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest01", 1) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02", 1) checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01", 0) checkSpansByResourceName(finishedSpans, "testing_test.go.TestMyTest02/sub01/sub03", 0) - itrTest03 := checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo", 1) + checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo", 1) checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/yellow_should_return_color", 0) checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/banana_should_return_fruit", 0) checkSpansByResourceName(finishedSpans, "testing_test.go.Test_Foo/duck_should_return_animal", 0) checkSpansByResourceName(finishedSpans, "testing_test.go.TestSkip", 1) - itrTest04 := checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithPanic", 1) - itrTest05 := checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithFail", 1) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithPanic", 1) + checkSpansByResourceName(finishedSpans, "testing_test.go.TestRetryWithFail", 1) checkSpansByResourceName(finishedSpans, "testing_test.go.TestNormalPassingAfterRetryAlwaysFail", 1) checkSpansByResourceName(finishedSpans, "testing_test.go.TestEarlyFlakeDetection", 1) // check ITR spans - var itrTests []mocktracer.Span - itrTests = append(itrTests, itrTest01...) - itrTests = append(itrTests, itrTest02...) - itrTests = append(itrTests, itrTest03...) - itrTests = append(itrTests, itrTest04...) - itrTests = append(itrTests, itrTest05...) - checkSpansByTagValue(itrTests, constants.TestStatus, constants.TestStatusSkip, 5) - checkSpansByTagValue(itrTests, constants.TestSkipReason, constants.SkippedByITRReason, 5) + // 5 tests skipped by ITR and 1 normal skipped test + checkSpansByTagValue(finishedSpans, constants.TestStatus, constants.TestStatusSkip, 6) + checkSpansByTagValue(finishedSpans, constants.TestSkipReason, constants.SkippedByITRReason, 5) + + // check unskippable tests + // 5 tests from unskippable suite in reflections_test.go and 2 unskippable tests from testing_test.go + checkSpansByTagValue(finishedSpans, constants.TestUnskippable, "true", 7) + checkSpansByTagValue(finishedSpans, constants.TestForcedToRun, "true", 1) // check spans by type checkSpansByType(finishedSpans, diff --git a/internal/civisibility/integrations/gotesting/testing.go b/internal/civisibility/integrations/gotesting/testing.go index d64757b546..770fd244d2 100644 --- a/internal/civisibility/integrations/gotesting/testing.go +++ b/internal/civisibility/integrations/gotesting/testing.go @@ -217,13 +217,18 @@ func (ddm *M) executeInternalTest(testInfo *testingTInfo) func(*testing.T) { // Check if the test needs to be skipped by ITR if testSkippedByITR { - test.SetTag(constants.TestSkippedByITR, "true") - test.CloseWithFinishTimeAndSkipReason(integrations.ResultStatusSkip, time.Now(), constants.SkippedByITRReason) - session.SetTag(constants.ITRTestsSkipped, "true") - session.SetTag(constants.ITRTestsSkippingCount, numOfTestsSkipped.Add(1)) - checkModuleAndSuite(module, suite) - t.Skip(constants.SkippedByITRReason) - return + // check if the test was marked as unskippable + if test.Context().Value(constants.TestUnskippable) != true { + test.SetTag(constants.TestSkippedByITR, "true") + test.CloseWithFinishTimeAndSkipReason(integrations.ResultStatusSkip, time.Now(), constants.SkippedByITRReason) + session.SetTag(constants.ITRTestsSkipped, "true") + session.SetTag(constants.ITRTestsSkippingCount, numOfTestsSkipped.Add(1)) + checkModuleAndSuite(module, suite) + t.Skip(constants.SkippedByITRReason) + return + } else { + test.SetTag(constants.TestForcedToRun, "true") + } } // Check if the coverage is enabled diff --git a/internal/civisibility/integrations/gotesting/testing_test.go b/internal/civisibility/integrations/gotesting/testing_test.go index 5501ee758d..7c4485ce62 100644 --- a/internal/civisibility/integrations/gotesting/testing_test.go +++ b/internal/civisibility/integrations/gotesting/testing_test.go @@ -117,10 +117,12 @@ func TestRetryWithFail(t *testing.T) { } } +//dd:test.unskippable func TestNormalPassingAfterRetryAlwaysFail(t *testing.T) {} var run int +//dd:test.unskippable func TestEarlyFlakeDetection(t *testing.T) { run++ fmt.Printf(" Run: %d", run) diff --git a/internal/civisibility/integrations/manual_api_ddtest.go b/internal/civisibility/integrations/manual_api_ddtest.go index 6451b6ac13..a0661536b3 100644 --- a/internal/civisibility/integrations/manual_api_ddtest.go +++ b/internal/civisibility/integrations/manual_api_ddtest.go @@ -155,8 +155,27 @@ func (t *tslvTest) SetTestFunc(fn *runtime.Func) { // parse the entire file where the function is defined to create an abstract syntax tree (AST) // if we can't parse the file (source code is not available) we silently bail out fset := token.NewFileSet() - fileNode, err := parser.ParseFile(fset, absolutePath, nil, parser.AllErrors) + fileNode, err := parser.ParseFile(fset, absolutePath, nil, parser.AllErrors|parser.ParseComments) if err == nil { + + // let's check if the suite was marked as unskippable before + isUnskippable, hasUnskippableValue := t.suite.ctx.Value(constants.TestUnskippable).(bool) + if !hasUnskippableValue { + // check for suite level unskippable comment at the top of the file + for _, commentGroup := range fileNode.Comments { + for _, comment := range commentGroup.List { + if strings.Contains(comment.Text, "//dd:suite.unskippable") { + isUnskippable = true + break + } + } + if isUnskippable { + break + } + } + t.suite.ctx = context.WithValue(t.suite.ctx, constants.TestUnskippable, isUnskippable) + } + // get the function name without the package name fullName := fn.Name() firstDot := strings.LastIndex(fullName, ".") + 1 @@ -164,6 +183,7 @@ func (t *tslvTest) SetTestFunc(fn *runtime.Func) { // variable to store the ending line of the function var endLine int + // traverse the AST to find the function declaration for the target function ast.Inspect(fileNode, func(n ast.Node) bool { // check if the current node is a function declaration @@ -172,6 +192,17 @@ func (t *tslvTest) SetTestFunc(fn *runtime.Func) { if funcDecl.Name.Name == name { // get the line number of the end of the function body endLine = fset.Position(funcDecl.Body.End()).Line + // check for comments above the function declaration to look for unskippable tag + // but only if we haven't found a suite level unskippable comment + if !isUnskippable && funcDecl.Doc != nil { + for _, comment := range funcDecl.Doc.List { + if strings.Contains(comment.Text, "//dd:test.unskippable") { + isUnskippable = true + break + } + } + } + // stop further inspection since we have found the target function return false } @@ -194,6 +225,12 @@ func (t *tslvTest) SetTestFunc(fn *runtime.Func) { if endLine >= startLine { t.SetTag(constants.TestSourceEndLine, endLine) } + + // if the function is marked as unskippable, set the appropriate tag + if isUnskippable { + t.SetTag(constants.TestUnskippable, "true") + t.ctx = context.WithValue(t.ctx, constants.TestUnskippable, true) + } } // get the codeowner of the function