[CIVIS-8549] better skipped tests and suites support

lib/datadog/ci/concurrent_span.rb

@@ -12,7 +12,8 @@ class ConcurrentSpan < Span
       def initialize(tracer_span)
         super
 
-        @mutex = Mutex.new
+        # we use Monitor instead of Mutex because it is reentrant
+        @mutex = Monitor.new
       end
 
       # Gets tag value by key. This method is thread-safe.

lib/datadog/ci/contrib/cucumber/formatter.rb

@@ -15,12 +15,12 @@ class Formatter
 
           def initialize(config)
             @ast_lookup = ::Cucumber::Formatter::AstLookup.new(config) if defined?(::Cucumber::Formatter::AstLookup)
-
             @config = config
-            @failed_tests_count = 0
 
             @current_test_suite = nil
 
+            @failed_tests_count = 0
+
             bind_events(config)
           end
 
@@ -80,17 +80,8 @@ def on_test_case_finished(event)
             test_span = CI.active_test
             return if test_span.nil?
 
-            # We need to track overall test failures manually if we are using cucumber < 8.0 because
-            # TestRunFinished event does not have a success attribute before 8.0.
-            #
-            if event.result.failed?
-              @failed_tests_count += 1
-
-              test_suite = @current_test_suite
-              test_suite.failed! if test_suite
-            end
-
-            finish_test(test_span, event.result)
+            finish_span(test_span, event.result)
+            @failed_tests_count += 1 if test_span.failed?
           end
 
           def on_test_step_started(event)
@@ -101,7 +92,7 @@ def on_test_step_finished(event)
             current_step_span = CI.active_span
             return if current_step_span.nil?
 
-            finish_test(current_step_span, event.result)
+            finish_span(current_step_span, event.result)
           end
 
           private
@@ -121,12 +112,12 @@ def test_suite_name(test_case)
             end
           end
 
-          def finish_test(span, result)
-            if result.skipped?
-              span.skipped!
-            elsif result.ok?
+          def finish_span(span, result)
+            if !result.passed? && result.ok?(@config.strict)
+              span.skipped!(reason: result.message)
+            elsif result.passed?
               span.passed!
-            elsif result.failed?
+            else
               span.failed!(exception: result.exception)
             end
             span.finish
@@ -155,18 +146,16 @@ def finish_session(result)
           def start_test_suite(test_suite_name)
             finish_current_test_suite
 
-            test_suite = CI.start_test_suite(test_suite_name)
-            # will be overridden if any test fails
-            test_suite.passed! if test_suite
-
-            @current_test_suite = test_suite
+            @current_test_suite = CI.start_test_suite(test_suite_name)
           end
 
           def finish_current_test_suite
             test_suite = @current_test_suite
             return unless test_suite
 
             test_suite.finish
+
+            @current_test_suite = nil
           end
 
           def same_test_suite_as_current?(test_suite_name)

lib/datadog/ci/contrib/minitest/hooks.rb

@@ -41,33 +41,19 @@ def after_teardown
             test_span = CI.active_test
             return super unless test_span
 
-            finish_test(test_span, result_code)
+            finish_with_result(test_span, result_code)
             if Helpers.parallel?(self.class)
-              finish_test_suite(test_span.test_suite, result_code)
+              finish_with_result(test_span.test_suite, result_code)
             end
 
             super
           end
 
           private
 
-          def finish_test(test_span, result_code)
-            finish_with_result(test_span, result_code)
-
-            # mark test suite as failed if any test failed
-            if test_span.failed?
-              test_suite = test_span.test_suite
-              test_suite.failed! if test_suite
-            end
-          end
-
-          def finish_test_suite(test_suite, result_code)
-            return unless test_suite
-
-            finish_with_result(test_suite, result_code)
-          end
-
           def finish_with_result(span, result_code)
+            return unless span
+
             case result_code
             when "."
               span.passed!

lib/datadog/ci/contrib/minitest/runnable.rb

@@ -20,7 +20,6 @@ def run(*)
               test_suite_name = Helpers.test_suite_name(self, method)
 
               test_suite = Datadog::CI.start_test_suite(test_suite_name)
-              test_suite.passed! if test_suite # will be overridden if any test fails
 
               results = super
               return results unless test_suite

lib/datadog/ci/contrib/rspec/example.rb

@@ -60,13 +60,11 @@ def run(*)
                     test_suite_span.failed! if test_suite_span
                   else
                     # :pending or nil
-                    if execution_result.pending_message
-                      test_span.skipped!(reason: execution_result.pending_message)
-                    elsif execution_result.example_skipped?
-                      test_span.skipped!(exception: execution_result.exception)
-                    else
-                      test_span.skipped!(exception: execution_result.pending_exception)
-                    end
+                    test_span.skipped!(
+                      reason: execution_result.pending_message,
+                      exception: execution_result.pending_exception
+                    )
+
                     test_suite_span.skipped! if test_suite_span
                   end
                 end

lib/datadog/ci/contrib/rspec/example_group.rb

@@ -15,24 +15,27 @@ def self.included(base)
 
           # Instance methods for configuration
           module ClassMethods
-            def run(*)
+            def run(reporter = ::RSpec::Core::NullReporter)
               return super unless datadog_configuration[:enabled]
               return super unless top_level?
 
               suite_name = "#{description} at #{file_path}"
               test_suite = Datadog::CI.start_test_suite(suite_name)
 
-              result = super
-              return result unless test_suite
+              success = super
+              return success unless test_suite
 
-              if result
+              if success && test_suite.passed_tests_count > 0
                 test_suite.passed!
+              elsif success
+                test_suite.skipped!
               else
                 test_suite.failed!
               end
+
               test_suite.finish
 
-              result
+              success
             end
 
             private

lib/datadog/ci/test.rb

@@ -62,6 +62,33 @@ def source_file
         get_tag(Ext::Test::TAG_SOURCE_FILE)
       end
 
+      # Sets the status of the span to "pass".
+      # @return [void]
+      def passed!
+        super
+
+        record_test_result(Ext::Test::Status::PASS)
+      end
+
+      # Sets the status of the span to "fail".
+      # @param [Exception] exception the exception that caused the test to fail.
+      # @return [void]
+      def failed!(exception: nil)
+        super
+
+        record_test_result(Ext::Test::Status::FAIL)
+      end
+
+      # Sets the status of the span to "skip".
+      # @param [Exception] exception the exception that caused the test to fail.
+      # @param [String] reason the reason why the test was skipped.
+      # @return [void]
+      def skipped!(exception: nil, reason: nil)
+        super
+
+        record_test_result(Ext::Test::Status::SKIP)
+      end
+
       # Sets the parameters for this test (e.g. Cucumber example or RSpec shared specs).
       # Parameters are needed to compute test fingerprint to distinguish between different tests having same names.
       #
@@ -81,6 +108,13 @@ def set_parameters(arguments, metadata = {})
           )
         )
       end
+
+      private
+
+      def record_test_result(datadog_status)
+        suite = test_suite
+        suite.record_test_result(datadog_status) if suite
+      end
     end
   end
 end

lib/datadog/ci/test_suite.rb

@@ -13,12 +13,63 @@ module CI
     #
     # @public_api
     class TestSuite < ConcurrentSpan
+      def initialize(tracer_span)
+        super
+
+        @test_suite_stats = Hash.new(0)
+      end
+
       # Finishes this test suite.
       # @return [void]
       def finish
-        super
+        synchronize do
+          # we try to derive test suite status from execution stats if no status was set explicitly
+          set_status_from_stats! if undefined?
+
+          super
+
+          recorder.deactivate_test_suite(name)
+        end
+      end
+
+      # @internal
+      def record_test_result(datadog_test_status)
+        synchronize do
+          @test_suite_stats[datadog_test_status] += 1
+        end
+      end
+
+      # @internal
+      def passed_tests_count
+        synchronize do
+          @test_suite_stats[Ext::Test::Status::PASS]
+        end
+      end
+
+      # @internal
+      def skipped_tests_count
+        synchronize do
+          @test_suite_stats[Ext::Test::Status::SKIP]
+        end
+      end
+
+      # @internal
+      def failed_tests_count
+        synchronize do
+          @test_suite_stats[Ext::Test::Status::FAIL]
+        end
+      end
+
+      private
 
-        recorder.deactivate_test_suite(name)
+      def set_status_from_stats!
+        if failed_tests_count > 0
+          failed!
+        elsif passed_tests_count == 0
+          skipped!
+        else
+          passed!
+        end
       end
     end
   end

sig/datadog/ci/concurrent_span.rbs

@@ -1,7 +1,7 @@
 module Datadog
   module CI
     class ConcurrentSpan < Span
-      @mutex: Thread::Mutex
+      @mutex: Monitor
 
       def initialize: (Datadog::Tracing::SpanOperation tracer_span) -> void
       def passed!: () -> void

sig/datadog/ci/contrib/cucumber/formatter.rbs

@@ -4,9 +4,12 @@ module Datadog
       module Cucumber
         class Formatter
           private
-          @failed_tests_count: Integer
-          @current_test_suite: Datadog::CI::Span?
           @ast_lookup: ::Cucumber::Formatter::AstLookup
+          @config: untyped
+
+          @current_test_suite: Datadog::CI::TestSuite?
+
+          @failed_tests_count: Integer
 
           attr_reader config: untyped
 
@@ -40,7 +43,7 @@ module Datadog
 
           def finish_session: (bool result) -> void
 
-          def finish_test: (Datadog::CI::Span test, Cucumber::Core::Test::Result result) -> void
+          def finish_span: (Datadog::CI::Span span, Cucumber::Core::Test::Result result) -> void
 
           def extract_parameters_hash: (untyped test_case) -> Hash[String, String]?
 

sig/datadog/ci/contrib/minitest/hooks.rbs

@@ -13,11 +13,7 @@ module Datadog
 
           def datadog_configuration: () -> untyped
 
-          def finish_test: (Datadog::CI::Test test_span, String result_code) -> void
-
-          def finish_test_suite: (Datadog::CI::TestSuite? test_suite, String result_code) -> void
-
-          def finish_with_result: (Datadog::CI::Span span, String result_code) -> void
+          def finish_with_result: (Datadog::CI::Span? span, String result_code) -> void
 
           def self.test_order: () -> (nil | :parallel | :sorted | :random | :alpha)
         end

sig/datadog/ci/test.rbs

@@ -8,6 +8,10 @@ module Datadog
       def test_module_id: () -> String?
       def test_session_id: () -> String?
       def source_file: () -> String?
+
+      private
+
+      def record_test_result: (String datadog_status) -> void
     end
   end
 end

sig/datadog/ci/test_suite.rbs

@@ -1,6 +1,17 @@
 module Datadog
   module CI
     class TestSuite < ConcurrentSpan
+      @test_suite_stats: Hash[String, Integer]
+
+      def record_test_result: (String datadog_test_status) -> void
+
+      def passed_tests_count: () -> Integer
+      def skipped_tests_count: () -> Integer
+      def failed_tests_count: () -> Integer
+
+      private
+
+      def set_status_from_stats!: () -> void
     end
   end
 end