From d10bd9821e5fde575002d2c51d073e1e77496402 Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 6 Aug 2024 15:05:10 +0200 Subject: [PATCH 1/8] move special ci-queue's test suite handling out of the test execution for rspec --- lib/datadog/ci/contrib/rspec/example.rb | 12 +++++------- .../contrib/ci_queue_rspec/instrumentation_spec.rb | 13 +++++++------ .../suite_under_test/some_test_rspec.rb | 3 +++ 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 0f5a16fd..7b1d9c92 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -37,6 +37,8 @@ def run(*args) test_suite_span = test_visibility_component.start_test_suite(suite_name) end + result = nil + test_visibility_component.trace_test( test_name, suite_name, @@ -60,24 +62,20 @@ def run(*args) case execution_result.status when :passed test_span&.passed! - test_suite_span&.passed! when :failed test_span&.failed!(exception: execution_result.exception) - test_suite_span&.failed! else # :pending or nil test_span&.skipped!( reason: execution_result.pending_message, exception: execution_result.pending_exception ) - - test_suite_span&.skipped! end + end - test_suite_span&.finish + test_suite_span&.finish - result - end + result end private diff --git a/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb index d16ee1dd..5255b664 100644 --- a/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb @@ -20,7 +20,7 @@ RSpec::Core::ConfigurationOptions.new([ "-Ispec/datadog/ci/contrib/ci_queue_rspec/suite_under_test", "--queue", - "list:.%2Fspec%2Fdatadog%2Fci%2Fcontrib%2Fci_queue_rspec%2Fsuite_under_test%2Fsome_test_rspec.rb%5B1%3A1%3A1%5D:.%2Fspec%2Fdatadog%2Fci%2Fcontrib%2Fci_queue_rspec%2Fsuite_under_test%2Fsome_test_rspec.rb%5B1%3A1%3A2%5D", + "list:.%2Fspec%2Fdatadog%2Fci%2Fcontrib%2Fci_queue_rspec%2Fsuite_under_test%2Fsome_test_rspec.rb%5B1%3A1%3A1%5D:.%2Fspec%2Fdatadog%2Fci%2Fcontrib%2Fci_queue_rspec%2Fsuite_under_test%2Fsome_test_rspec.rb%5B1%3A1%3A2%5D:.%2Fspec%2Fdatadog%2Fci%2Fcontrib%2Fci_queue_rspec%2Fsuite_under_test%2Fsome_test_rspec.rb%5B1%3A1%3A3%5D", "--require", "some_test_rspec.rb", "--build", @@ -71,23 +71,24 @@ def with_new_rspec_environment expect([test_session_span, test_module_span]).to all have_fail_status # test suite spans are created for each test as for parallel execution - expect(test_suite_spans).to have(2).items + expect(test_suite_spans).to have(3).items expect(test_suite_spans).to have_tag_values_no_order( :status, - [Datadog::CI::Ext::Test::Status::FAIL, Datadog::CI::Ext::Test::Status::PASS] + [Datadog::CI::Ext::Test::Status::FAIL, Datadog::CI::Ext::Test::Status::PASS, Datadog::CI::Ext::Test::Status::SKIP] ) expect(test_suite_spans).to have_tag_values_no_order( :suite, [ "SomeTest at ./spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb (ci-queue running example [nested fails])", - "SomeTest at ./spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb (ci-queue running example [nested foo])" + "SomeTest at ./spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb (ci-queue running example [nested foo])", + "SomeTest at ./spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb (ci-queue running example [nested is skipped])" ] ) # there is test span for every test case - expect(test_spans).to have(2).items + expect(test_spans).to have(3).items # each test span has its own test suite - expect(test_spans).to have_unique_tag_values_count(:test_suite_id, 2) + expect(test_spans).to have_unique_tag_values_count(:test_suite_id, 3) # every test span is connected to test module and test session expect(test_spans).to all have_test_tag(:test_module_id) diff --git a/spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb b/spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb index 7b775cb9..97e4a95a 100644 --- a/spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb +++ b/spec/datadog/ci/contrib/ci_queue_rspec/suite_under_test/some_test_rspec.rb @@ -9,5 +9,8 @@ it "fails" do expect(1).to eq(2) end + + it "is skipped", skip: true do + end end end From 8f80acd0f05248382c2bbf52d132816ad0a29e4a Mon Sep 17 00:00:00 2001 From: Andrey Date: Tue, 6 Aug 2024 17:08:56 +0200 Subject: [PATCH 2/8] first rough concept of #with_retries method --- lib/datadog/ci/configuration/components.rb | 3 +- lib/datadog/ci/contrib/rspec/example.rb | 70 +++++++++++-------- lib/datadog/ci/test_retries/component.rb | 23 ++++++ lib/datadog/ci/test_retries/null_component.rb | 28 ++++++++ lib/datadog/ci/test_retries/strategy/base.rb | 18 +++++ .../ci/test_retries/strategy/no_retry.rb | 14 ++++ 6 files changed, 124 insertions(+), 32 deletions(-) create mode 100644 lib/datadog/ci/test_retries/null_component.rb create mode 100644 lib/datadog/ci/test_retries/strategy/base.rb create mode 100644 lib/datadog/ci/test_retries/strategy/no_retry.rb diff --git a/lib/datadog/ci/configuration/components.rb b/lib/datadog/ci/configuration/components.rb index 3c45ea19..7c6416d3 100644 --- a/lib/datadog/ci/configuration/components.rb +++ b/lib/datadog/ci/configuration/components.rb @@ -10,6 +10,7 @@ require_relative "../test_optimisation/coverage/transport" require_relative "../test_optimisation/coverage/writer" require_relative "../test_retries/component" +require_relative "../test_retries/null_component" require_relative "../test_visibility/component" require_relative "../test_visibility/flush" require_relative "../test_visibility/null_component" @@ -35,7 +36,7 @@ def initialize(settings) @test_visibility = TestVisibility::NullComponent.new @git_tree_upload_worker = DummyWorker.new @ci_remote = nil - @test_retries = nil + @test_retries = TestRetries::NullComponent.new # Activate CI mode if enabled if settings.ci.enabled diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 7b1d9c92..d8123c9f 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -39,37 +39,41 @@ def run(*args) result = nil - test_visibility_component.trace_test( - test_name, - suite_name, - tags: { - CI::Ext::Test::TAG_FRAMEWORK => Ext::FRAMEWORK, - CI::Ext::Test::TAG_FRAMEWORK_VERSION => CI::Contrib::RSpec::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]} - ) - }, - service: datadog_configuration[:service_name] - ) do |test_span| - test_span&.itr_unskippable! if metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] - - metadata[:skip] = CI::Ext::Test::ITR_TEST_SKIP_REASON if test_span&.skipped_by_itr? - - result = super - - case execution_result.status - when :passed - test_span&.passed! - when :failed - test_span&.failed!(exception: execution_result.exception) - else - # :pending or nil - test_span&.skipped!( - reason: execution_result.pending_message, - exception: execution_result.pending_exception - ) + test_retries_component.with_retries do |retry_callback| + test_visibility_component.trace_test( + test_name, + suite_name, + tags: { + CI::Ext::Test::TAG_FRAMEWORK => Ext::FRAMEWORK, + CI::Ext::Test::TAG_FRAMEWORK_VERSION => CI::Contrib::RSpec::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]} + ) + }, + service: datadog_configuration[:service_name] + ) do |test_span| + test_span&.itr_unskippable! if metadata[CI::Ext::Test::ITR_UNSKIPPABLE_OPTION] + + metadata[:skip] = CI::Ext::Test::ITR_TEST_SKIP_REASON if test_span&.skipped_by_itr? + + result = super + + case execution_result.status + when :passed + test_span&.passed! + when :failed + test_span&.failed!(exception: execution_result.exception) + else + # :pending or nil + test_span&.skipped!( + reason: execution_result.pending_message, + exception: execution_result.pending_exception + ) + end + + retry_callback.call(test_span) end end @@ -101,6 +105,10 @@ def test_visibility_component Datadog.send(:components).test_visibility end + def test_retries_component + Datadog.send(:components).test_retries + end + def ci_queue? !!defined?(::RSpec::Queue::ExampleExtension) && self.class.ancestors.include?(::RSpec::Queue::ExampleExtension) diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index b04a3938..1486d704 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "strategy/no_retry" + module Datadog module CI module TestRetries @@ -17,11 +19,32 @@ def initialize( @retry_failed_tests_enabled = false @retry_failed_tests_max_attempts = retry_failed_tests_max_attempts @retry_failed_tests_total_limit = retry_failed_tests_total_limit + + # TODO: increment in #build_strategy method if test failed and should be retried + @retry_failed_tests_count = 0 end def configure(library_settings) @retry_failed_tests_enabled = library_settings.flaky_test_retries_enabled? end + + def with_retries(&block) + retry_strategy = nil + + finished_proc = proc do |test_span| + if retry_strategy.nil? + retry_strategy = Strategy::NoRetry.new + else + retry_strategy.track_retry(test_span) + end + end + + loop do + yield finished_proc + + break unless retry_strategy&.should_retry? + end + end end end end diff --git a/lib/datadog/ci/test_retries/null_component.rb b/lib/datadog/ci/test_retries/null_component.rb new file mode 100644 index 00000000..fb2f7140 --- /dev/null +++ b/lib/datadog/ci/test_retries/null_component.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require_relative "component" + +module Datadog + module CI + module TestRetries + class NullComponent < Component + attr_reader :retry_failed_tests_enabled, :retry_failed_tests_max_attempts, :retry_failed_tests_total_limit + + def initialize + # enabled only by remote settings + @retry_failed_tests_enabled = false + @retry_failed_tests_max_attempts = 0 + @retry_failed_tests_total_limit = 0 + end + + def configure(library_settings) + end + + def with_retries(&block) + no_action = proc {} + yield no_action + end + end + end + end +end diff --git a/lib/datadog/ci/test_retries/strategy/base.rb b/lib/datadog/ci/test_retries/strategy/base.rb new file mode 100644 index 00000000..bed10636 --- /dev/null +++ b/lib/datadog/ci/test_retries/strategy/base.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module Datadog + module CI + module TestRetries + module Strategy + class Base + def should_retry? + false + end + + def track_retry(test_span) + end + end + end + end + end +end diff --git a/lib/datadog/ci/test_retries/strategy/no_retry.rb b/lib/datadog/ci/test_retries/strategy/no_retry.rb new file mode 100644 index 00000000..5c386d9e --- /dev/null +++ b/lib/datadog/ci/test_retries/strategy/no_retry.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require_relative "base" + +module Datadog + module CI + module TestRetries + module Strategy + class NoRetry < Base + end + end + end + end +end From 3dfd553cee605c55fe312855ba590ed10c0f0c83 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 7 Aug 2024 15:39:28 +0200 Subject: [PATCH 3/8] first working RSpec test retries prototype --- lib/datadog/ci/contrib/rspec/example.rb | 25 +++- lib/datadog/ci/test_retries/component.rb | 30 ++++- lib/datadog/ci/test_retries/strategy/base.rb | 3 +- .../ci/test_retries/strategy/no_retry.rb | 2 + .../ci/test_retries/strategy/retry_failed.rb | 33 +++++ sig/datadog/ci/contrib/rspec/example.rbs | 5 +- sig/datadog/ci/test_retries/component.rbs | 10 ++ .../ci/test_retries/null_component.rbs | 25 ++++ sig/datadog/ci/test_retries/strategy/base.rbs | 13 ++ .../ci/test_retries/strategy/no_retry.rbs | 11 ++ .../ci/test_retries/strategy/retry_failed.rbs | 21 +++ .../ci_queue_rspec/instrumentation_spec.rb | 5 +- .../ci/contrib/rspec/instrumentation_spec.rb | 125 +++++++++++++++++- vendor/rbs/rspec/0/rspec.rbs | 5 + 14 files changed, 298 insertions(+), 15 deletions(-) create mode 100644 lib/datadog/ci/test_retries/strategy/retry_failed.rb create mode 100644 sig/datadog/ci/test_retries/null_component.rbs create mode 100644 sig/datadog/ci/test_retries/strategy/base.rbs create mode 100644 sig/datadog/ci/test_retries/strategy/no_retry.rbs create mode 100644 sig/datadog/ci/test_retries/strategy/retry_failed.rbs diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index d8123c9f..084137db 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -37,8 +37,6 @@ def run(*args) test_suite_span = test_visibility_component.start_test_suite(suite_name) end - result = nil - test_retries_component.with_retries do |retry_callback| test_visibility_component.trace_test( test_name, @@ -58,7 +56,12 @@ def run(*args) metadata[:skip] = CI::Ext::Test::ITR_TEST_SKIP_REASON if test_span&.skipped_by_itr? - result = super + # before each run remove any previous exception + @exception = nil + # don't report test to RSpec::Core::Reporter until retries are done + @skip_reporting = true + + super case execution_result.status when :passed @@ -77,9 +80,23 @@ def run(*args) end end + # after retries are done, we can report the test to RSpec + @skip_reporting = false + test_suite_span&.finish - result + # Finish spec with latest retry's result + # TODO: when implementing new test retries make sure to clean @exception before calling this method + # if test passed at least once + finish(reporter) + end + + def finish(reporter) + # by default finish test but do not report it to RSpec::Core::Reporter + # it is going to be reported once after retries are done + return super unless @skip_reporting + + super(::RSpec::Core::NullReporter) end private diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index 1486d704..dfcb39c3 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "strategy/no_retry" +require_relative "strategy/retry_failed" module Datadog module CI @@ -20,7 +21,6 @@ def initialize( @retry_failed_tests_max_attempts = retry_failed_tests_max_attempts @retry_failed_tests_total_limit = retry_failed_tests_total_limit - # TODO: increment in #build_strategy method if test failed and should be retried @retry_failed_tests_count = 0 end @@ -29,22 +29,42 @@ def configure(library_settings) end def with_retries(&block) + # @type var retry_strategy: Strategy::Base retry_strategy = nil - finished_proc = proc do |test_span| + test_finished_callback = lambda do |test_span| if retry_strategy.nil? - retry_strategy = Strategy::NoRetry.new + # we always run test at least once and after first pass create a correct retry strategy + retry_strategy = build_strategy(test_span) else - retry_strategy.track_retry(test_span) + # after each retry we record the result, strategy will decide if we should retry again + retry_strategy&.record_retry(test_span) end end loop do - yield finished_proc + yield test_finished_callback break unless retry_strategy&.should_retry? end end + + # TODO: synchronize this! This object is shared between threads + def build_strategy(test_span) + if should_retry_failed_test?(test_span) + @retry_failed_tests_count += 1 + + Strategy::RetryFailed.new(max_attempts: @retry_failed_tests_max_attempts) + else + Strategy::NoRetry.new + end + end + + private + + def should_retry_failed_test?(test_span) + @retry_failed_tests_enabled && !!test_span&.failed? && @retry_failed_tests_count < @retry_failed_tests_total_limit + end end end end diff --git a/lib/datadog/ci/test_retries/strategy/base.rb b/lib/datadog/ci/test_retries/strategy/base.rb index bed10636..3c69fd84 100644 --- a/lib/datadog/ci/test_retries/strategy/base.rb +++ b/lib/datadog/ci/test_retries/strategy/base.rb @@ -9,7 +9,8 @@ def should_retry? false end - def track_retry(test_span) + def record_retry(test_span) + test_span&.set_tag(Ext::Test::TAG_IS_RETRY, "true") end end end diff --git a/lib/datadog/ci/test_retries/strategy/no_retry.rb b/lib/datadog/ci/test_retries/strategy/no_retry.rb index 5c386d9e..1a4541d8 100644 --- a/lib/datadog/ci/test_retries/strategy/no_retry.rb +++ b/lib/datadog/ci/test_retries/strategy/no_retry.rb @@ -7,6 +7,8 @@ module CI module TestRetries module Strategy class NoRetry < Base + def record_retry(test_span) + end end end end diff --git a/lib/datadog/ci/test_retries/strategy/retry_failed.rb b/lib/datadog/ci/test_retries/strategy/retry_failed.rb new file mode 100644 index 00000000..cc753cad --- /dev/null +++ b/lib/datadog/ci/test_retries/strategy/retry_failed.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require_relative "base" + +require_relative "../../ext/test" + +module Datadog + module CI + module TestRetries + module Strategy + class RetryFailed < Base + def initialize(max_attempts:) + @max_attempts = max_attempts + + @attempts = 0 + @passed_once = false + end + + def should_retry? + @attempts < @max_attempts && !@passed_once + end + + def record_retry(test_span) + super + + @attempts += 1 + @passed_once = true if test_span&.passed? + end + end + end + end + end +end diff --git a/sig/datadog/ci/contrib/rspec/example.rbs b/sig/datadog/ci/contrib/rspec/example.rbs index 209e019c..a374c150 100644 --- a/sig/datadog/ci/contrib/rspec/example.rbs +++ b/sig/datadog/ci/contrib/rspec/example.rbs @@ -4,8 +4,8 @@ module Datadog module RSpec module Example def self.included: (untyped base) -> untyped - module InstanceMethods - include ::RSpec::Core::Example + module InstanceMethods : ::RSpec::Core::Example + @skip_reporting: bool def run: (untyped example_group_instance, untyped reporter) -> untyped @@ -14,6 +14,7 @@ module Datadog def fetch_top_level_example_group: () -> Hash[Symbol, untyped] def datadog_configuration: () -> untyped def test_visibility_component: () -> Datadog::CI::TestVisibility::Component + def test_retries_component: () -> Datadog::CI::TestRetries::Component def ci_queue?: () -> bool end end diff --git a/sig/datadog/ci/test_retries/component.rbs b/sig/datadog/ci/test_retries/component.rbs index 9d902177..8ef073fa 100644 --- a/sig/datadog/ci/test_retries/component.rbs +++ b/sig/datadog/ci/test_retries/component.rbs @@ -8,9 +8,19 @@ module Datadog attr_reader retry_failed_tests_total_limit: Integer + @retry_failed_tests_count: Integer + def initialize: (retry_failed_tests_max_attempts: Integer, retry_failed_tests_total_limit: Integer) -> void def configure: (Datadog::CI::Remote::LibrarySettings library_settings) -> void + + def with_retries: () { (untyped) -> void } -> void + + def build_strategy: (Datadog::CI::Test test) -> Datadog::CI::TestRetries::Strategy::Base + + private + + def should_retry_failed_test?: (Datadog::CI::Test test) -> bool end end end diff --git a/sig/datadog/ci/test_retries/null_component.rbs b/sig/datadog/ci/test_retries/null_component.rbs new file mode 100644 index 00000000..23a1d0bc --- /dev/null +++ b/sig/datadog/ci/test_retries/null_component.rbs @@ -0,0 +1,25 @@ +module Datadog + module CI + module TestRetries + class NullComponent < Component + @retry_failed_tests_enabled: untyped + + @retry_failed_tests_max_attempts: untyped + + @retry_failed_tests_total_limit: untyped + + attr_reader retry_failed_tests_enabled: untyped + + attr_reader retry_failed_tests_max_attempts: untyped + + attr_reader retry_failed_tests_total_limit: untyped + + def initialize: () -> void + + def configure: (untyped library_settings) -> nil + + def with_retries: () { (untyped) -> untyped } -> untyped + end + end + end +end diff --git a/sig/datadog/ci/test_retries/strategy/base.rbs b/sig/datadog/ci/test_retries/strategy/base.rbs new file mode 100644 index 00000000..8440b3e3 --- /dev/null +++ b/sig/datadog/ci/test_retries/strategy/base.rbs @@ -0,0 +1,13 @@ +module Datadog + module CI + module TestRetries + module Strategy + class Base + def should_retry?: () -> bool + + def record_retry: (Datadog::CI::Test test_span) -> void + end + end + end + end +end diff --git a/sig/datadog/ci/test_retries/strategy/no_retry.rbs b/sig/datadog/ci/test_retries/strategy/no_retry.rbs new file mode 100644 index 00000000..25ae4c6b --- /dev/null +++ b/sig/datadog/ci/test_retries/strategy/no_retry.rbs @@ -0,0 +1,11 @@ +module Datadog + module CI + module TestRetries + module Strategy + class NoRetry < Base + def record_retry: (Datadog::CI::Test test_span) -> void + end + end + end + end +end diff --git a/sig/datadog/ci/test_retries/strategy/retry_failed.rbs b/sig/datadog/ci/test_retries/strategy/retry_failed.rbs new file mode 100644 index 00000000..02c90b13 --- /dev/null +++ b/sig/datadog/ci/test_retries/strategy/retry_failed.rbs @@ -0,0 +1,21 @@ +module Datadog + module CI + module TestRetries + module Strategy + class RetryFailed < Base + @max_attempts: Integer + + @attempts: Integer + + @passed_once: bool + + def initialize: (max_attempts: Integer) -> void + + def should_retry?: () -> bool + + def record_retry: (Datadog::CI::Test test_span) -> void + end + end + end + end +end diff --git a/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb index 5255b664..3131c505 100644 --- a/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/ci_queue_rspec/instrumentation_spec.rb @@ -13,6 +13,7 @@ include_context "CI mode activated" do let(:integration_name) { :rspec } + let(:flaky_test_retries_enabled) { true } end let(:run_id) { SecureRandom.random_number(2**64 - 1) } @@ -85,8 +86,8 @@ def with_new_rspec_environment ] ) - # there is test span for every test case - expect(test_spans).to have(3).items + # there is test span for every test case + 5 retries + expect(test_spans).to have(8).items # each test span has its own test suite expect(test_spans).to have_unique_tag_values_count(:test_suite_id, 3) diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index fc301ce8..c64fd235 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -31,6 +31,7 @@ def rspec_session_run( with_failed_test: false, with_shared_test: false, with_shared_context: false, + with_flaky_test: false, unskippable: { test: false, context: false, @@ -41,6 +42,10 @@ def rspec_session_run( test_meta = unskippable[:test] ? {Datadog::CI::Ext::Test::ITR_UNSKIPPABLE_OPTION => true} : {} context_meta = unskippable[:context] ? {Datadog::CI::Ext::Test::ITR_UNSKIPPABLE_OPTION => true} : {} suite_meta = unskippable[:suite] ? {Datadog::CI::Ext::Test::ITR_UNSKIPPABLE_OPTION => true} : {} + + max_flaky_test_failures = 4 + flaky_test_failures = 0 + with_new_rspec_environment do spec = RSpec.describe "SomeTest", suite_meta do context "nested", context_meta do @@ -64,6 +69,17 @@ def rspec_session_run( require_relative "some_shared_context" include_context "Shared context" end + + if with_flaky_test + it "flaky" do + if flaky_test_failures < max_flaky_test_failures + flaky_test_failures += 1 + expect(1 + 1).to eq(3) + else + expect(1 + 1).to eq(2) + end + end + end end end @@ -117,7 +133,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, "90") + expect(first_test_span).to have_test_tag(:source_start, "106") expect(first_test_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -800,4 +816,111 @@ def rspec_skipped_session_run expect(test_spans).to be_empty end end + + context "session with flaky spec and failed test retries enabled" do + include_context "CI mode activated" do + let(:integration_name) { :rspec } + let(:integration_options) { {service_name: "lspec"} } + + let(:flaky_test_retries_enabled) { true } + end + + it "retries test until it passes" do + rspec_session_run(with_flaky_test: true) + + # 1 initial run of flaky test + 4 retries until pass + 1 passing test = 6 spans + expect(test_spans).to have(6).items + + failed_spans, passed_spans = test_spans.partition { |span| span.get_tag("test.status") == "fail" } + expect(failed_spans).to have(4).items # see steps.rb + expect(passed_spans).to have(2).items + + test_spans_by_test_name = test_spans.group_by { |span| span.get_tag("test.name") } + expect(test_spans_by_test_name["nested flaky"]).to have(5).items + + # count how many spans were marked as retries + retries_count = test_spans.count { |span| span.get_tag("test.is_retry") == "true" } + expect(retries_count).to eq(4) + + expect(test_spans_by_test_name["nested foo"]).to have(1).item + + expect(test_suite_spans).to have(1).item + expect(test_suite_spans.first).to have_pass_status + + expect(test_session_span).to have_pass_status + end + end + + context "session with flaky spec and failed test retries enabled with insufficient retries limit" do + include_context "CI mode activated" do + let(:integration_name) { :rspec } + let(:integration_options) { {service_name: "lspec"} } + + let(:flaky_test_retries_enabled) { true } + let(:retry_failed_tests_max_attempts) { 3 } + end + + it "retries test until it passes" do + rspec_session_run(with_flaky_test: true) + + # 1 initial run of flaky test + 3 unsuccessful retries + 1 passing test = 5 spans + expect(test_spans).to have(5).items + + failed_spans, passed_spans = test_spans.partition { |span| span.get_tag("test.status") == "fail" } + expect(failed_spans).to have(4).items + expect(passed_spans).to have(1).items + + test_spans_by_test_name = test_spans.group_by { |span| span.get_tag("test.name") } + expect(test_spans_by_test_name["nested flaky"]).to have(4).items + + # count how many spans were marked as retries + retries_count = test_spans.count { |span| span.get_tag("test.is_retry") == "true" } + expect(retries_count).to eq(3) + + expect(test_spans_by_test_name["nested foo"]).to have(1).item + + expect(test_suite_spans).to have(1).item + expect(test_suite_spans.first).to have_fail_status + + expect(test_session_span).to have_fail_status + end + end + + context "session with flaky and failed specs and failed test retries enabled with low overall retries limit" do + include_context "CI mode activated" do + let(:integration_name) { :rspec } + let(:integration_options) { {service_name: "lspec"} } + + let(:flaky_test_retries_enabled) { true } + let(:retry_failed_tests_total_limit) { 1 } + end + + it "retries failed test with no success and bails out of retrying flaky test" do + rspec_session_run(with_flaky_test: true, with_failed_test: true) + + # 1 passing test + 1 failed test + 5 unsuccessful retries + 1 failed run of flaky test without retries = 8 spans + expect(test_spans).to have(8).items + + failed_spans, passed_spans = test_spans.partition { |span| span.get_tag("test.status") == "fail" } + expect(failed_spans).to have(7).items + expect(passed_spans).to have(1).items + + test_spans_by_test_name = test_spans.group_by { |span| span.get_tag("test.name") } + + # it bailed out of retrying flaky test because global failed tests limit was exhausted already + expect(test_spans_by_test_name["nested flaky"]).to have(1).item + + # count how many spans were marked as retries + retries_count = test_spans.count { |span| span.get_tag("test.is_retry") == "true" } + expect(retries_count).to eq(5) + + # it retried failing test 5 times + expect(test_spans_by_test_name["nested fails"]).to have(6).items + + expect(test_suite_spans).to have(1).item + expect(test_suite_spans.first).to have_fail_status + + expect(test_session_span).to have_fail_status + end + end end diff --git a/vendor/rbs/rspec/0/rspec.rbs b/vendor/rbs/rspec/0/rspec.rbs index d08273cc..c8d6c587 100644 --- a/vendor/rbs/rspec/0/rspec.rbs +++ b/vendor/rbs/rspec/0/rspec.rbs @@ -15,11 +15,16 @@ module RSpec::Queue::ExampleExtension end module RSpec::Core::Example + @exception: untyped + + attr_reader reporter: untyped + def run: () -> untyped def execution_result: () -> untyped def metadata: () -> untyped def description: () -> String def full_description: () -> String + def finish: (untyped) -> void end module RSpec::Core::Runner From 426a243de163eb5b5ba4093db54c02f88b8071cf Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 8 Aug 2024 15:47:31 +0200 Subject: [PATCH 4/8] more unit tests for test retry classes --- lib/datadog/ci/contrib/rspec/example.rb | 10 +- lib/datadog/ci/remote/library_settings.rb | 3 + lib/datadog/ci/test_retries/component.rb | 20 ++-- .../ci/test_retries/strategy/retry_failed.rb | 2 + sig/datadog/ci/test_retries/component.rbs | 4 +- .../ci/test_retries/strategy/retry_failed.rbs | 2 +- .../datadog/ci/test_retries/component_spec.rb | 109 ++++++++++++++++++ .../strategy/retry_failed_spec.rb | 30 +++++ 8 files changed, 166 insertions(+), 14 deletions(-) create mode 100644 spec/datadog/ci/test_retries/strategy/retry_failed_spec.rb diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 084137db..26a2197d 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -34,9 +34,12 @@ def run(*args) if ci_queue? suite_name = "#{suite_name} (ci-queue running example [#{test_name}])" - test_suite_span = test_visibility_component.start_test_suite(suite_name) + ci_queue_test_span = test_visibility_component.start_test_suite(suite_name) end + # don't report test to RSpec::Core::Reporter until retries are done + @skip_reporting = true + test_retries_component.with_retries do |retry_callback| test_visibility_component.trace_test( test_name, @@ -58,8 +61,6 @@ def run(*args) # before each run remove any previous exception @exception = nil - # don't report test to RSpec::Core::Reporter until retries are done - @skip_reporting = true super @@ -83,7 +84,8 @@ def run(*args) # after retries are done, we can report the test to RSpec @skip_reporting = false - test_suite_span&.finish + # this is a special case for ci-queue, we need to finish the test suite span + ci_queue_test_span&.finish # Finish spec with latest retry's result # TODO: when implementing new test retries make sure to clean @exception before calling this method diff --git a/lib/datadog/ci/remote/library_settings.rb b/lib/datadog/ci/remote/library_settings.rb index 63984e20..cac57ff8 100644 --- a/lib/datadog/ci/remote/library_settings.rb +++ b/lib/datadog/ci/remote/library_settings.rb @@ -71,6 +71,9 @@ def tests_skipping_enabled? end def flaky_test_retries_enabled? + # DO NOT COMMIT THIS LINE: + return true + return @flaky_test_retries_enabled if defined?(@flaky_test_retries_enabled) @flaky_test_retries_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY) diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index dfcb39c3..231615a8 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -10,7 +10,8 @@ module TestRetries # - retrying failed tests - improve success rate of CI pipelines # - retrying new tests - detect flaky tests as early as possible to prevent them from being merged class Component - attr_reader :retry_failed_tests_enabled, :retry_failed_tests_max_attempts, :retry_failed_tests_total_limit + attr_reader :retry_failed_tests_enabled, :retry_failed_tests_max_attempts, + :retry_failed_tests_total_limit, :retry_failed_tests_count def initialize( retry_failed_tests_max_attempts:, @@ -20,8 +21,10 @@ def initialize( @retry_failed_tests_enabled = false @retry_failed_tests_max_attempts = retry_failed_tests_max_attempts @retry_failed_tests_total_limit = retry_failed_tests_total_limit - + # counter that store the current number of failed tests retried @retry_failed_tests_count = 0 + + @mutex = Mutex.new end def configure(library_settings) @@ -49,14 +52,15 @@ def with_retries(&block) end end - # TODO: synchronize this! This object is shared between threads def build_strategy(test_span) - if should_retry_failed_test?(test_span) - @retry_failed_tests_count += 1 + @mutex.synchronize do + if should_retry_failed_test?(test_span) + @retry_failed_tests_count += 1 - Strategy::RetryFailed.new(max_attempts: @retry_failed_tests_max_attempts) - else - Strategy::NoRetry.new + Strategy::RetryFailed.new(max_attempts: @retry_failed_tests_max_attempts) + else + Strategy::NoRetry.new + end end end diff --git a/lib/datadog/ci/test_retries/strategy/retry_failed.rb b/lib/datadog/ci/test_retries/strategy/retry_failed.rb index cc753cad..bdfa1579 100644 --- a/lib/datadog/ci/test_retries/strategy/retry_failed.rb +++ b/lib/datadog/ci/test_retries/strategy/retry_failed.rb @@ -9,6 +9,8 @@ module CI module TestRetries module Strategy class RetryFailed < Base + attr_reader :max_attempts + def initialize(max_attempts:) @max_attempts = max_attempts diff --git a/sig/datadog/ci/test_retries/component.rbs b/sig/datadog/ci/test_retries/component.rbs index 8ef073fa..29760aee 100644 --- a/sig/datadog/ci/test_retries/component.rbs +++ b/sig/datadog/ci/test_retries/component.rbs @@ -8,7 +8,9 @@ module Datadog attr_reader retry_failed_tests_total_limit: Integer - @retry_failed_tests_count: Integer + attr_reader retry_failed_tests_count: Integer + + @mutex: Thread::Mutex def initialize: (retry_failed_tests_max_attempts: Integer, retry_failed_tests_total_limit: Integer) -> void diff --git a/sig/datadog/ci/test_retries/strategy/retry_failed.rbs b/sig/datadog/ci/test_retries/strategy/retry_failed.rbs index 02c90b13..057f121e 100644 --- a/sig/datadog/ci/test_retries/strategy/retry_failed.rbs +++ b/sig/datadog/ci/test_retries/strategy/retry_failed.rbs @@ -3,7 +3,7 @@ module Datadog module TestRetries module Strategy class RetryFailed < Base - @max_attempts: Integer + attr_reader max_attempts: Integer @attempts: Integer diff --git a/spec/datadog/ci/test_retries/component_spec.rb b/spec/datadog/ci/test_retries/component_spec.rb index 84ce74bd..933e4e95 100644 --- a/spec/datadog/ci/test_retries/component_spec.rb +++ b/spec/datadog/ci/test_retries/component_spec.rb @@ -51,4 +51,113 @@ it { is_expected.to eq(retry_failed_tests_total_limit) } end + + describe "#build_strategy" do + subject { component.build_strategy(test_span) } + + let(:test_failed) { false } + let(:test_span) { instance_double(Datadog::CI::Test, failed?: test_failed) } + + before do + component.configure(library_settings) + end + + context "when retry failed tests is enabled" do + let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: true) } + + context "when test span is failed" do + let(:test_failed) { true } + + context "when failed tests retry limit is not reached" do + let(:retry_failed_tests_total_limit) { 1 } + + it "creates RetryFailed strategy" do + expect(subject).to be_a(Datadog::CI::TestRetries::Strategy::RetryFailed) + expect(subject.max_attempts).to eq(retry_failed_tests_max_attempts) + end + end + + context "when failed tests retry limit is reached" do + let(:retry_failed_tests_total_limit) { 1 } + + before do + component.build_strategy(test_span) + end + + it { is_expected.to be_a(Datadog::CI::TestRetries::Strategy::NoRetry) } + end + + context "when failed tests retry limit is reached with multithreading test runner" do + let(:threads_count) { 10 } + let(:retry_failed_tests_total_limit) { threads_count } + + before do + threads = (1..threads_count).map do + Thread.new { component.build_strategy(test_span) } + end + + threads.each(&:join) + end + + it "correctly exhausts failed tests limit" do + is_expected.to be_a(Datadog::CI::TestRetries::Strategy::NoRetry) + end + end + end + + context "when test span is passed" do + let(:test_failed) { false } + + it { is_expected.to be_a(Datadog::CI::TestRetries::Strategy::NoRetry) } + end + end + + context "when retry failed tests is disabled" do + let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: false) } + + it { is_expected.to be_a(Datadog::CI::TestRetries::Strategy::NoRetry) } + end + end + + describe "#with_retries" do + let(:test_failed) { false } + let(:test_span) { instance_double(Datadog::CI::Test, failed?: test_failed, passed?: false, set_tag: true) } + + subject(:runs_count) do + runs_count = 0 + component.with_retries do |test_finished_callback| + runs_count += 1 + test_finished_callback.call(test_span) + end + + runs_count + end + + before do + component.configure(library_settings) + end + + context "when no retries strategy is used" do + let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: false) } + + it { is_expected.to eq(1) } + end + + context "when retried failed tests strategy is used" do + let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: true) } + + context "when test span is failed" do + let(:test_failed) { true } + let(:retry_failed_tests_max_attempts) { 4 } + + it { is_expected.to eq(retry_failed_tests_max_attempts + 1) } + end + + context "when test span is passed" do + let(:test_failed) { false } + + it { is_expected.to eq(1) } + end + end + end end diff --git a/spec/datadog/ci/test_retries/strategy/retry_failed_spec.rb b/spec/datadog/ci/test_retries/strategy/retry_failed_spec.rb new file mode 100644 index 00000000..9a8d072e --- /dev/null +++ b/spec/datadog/ci/test_retries/strategy/retry_failed_spec.rb @@ -0,0 +1,30 @@ +require_relative "../../../../../lib/datadog/ci/test_retries/strategy/retry_failed" + +RSpec.describe Datadog::CI::TestRetries::Strategy::RetryFailed do + let(:max_attempts) { 3 } + subject(:strategy) { described_class.new(max_attempts: max_attempts) } + + describe "#should_retry?" do + subject { strategy.should_retry? } + + context "when the test has not passed yet" do + let(:test_span) { double(:test_span, set_tag: true, passed?: false) } + + it { is_expected.to be true } + + context "when the max attempts have been reached" do + before { max_attempts.times { strategy.record_retry(test_span) } } + + it { is_expected.to be false } + end + end + + context "when the test has passed" do + let(:test_span) { double(:test_span, set_tag: true, passed?: true) } + + before { strategy.record_retry(test_span) } + + it { is_expected.to be false } + end + end +end From c5f88ba3cfae8f396dcbefe34d34d2828e349956 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 8 Aug 2024 15:48:47 +0200 Subject: [PATCH 5/8] remove the line that should not have been committed --- lib/datadog/ci/remote/library_settings.rb | 3 -- .../steps_10966227753502832788.rb | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb diff --git a/lib/datadog/ci/remote/library_settings.rb b/lib/datadog/ci/remote/library_settings.rb index cac57ff8..63984e20 100644 --- a/lib/datadog/ci/remote/library_settings.rb +++ b/lib/datadog/ci/remote/library_settings.rb @@ -71,9 +71,6 @@ def tests_skipping_enabled? end def flaky_test_retries_enabled? - # DO NOT COMMIT THIS LINE: - return true - return @flaky_test_retries_enabled if defined?(@flaky_test_retries_enabled) @flaky_test_retries_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY) diff --git a/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb b/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb new file mode 100644 index 00000000..84fe8ff6 --- /dev/null +++ b/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb @@ -0,0 +1,47 @@ +require_relative "helpers/helper" + +max_flaky_test_failures = 4 +flaky_test_executions = 0 + +Before do + Datadog::CI.active_test.set_tag("cucumber_before_hook_executed", "true") +end + +After do + Datadog::CI.active_test.set_tag("cucumber_after_hook_executed", "true") +end + +AfterStep do + Datadog::CI.active_test.set_tag("cucumber_after_step_hook_executed", "true") +end + +Then "datadog" do + Helper.help? +end + +Then "failure" do + expect(1 + 1).to eq(3) +end + +Then "pending" do + pending("implementation") +end + +Then "skip" do + skip_this_scenario +end + +Then(/I add (-?\d+) and (-?\d+)/) do |n1, n2| + @res = n1.to_i + n2.to_i +end + +Then(/the result should be (-?\d+)/) do |res| + expect(@res).to eq(res.to_i) +end + +Then "flaky" do + if flaky_test_executions < max_flaky_test_failures + flaky_test_executions += 1 + raise "Flaky test failure" + end +end From 30dd0cb43037d6f661be41b7e2f219275862d7a6 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 8 Aug 2024 15:49:19 +0200 Subject: [PATCH 6/8] remove autogenerated file --- .../steps_10966227753502832788.rb | 47 ------------------- 1 file changed, 47 deletions(-) delete mode 100644 spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb diff --git a/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb b/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb deleted file mode 100644 index 84fe8ff6..00000000 --- a/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps_10966227753502832788.rb +++ /dev/null @@ -1,47 +0,0 @@ -require_relative "helpers/helper" - -max_flaky_test_failures = 4 -flaky_test_executions = 0 - -Before do - Datadog::CI.active_test.set_tag("cucumber_before_hook_executed", "true") -end - -After do - Datadog::CI.active_test.set_tag("cucumber_after_hook_executed", "true") -end - -AfterStep do - Datadog::CI.active_test.set_tag("cucumber_after_step_hook_executed", "true") -end - -Then "datadog" do - Helper.help? -end - -Then "failure" do - expect(1 + 1).to eq(3) -end - -Then "pending" do - pending("implementation") -end - -Then "skip" do - skip_this_scenario -end - -Then(/I add (-?\d+) and (-?\d+)/) do |n1, n2| - @res = n1.to_i + n2.to_i -end - -Then(/the result should be (-?\d+)/) do |res| - expect(@res).to eq(res.to_i) -end - -Then "flaky" do - if flaky_test_executions < max_flaky_test_failures - flaky_test_executions += 1 - raise "Flaky test failure" - end -end From abed84131c443e3f687e3246faa16d12b75aa387 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 8 Aug 2024 15:51:59 +0200 Subject: [PATCH 7/8] add debug logging --- lib/datadog/ci/test_retries/component.rb | 1 + lib/datadog/ci/test_retries/strategy/retry_failed.rb | 2 ++ 2 files changed, 3 insertions(+) diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index 231615a8..27c74428 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -55,6 +55,7 @@ def with_retries(&block) def build_strategy(test_span) @mutex.synchronize do if should_retry_failed_test?(test_span) + Datadog.logger.debug("Failed test retry starts") @retry_failed_tests_count += 1 Strategy::RetryFailed.new(max_attempts: @retry_failed_tests_max_attempts) diff --git a/lib/datadog/ci/test_retries/strategy/retry_failed.rb b/lib/datadog/ci/test_retries/strategy/retry_failed.rb index bdfa1579..e5fa7fa4 100644 --- a/lib/datadog/ci/test_retries/strategy/retry_failed.rb +++ b/lib/datadog/ci/test_retries/strategy/retry_failed.rb @@ -27,6 +27,8 @@ def record_retry(test_span) @attempts += 1 @passed_once = true if test_span&.passed? + + Datadog.logger.debug { "Retry Attempts [#{@attempts} / #{@max_attempts}], Passed: [#{@passed_once}]" } end end end From 9e6f62e3a6a0c3c27a04adf071d7bed74c4e9250 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 8 Aug 2024 16:20:18 +0200 Subject: [PATCH 8/8] test that memoized values are cleared between retries --- spec/datadog/ci/contrib/rspec/instrumentation_spec.rb | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index c64fd235..0f472772 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -46,9 +46,13 @@ def rspec_session_run( max_flaky_test_failures = 4 flaky_test_failures = 0 + current_let_value = 0 + with_new_rspec_environment do spec = RSpec.describe "SomeTest", suite_meta do context "nested", context_meta do + let(:let_value) { current_let_value += 1 } + it "foo", test_meta do expect(1 + 1).to eq(2) end @@ -72,6 +76,7 @@ def rspec_session_run( if with_flaky_test it "flaky" do + Datadog::CI.active_test&.set_tag("let_value", let_value) if flaky_test_failures < max_flaky_test_failures flaky_test_failures += 1 expect(1 + 1).to eq(3) @@ -133,7 +138,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, "106") + expect(first_test_span).to have_test_tag(:source_start, "111") expect(first_test_span).to have_test_tag( :codeowners, "[\"@DataDog/ruby-guild\", \"@DataDog/ci-app-libraries\"]" @@ -838,6 +843,10 @@ def rspec_skipped_session_run test_spans_by_test_name = test_spans.group_by { |span| span.get_tag("test.name") } expect(test_spans_by_test_name["nested flaky"]).to have(5).items + # check that let values are cleared between retries + let_values = test_spans_by_test_name["nested flaky"].map { |span| span.get_tag("let_value") } + expect(let_values).to eq([1, 2, 3, 4, 5]) + # count how many spans were marked as retries retries_count = test_spans.count { |span| span.get_tag("test.is_retry") == "true" } expect(retries_count).to eq(4)