Address RSpec lifecycle review feedback

Author: anmarchenko

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

@@ -26,8 +26,7 @@ module InstanceMethods
             # ============================================
 
             def run(*args)
-              return super unless datadog_configuration[:enabled]
-              return super if ::RSpec.configuration.dry_run? && !datadog_configuration[:dry_run_enabled]
+              return super unless datadog_tracing_enabled?
 
               @datadog_test_tracing_component = test_tracing_component
 
@@ -198,19 +197,39 @@ def datadog_context_ids
             # ============================================
 
             def run_before_example
-              test_tracing_component.trace(Ext::BEFORE_STEP_SPAN_NAME, type: Ext::STEP_SPAN_TYPE) { super }
+              return super unless @datadog_test_tracing_component
+
+              skip_exception = nil
+              # @type var skip_exception: ::RSpec::Core::Pending::SkipDeclaredInExample?
+              result = test_tracing_component.trace(Ext::BEFORE_STEP_SPAN_NAME, type: Ext::STEP_SPAN_TYPE) do
+                super
+              rescue ::RSpec::Core::Pending::SkipDeclaredInExample => e
+                skip_exception = e
+              end
+
+              raise skip_exception if skip_exception
+
+              result
             end
 
             def run_after_example
+              return super unless @datadog_test_tracing_component
+
               exception_before_hooks = exception
 
               test_tracing_component.trace(Ext::AFTER_STEP_SPAN_NAME, type: Ext::STEP_SPAN_TYPE) do |span|
                 result = super
-                span.failed!(exception: exception) if exception && exception != exception_before_hooks
+                span&.failed!(exception: exception) if exception && exception != exception_before_hooks
                 result
               end
             end
 
+            def datadog_tracing_enabled?
+              Datadog.configuration.ci.enabled &&
+                datadog_configuration[:enabled] &&
+                (!::RSpec.configuration.dry_run? || datadog_configuration[:dry_run_enabled])
+            end
+
             def build_test_tags
               # @type var tags : Hash[String, String]
               tags = {

sig/datadog/ci/contrib/rspec/example.rbs

@@ -42,6 +42,7 @@ module Datadog
             # Run method helpers
             def run_before_example: () -> untyped
             def run_after_example: () -> untyped
+            def datadog_tracing_enabled?: () -> bool
             def build_test_tags: () -> Hash[String, String]
             def handle_test_result: (Datadog::CI::Test? test_span, Exception? test_failure) -> Exception?
             def update_formatter_metadata: (Datadog::CI::Test? test_span) -> void

spec/datadog/ci/contrib/rspec/instrumentation_spec.rb

@@ -307,6 +307,36 @@ def rspec_session_run(
       expect(custom_spans.map(&:name)).to contain_exactly("before", "after")
     end
 
+    it "does not trace lifecycle steps when the RSpec integration is disabled" do
+      rspec_configuration = Datadog.configuration.ci[:rspec]
+      allow(rspec_configuration).to receive(:[]).and_call_original
+      allow(rspec_configuration).to receive(:[]).with(:enabled).and_return(false)
+
+      result = with_new_rspec_environment do
+        RSpec.describe "some test" do
+          after { raise "failure" }
+          it("fails in after") {}
+        end.run
+      end
+
+      expect(result).to be(false)
+      expect(spans).to be_empty
+    end
+
+    it "does not trace lifecycle steps when Datadog CI is disabled" do
+      allow(Datadog.configuration.ci).to receive(:enabled).and_return(false)
+
+      result = with_new_rspec_environment do
+        RSpec.describe "some test" do
+          after { raise "failure" }
+          it("fails in after") {}
+        end.run
+      end
+
+      expect(result).to be(false)
+      expect(spans).to be_empty
+    end
+
     context "catches failures" do
       def expect_failure
         expect(first_test_span).to have_fail_status
@@ -464,6 +494,23 @@ def expect_failure
         expect(first_test_span).not_to have_error
       end
 
+      it "with skip call in before hook" do
+        with_new_rspec_environment do
+          RSpec.describe "some skipped test" do
+            before { skip }
+
+            it "foo" do
+              expect(1 + 1).to eq(5)
+            end
+          end.run
+        end
+
+        expect(first_test_span).to have_skip_status
+        expect(first_test_span).not_to have_error
+        expect(custom_spans.map(&:name)).to include("before")
+        expect(custom_spans.find { |span| span.name == "before" }).not_to have_error
+      end
+
       it "with skip call and reason given" do
         with_new_rspec_environment do
           RSpec.describe "some skipped test" do

vendor/rbs/rspec/0/rspec.rbs

@@ -6,6 +6,12 @@ end
 module RSpec::Core
 end
 
+module RSpec::Core::Pending
+end
+
+class RSpec::Core::Pending::SkipDeclaredInExample < StandardError
+end
+
 module RSpec::Queue
 end