From ee04f6afd16885f64ee2abe1b3c113ac28da4671 Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 3 Dec 2024 13:13:19 +0100 Subject: [PATCH 01/10] add automated test to check that context hooks are not called when all tests are skipped by test impact analysis --- .../ci/contrib/rspec/instrumentation_spec.rb | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 22e2c1ae..70ee5e4b 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -2,6 +2,7 @@ RSpec.describe "RSpec instrumentation" do let(:integration) { Datadog::CI::Contrib::Instrumentation.fetch_integration(:rspec) } + let(:before_all_spy) { spy(:before_all, call: nil) } before do # expect that public manual API isn't used @@ -36,9 +37,14 @@ def rspec_session_run( flaky_test_that_fails_once_passes = 0 current_let_value = 0 + before_all_spy_local = before_all_spy with_new_rspec_environment do spec = RSpec.describe "SomeTest", suite_meta do + before(:all) do + before_all_spy_local.call + end + context "nested", context_meta do let(:let_value) { current_let_value += 1 } @@ -150,7 +156,7 @@ def rspec_session_run( :source_file, "spec/datadog/ci/contrib/rspec/instrumentation_spec.rb" ) - expect(first_test_span).to have_test_tag(:source_start, "123") + expect(first_test_span).to have_test_tag(:source_start, "129") expect(first_test_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -580,7 +586,7 @@ def expect_failure :source_file, "spec/datadog/ci/contrib/rspec/instrumentation_spec.rb" ) - expect(first_test_suite_span).to have_test_tag(:source_start, "41") + expect(first_test_suite_span).to have_test_tag(:source_start, "43") expect(first_test_suite_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -785,6 +791,12 @@ def rspec_skipped_session_run expect(test_session_span).to have_test_tag(:itr_test_skipping_count, 2) end + it "does not run context hooks" do + rspec_session_run(with_failed_test: true) + + expect(before_all_spy).not_to have_received(:call) + end + context "but some tests are unskippable" do context "when a test is unskippable" do it "runs the test and adds forced run tag" do From 6df2a5aed39c5d7c7c3cb4be2b1e47f98ebfaada Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 3 Dec 2024 13:51:58 +0100 Subject: [PATCH 02/10] make transport spec more stable as msgpack event size has increased --- spec/datadog/ci/test_visibility/transport_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/datadog/ci/test_visibility/transport_spec.rb b/spec/datadog/ci/test_visibility/transport_spec.rb index e241ed22..4d0cfe40 100644 --- a/spec/datadog/ci/test_visibility/transport_spec.rb +++ b/spec/datadog/ci/test_visibility/transport_spec.rb @@ -209,9 +209,9 @@ end context "when chunking is used" do - # one test event is approximately 1000 bytes currently + # one test event is approximately 1200 bytes currently # ATTENTION: might break if more data is added to test spans in #produce_test_trace method - let(:max_payload_size) { 2000 } + let(:max_payload_size) { 2500 } it "sends events in two chunks" do responses = subject From 4fda3daeee5b32ddf5ffd8e7f9e6b345d38a8b48 Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 3 Dec 2024 14:08:54 +0100 Subject: [PATCH 03/10] add test that before hooks must run if at least one test is not skipped in ExampleGroup --- spec/datadog/ci/contrib/rspec/instrumentation_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 70ee5e4b..1fb48312 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -757,6 +757,12 @@ def rspec_skipped_session_run expect(itr_skipped_test).to have_test_tag(:itr_skipped_by_itr, "true") end + it "runs context hooks" do + rspec_session_run(with_failed_test: true) + + expect(before_all_spy).to have_received(:call) + end + it "sends test session level tags" do rspec_session_run(with_failed_test: true) From e80271e2325bd2a6468afedc98e1126b9960b3be Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 3 Dec 2024 14:09:11 +0100 Subject: [PATCH 04/10] testing metadata[:skip] approach for skipping context hooks --- lib/datadog/ci/contrib/rspec/example_group.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index 35a38f74..48017bca 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -29,6 +29,9 @@ def run(reporter = ::RSpec::Core::NullReporter) } ) + # try skipping the whole example group + metadata[:skip] = true + success = super return success unless test_suite From e8a361dd0098a4f7b82199fbdeb3726ea2920219 Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 3 Dec 2024 16:28:11 +0100 Subject: [PATCH 05/10] introduce TestOptimisation::Component#skippable? method to encapsulate the logic to check if a test should be skipped in the current test run --- lib/datadog/ci/contrib/rspec/example_group.rb | 5 ++++- lib/datadog/ci/test.rb | 15 ++++++++------- lib/datadog/ci/test_optimisation/component.rb | 13 +++++++++---- sig/datadog/ci/test.rbs | 4 ++-- sig/datadog/ci/test_optimisation/component.rbs | 2 ++ 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index 48017bca..124d4427 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -30,7 +30,10 @@ def run(reporter = ::RSpec::Core::NullReporter) ) # try skipping the whole example group - metadata[:skip] = true + # all_skipped = descendant_filtered_examples.all? do |example| + # end + all_skipped = false + metadata[:skip] = true if all_skipped success = super return success unless test_suite diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index 00912e43..d5d47b81 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -17,6 +17,11 @@ def name get_tag(Ext::Test::TAG_NAME) end + # @return [String] the test id according to Datadog's test impact analysis. + def datadog_test_id + @datadog_test_id ||= Utils::TestRun.datadog_test_id(name, test_suite_name, parameters) + end + # Finishes the current test. # @return [void] def finish @@ -140,22 +145,18 @@ def parameters # @internal def any_retry_passed? - !!test_suite&.any_test_retry_passed?(test_id) + !!test_suite&.any_test_retry_passed?(datadog_test_id) end private - def test_id - @test_id ||= Utils::TestRun.datadog_test_id(name, test_suite_name, parameters) - end - def record_test_result(datadog_status) # if this test was already executed in this test suite, mark it as retried - if test_suite&.test_executed?(test_id) + if test_suite&.test_executed?(datadog_test_id) set_tag(Ext::Test::TAG_IS_RETRY, "true") end - test_suite&.record_test_result(test_id, datadog_status) + test_suite&.record_test_result(datadog_test_id, datadog_status) end end end diff --git a/lib/datadog/ci/test_optimisation/component.rb b/lib/datadog/ci/test_optimisation/component.rb index 562ca6c9..e13add52 100644 --- a/lib/datadog/ci/test_optimisation/component.rb +++ b/lib/datadog/ci/test_optimisation/component.rb @@ -144,11 +144,16 @@ def stop_coverage(test) event end + def skippable?(test) + return false if !enabled? || !skipping_tests? + + @skippable_tests.include?(test.datadog_test_id) + end + def mark_if_skippable(test) return if !enabled? || !skipping_tests? - datadog_test_id = Utils::TestRun.datadog_test_id(test.name, test.test_suite_name, test.parameters) - if @skippable_tests.include?(datadog_test_id) + if skippable?(test) if forked? Datadog.logger.warn { "Intelligent test runner is not supported for forking test runners yet" } return @@ -156,9 +161,9 @@ def mark_if_skippable(test) test.set_tag(Ext::Test::TAG_ITR_SKIPPED_BY_ITR, "true") - Datadog.logger.debug { "Marked test as skippable: #{datadog_test_id}" } + Datadog.logger.debug { "Marked test as skippable: #{test.datadog_test_id}" } else - Datadog.logger.debug { "Test is not skippable: #{datadog_test_id}" } + Datadog.logger.debug { "Test is not skippable: #{test.datadog_test_id}" } end end diff --git a/sig/datadog/ci/test.rbs b/sig/datadog/ci/test.rbs index 0c2aaef6..63aaf06c 100644 --- a/sig/datadog/ci/test.rbs +++ b/sig/datadog/ci/test.rbs @@ -1,8 +1,9 @@ module Datadog module CI class Test < Span - @test_id: String + @datadog_test_id: String + def datadog_test_id: () -> String def finish: () -> void def test_suite: () -> Datadog::CI::TestSuite? def test_suite_id: () -> String? @@ -17,7 +18,6 @@ module Datadog private - def test_id: () -> String def record_test_result: (String datadog_status) -> void end end diff --git a/sig/datadog/ci/test_optimisation/component.rbs b/sig/datadog/ci/test_optimisation/component.rbs index afbd7bf8..0a745c61 100644 --- a/sig/datadog/ci/test_optimisation/component.rbs +++ b/sig/datadog/ci/test_optimisation/component.rbs @@ -45,6 +45,8 @@ module Datadog def stop_coverage: (Datadog::CI::Test test) -> Datadog::CI::TestOptimisation::Coverage::Event? + def skippable?: (Datadog::CI::Test test) -> bool + def mark_if_skippable: (Datadog::CI::Test test) -> void def count_skipped_test: (Datadog::CI::Test test) -> void From 64f1517bccbe2bfafb8a425ea80d9d06db28e557 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 4 Dec 2024 11:33:47 +0100 Subject: [PATCH 06/10] refactor RSpec::Core::Example instrumentation to support checking if Example is skippable by test impact analysis --- lib/datadog/ci/contrib/rspec/example.rb | 68 ++++++++++++++++-------- sig/datadog/ci/contrib/rspec/example.rbs | 11 ++++ 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index b060f549..36397c7a 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -21,38 +21,21 @@ def run(*args) return super if ::RSpec.configuration.dry_run? && !datadog_configuration[:dry_run_enabled] return super unless datadog_configuration[:enabled] - test_name = full_description.strip - if metadata[:description].empty? - # for unnamed it blocks this appends something like "example at ./spec/some_spec.rb:10" - test_name << " #{description}" - end - - test_suite_description = fetch_top_level_example_group[:description] - suite_name = "#{test_suite_description} at #{metadata[:example_group][:rerun_file_path]}" - - # remove example group description from test name to avoid duplication - test_name = test_name.sub(test_suite_description, "").strip - - if ci_queue? - suite_name = "#{suite_name} (ci-queue running example [#{test_name}])" - ci_queue_test_span = test_visibility_component.start_test_suite(suite_name) - end + test_suite_span = test_visibility_component.start_test_suite(datadog_test_suite_name) if ci_queue? # don't report test to RSpec::Core::Reporter until retries are done @skip_reporting = true test_retries_component.with_retries do test_visibility_component.trace_test( - test_name, - suite_name, + datadog_test_name, + datadog_test_suite_name, tags: { CI::Ext::Test::TAG_FRAMEWORK => Ext::FRAMEWORK, CI::Ext::Test::TAG_FRAMEWORK_VERSION => datadog_integration.version.to_s, CI::Ext::Test::TAG_SOURCE_FILE => Git::LocalRepository.relative_to_root(metadata[:file_path]), CI::Ext::Test::TAG_SOURCE_START => metadata[:line_number].to_s, - CI::Ext::Test::TAG_PARAMETERS => Utils::TestRun.test_parameters( - metadata: {"scoped_id" => metadata[:scoped_id]} - ) + CI::Ext::Test::TAG_PARAMETERS => datadog_test_parameters }, service: datadog_configuration[:service_name] ) do |test_span| @@ -87,8 +70,8 @@ def run(*args) end end - # this is a special case for ci-queue, we need to finish the test suite span - ci_queue_test_span&.finish + # this is a special case for ci-queue, we need to finish the test suite span created for a single test + test_suite_span&.finish # after retries are done, we can finally report the test to RSpec @skip_reporting = false @@ -129,6 +112,45 @@ def datadog_configuration Datadog.configuration.ci[:rspec] end + def datadog_test_suite_description + @datadog_test_suite_description ||= fetch_top_level_example_group[:description] + end + + def datadog_test_name + return @datadog_test_name if defined?(@datadog_test_name) + + test_name = full_description.strip + if metadata[:description].empty? + # for unnamed it blocks this appends something like "example at ./spec/some_spec.rb:10" + test_name << " #{description}" + end + + # remove example group description from test name to avoid duplication + test_name = test_name.sub(datadog_test_suite_description, "").strip + + @datadog_test_name = test_name + end + + def datadog_test_suite_name + return @datadog_test_suite_name if defined?(@datadog_test_suite_name) + + suite_name = "#{datadog_test_suite_description} at #{metadata[:example_group][:rerun_file_path]}" + + if ci_queue? + suite_name = "#{suite_name} (ci-queue running example [#{datadog_test_name}])" + end + + @datadog_test_suite_name = suite_name + end + + def datadog_test_parameters + return @datadog_test_parameters if defined?(@datadog_test_parameters) + + @datadog_test_parameters = Utils::TestRun.test_parameters( + metadata: {"scoped_id" => metadata[:scoped_id]} + ) + end + def test_visibility_component Datadog.send(:components).test_visibility end diff --git a/sig/datadog/ci/contrib/rspec/example.rbs b/sig/datadog/ci/contrib/rspec/example.rbs index d324d9bd..c6fe8261 100644 --- a/sig/datadog/ci/contrib/rspec/example.rbs +++ b/sig/datadog/ci/contrib/rspec/example.rbs @@ -7,6 +7,12 @@ module Datadog module InstanceMethods : ::RSpec::Core::Example @skip_reporting: bool + @datadog_test_suite_description: String + + @datadog_test_name: String + @datadog_test_suite_name: String + @datadog_test_parameters: String + def run: (untyped example_group_instance, untyped reporter) -> untyped private @@ -17,6 +23,11 @@ module Datadog def test_visibility_component: () -> Datadog::CI::TestVisibility::Component def test_retries_component: () -> Datadog::CI::TestRetries::Component def ci_queue?: () -> bool + + def datadog_test_suite_description: () -> String + def datadog_test_name: () -> String + def datadog_test_suite_name: () -> String + def datadog_test_parameters: () -> String end end end From ee526094acda748fa2d011930207ee4d2b52b9e4 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 4 Dec 2024 11:36:19 +0100 Subject: [PATCH 07/10] add #datadog_unskippable? method to RSpec::Core::Example --- lib/datadog/ci/contrib/rspec/example.rb | 6 +++++- sig/datadog/ci/contrib/rspec/example.rbs | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 36397c7a..c6172d91 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -39,7 +39,7 @@ def run(*args) }, service: datadog_configuration[:service_name] ) do |test_span| - test_span&.itr_unskippable! if metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] + test_span&.itr_unskippable! if datadog_unskippable? metadata[:skip] = CI::Ext::Test::ITR_TEST_SKIP_REASON if test_span&.skipped_by_itr? @@ -151,6 +151,10 @@ def datadog_test_parameters ) end + def datadog_unskippable? + !!metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] + end + def test_visibility_component Datadog.send(:components).test_visibility end diff --git a/sig/datadog/ci/contrib/rspec/example.rbs b/sig/datadog/ci/contrib/rspec/example.rbs index c6fe8261..10358013 100644 --- a/sig/datadog/ci/contrib/rspec/example.rbs +++ b/sig/datadog/ci/contrib/rspec/example.rbs @@ -28,6 +28,7 @@ module Datadog def datadog_test_name: () -> String def datadog_test_suite_name: () -> String def datadog_test_parameters: () -> String + def datadog_unskippable?: () -> bool end end end From b2fe2bb614e7787ac22c671633732302728e888f Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 4 Dec 2024 12:33:55 +0100 Subject: [PATCH 08/10] rspec skips context hooks if all descendant examples are skipped by datadog --- lib/datadog/ci/contrib/rspec/example.rb | 16 ++++++++++++---- lib/datadog/ci/contrib/rspec/example_group.rb | 19 +++++++++++++------ sig/datadog/ci/contrib/rspec/example.rbs | 5 ++++- .../ci/contrib/rspec/example_group.rbs | 6 +++++- vendor/rbs/rspec/0/rspec.rbs | 1 + 5 files changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index c6172d91..8c3b5da7 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -89,6 +89,18 @@ def finish(reporter) super(::RSpec::Core::NullReporter) end + def datadog_test_id + @datadog_test_id ||= Utils::TestRun.datadog_test_id( + datadog_test_name, + datadog_test_suite_name, + datadog_test_parameters + ) + end + + def datadog_unskippable? + !!metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] + end + private def fetch_top_level_example_group @@ -151,10 +163,6 @@ def datadog_test_parameters ) end - def datadog_unskippable? - !!metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] - end - def test_visibility_component Datadog.send(:components).test_visibility end diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index 124d4427..bc869668 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -21,7 +21,7 @@ def run(reporter = ::RSpec::Core::NullReporter) return super unless top_level? suite_name = "#{description} at #{file_path}" - test_suite = test_visibility_component.start_test_suite( + test_suite = test_visibility_component&.start_test_suite( suite_name, tags: { CI::Ext::Test::TAG_SOURCE_FILE => Git::LocalRepository.relative_to_root(metadata[:file_path]), @@ -29,11 +29,8 @@ def run(reporter = ::RSpec::Core::NullReporter) } ) - # try skipping the whole example group - # all_skipped = descendant_filtered_examples.all? do |example| - # end - all_skipped = false - metadata[:skip] = true if all_skipped + # skip the context hooks if all descendant example are going to be skipped + metadata[:skip] = true if all_examples_skipped_by_datadog? success = super return success unless test_suite @@ -53,6 +50,12 @@ def run(reporter = ::RSpec::Core::NullReporter) private + def all_examples_skipped_by_datadog? + descendant_filtered_examples.all? do |example| + !example.datadog_unskippable? && test_optimisation_component&.skippable?(example) + end + end + def datadog_configuration Datadog.configuration.ci[:rspec] end @@ -60,6 +63,10 @@ def datadog_configuration def test_visibility_component Datadog.send(:components).test_visibility end + + def test_optimisation_component + Datadog.send(:components).test_optimisation + end end end end diff --git a/sig/datadog/ci/contrib/rspec/example.rbs b/sig/datadog/ci/contrib/rspec/example.rbs index 10358013..35d59ec6 100644 --- a/sig/datadog/ci/contrib/rspec/example.rbs +++ b/sig/datadog/ci/contrib/rspec/example.rbs @@ -9,12 +9,16 @@ module Datadog @datadog_test_suite_description: String + @datadog_test_id: String @datadog_test_name: String @datadog_test_suite_name: String @datadog_test_parameters: String def run: (untyped example_group_instance, untyped reporter) -> untyped + def datadog_test_id: () -> String + def datadog_unskippable?: () -> bool + private def fetch_top_level_example_group: () -> Hash[Symbol, untyped] @@ -28,7 +32,6 @@ module Datadog def datadog_test_name: () -> String def datadog_test_suite_name: () -> String def datadog_test_parameters: () -> String - def datadog_unskippable?: () -> bool end end end diff --git a/sig/datadog/ci/contrib/rspec/example_group.rbs b/sig/datadog/ci/contrib/rspec/example_group.rbs index 4571dc2c..9cadcd98 100644 --- a/sig/datadog/ci/contrib/rspec/example_group.rbs +++ b/sig/datadog/ci/contrib/rspec/example_group.rbs @@ -12,9 +12,13 @@ module Datadog private + def all_examples_skipped_by_datadog?: () -> bool + def datadog_configuration: () -> untyped - def test_visibility_component: () -> Datadog::CI::TestVisibility::Component + def test_visibility_component: () -> Datadog::CI::TestVisibility::Component? + + def test_optimisation_component: () -> Datadog::CI::TestOptimisation::Component? end end end diff --git a/vendor/rbs/rspec/0/rspec.rbs b/vendor/rbs/rspec/0/rspec.rbs index f4f228ca..5bdc9847 100644 --- a/vendor/rbs/rspec/0/rspec.rbs +++ b/vendor/rbs/rspec/0/rspec.rbs @@ -46,6 +46,7 @@ module RSpec::Core::ExampleGroup def file_path: () -> String def description: () -> String def metadata: () -> untyped + def descendant_filtered_examples: () -> Array[untyped] end end From 1d61ee5dfc097d768bb8fe8d579ed61154e08c2f Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 4 Dec 2024 12:53:11 +0100 Subject: [PATCH 09/10] make sure that skipping context hooks works for nested example groups --- lib/datadog/ci/contrib/rspec/example_group.rb | 10 +++++++--- .../ci/contrib/rspec/instrumentation_spec.rb | 19 +++++++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index bc869668..5050a2b3 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -18,6 +18,13 @@ module ClassMethods def run(reporter = ::RSpec::Core::NullReporter) return super if ::RSpec.configuration.dry_run? && !datadog_configuration[:dry_run_enabled] return super unless datadog_configuration[:enabled] + + # skip the context hooks if all descendant example are going to be skipped + # IMPORTANT: must happen before top_level? check because skipping hooks must happen + # even if it is a nested context + metadata[:skip] = true if all_examples_skipped_by_datadog? + + # return early because we create Datadog::CI::TestSuite only for top-level example groups return super unless top_level? suite_name = "#{description} at #{file_path}" @@ -29,9 +36,6 @@ def run(reporter = ::RSpec::Core::NullReporter) } ) - # skip the context hooks if all descendant example are going to be skipped - metadata[:skip] = true if all_examples_skipped_by_datadog? - success = super return success unless test_suite diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 1fb48312..0cc91277 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -3,6 +3,7 @@ RSpec.describe "RSpec instrumentation" do let(:integration) { Datadog::CI::Contrib::Instrumentation.fetch_integration(:rspec) } let(:before_all_spy) { spy(:before_all, call: nil) } + let(:before_context_spy) { spy(:before_context, call: nil) } before do # expect that public manual API isn't used @@ -19,6 +20,7 @@ def rspec_session_run( with_flaky_test: false, with_canceled_test: false, with_flaky_test_that_fails_once: false, + with_test_outside_context: false, unskippable: { test: false, context: false, @@ -38,6 +40,7 @@ def rspec_session_run( current_let_value = 0 before_all_spy_local = before_all_spy + before_context_spy_local = before_context_spy with_new_rspec_environment do spec = RSpec.describe "SomeTest", suite_meta do @@ -46,6 +49,10 @@ def rspec_session_run( end context "nested", context_meta do + before(:all) do + before_context_spy_local.call + end + let(:let_value) { current_let_value += 1 } it "foo", test_meta do @@ -100,6 +107,12 @@ def rspec_session_run( end end end + + if with_test_outside_context + it "is outside of context" do + expect(1 + 1).to eq(2) + end + end end options_array = %w[--pattern none] @@ -156,7 +169,7 @@ def rspec_session_run( :source_file, "spec/datadog/ci/contrib/rspec/instrumentation_spec.rb" ) - expect(first_test_span).to have_test_tag(:source_start, "129") + expect(first_test_span).to have_test_tag(:source_start, "142") expect(first_test_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -586,7 +599,7 @@ def expect_failure :source_file, "spec/datadog/ci/contrib/rspec/instrumentation_spec.rb" ) - expect(first_test_suite_span).to have_test_tag(:source_start, "43") + expect(first_test_suite_span).to have_test_tag(:source_start, "46") expect(first_test_suite_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -761,6 +774,7 @@ def rspec_skipped_session_run rspec_session_run(with_failed_test: true) expect(before_all_spy).to have_received(:call) + expect(before_context_spy).to have_received(:call) end it "sends test session level tags" do @@ -801,6 +815,7 @@ def rspec_skipped_session_run rspec_session_run(with_failed_test: true) expect(before_all_spy).not_to have_received(:call) + expect(before_context_spy).not_to have_received(:call) end context "but some tests are unskippable" do From 1d9c8049af256dabf6e610d757d7cf9f447a9fb2 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 4 Dec 2024 13:32:05 +0100 Subject: [PATCH 10/10] additional tests for different context hooks scenarios --- .../ci/contrib/rspec/instrumentation_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 0cc91277..957bb716 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -818,6 +818,20 @@ def rspec_skipped_session_run expect(before_context_spy).not_to have_received(:call) end + it "runs top-level hook when there is a test not skipped by datadog, but it skips hooks for the context where all tests are skipped" do + rspec_session_run(with_failed_test: true, with_test_outside_context: true) + + expect(before_all_spy).to have_received(:call) + expect(before_context_spy).not_to have_received(:call) + end + + it "runs context hook if any test is marked unskippable" do + rspec_session_run(with_failed_test: true, unskippable: {test: true}) + + expect(before_all_spy).to have_received(:call) + expect(before_context_spy).to have_received(:call) + end + context "but some tests are unskippable" do context "when a test is unskippable" do it "runs the test and adds forced run tag" do