diff --git a/lib/datadog/ci/contrib/minitest/runner.rb b/lib/datadog/ci/contrib/minitest/runner.rb index 86c45d28..b99beff6 100644 --- a/lib/datadog/ci/contrib/minitest/runner.rb +++ b/lib/datadog/ci/contrib/minitest/runner.rb @@ -28,6 +28,18 @@ def init_plugins(*args) test_visibility_component.start_test_module(Ext::FRAMEWORK) end + def run_one_method(klass, method_name) + return super unless datadog_configuration[:enabled] + + result = nil + + test_retries_component.with_retries do + result = super + end + + result + end + private def datadog_configuration @@ -37,6 +49,10 @@ def datadog_configuration def test_visibility_component Datadog.send(:components).test_visibility end + + def test_retries_component + Datadog.send(:components).test_retries + end end end end diff --git a/lib/datadog/ci/contrib/minitest/test.rb b/lib/datadog/ci/contrib/minitest/test.rb index 27ed2056..58a498e4 100644 --- a/lib/datadog/ci/contrib/minitest/test.rb +++ b/lib/datadog/ci/contrib/minitest/test.rb @@ -71,6 +71,7 @@ def finish_with_result(span, result_code) when "S" span.skipped!(reason: failure.message) end + span.finish end diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 26a2197d..c1753f80 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -40,7 +40,7 @@ def run(*args) # don't report test to RSpec::Core::Reporter until retries are done @skip_reporting = true - test_retries_component.with_retries do |retry_callback| + test_retries_component.with_retries do test_visibility_component.trace_test( test_name, suite_name, @@ -76,8 +76,6 @@ def run(*args) exception: execution_result.pending_exception ) end - - retry_callback.call(test_span) end end diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index 27c74428..f4f85bf2 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -45,11 +45,15 @@ def with_retries(&block) end end + test_visibility_component.set_test_finished_callback(test_finished_callback) + loop do - yield test_finished_callback + yield break unless retry_strategy&.should_retry? end + ensure + test_visibility_component.remove_test_finished_callback end def build_strategy(test_span) @@ -70,6 +74,10 @@ def build_strategy(test_span) def should_retry_failed_test?(test_span) @retry_failed_tests_enabled && !!test_span&.failed? && @retry_failed_tests_count < @retry_failed_tests_total_limit end + + def test_visibility_component + Datadog.send(:components).test_visibility + end end end end diff --git a/lib/datadog/ci/test_visibility/component.rb b/lib/datadog/ci/test_visibility/component.rb index 750d0edc..f30678e8 100644 --- a/lib/datadog/ci/test_visibility/component.rb +++ b/lib/datadog/ci/test_visibility/component.rb @@ -19,6 +19,8 @@ module TestVisibility class Component attr_reader :test_suite_level_visibility_enabled + FIBER_LOCAL_TEST_FINISHED_CALLBACK_KEY = :__dd_test_finished_callback + def initialize( test_suite_level_visibility_enabled: false, codeowners: Codeowners::Parser.new(Git::LocalRepository.root).parse @@ -125,6 +127,15 @@ def deactivate_test_suite(test_suite_name) @context.deactivate_test_suite(test_suite_name) end + # sets fiber-local callback to be called when test is finished + def set_test_finished_callback(callback) + Thread.current[FIBER_LOCAL_TEST_FINISHED_CALLBACK_KEY] = callback + end + + def remove_test_finished_callback + Thread.current[FIBER_LOCAL_TEST_FINISHED_CALLBACK_KEY] = nil + end + def itr_enabled? test_optimisation.enabled? end @@ -192,6 +203,8 @@ def on_test_finished(test) test_optimisation.count_skipped_test(test) Telemetry.event_finished(test) + + Thread.current[FIBER_LOCAL_TEST_FINISHED_CALLBACK_KEY]&.call(test) end # HELPERS diff --git a/lib/datadog/ci/test_visibility/null_component.rb b/lib/datadog/ci/test_visibility/null_component.rb index 59051b83..0ab8bfa4 100644 --- a/lib/datadog/ci/test_visibility/null_component.rb +++ b/lib/datadog/ci/test_visibility/null_component.rb @@ -47,6 +47,12 @@ def itr_enabled? false end + def set_test_finished_callback(_) + end + + def remove_test_finished_callback + end + private def skip_tracing(block = nil) diff --git a/sig/datadog/ci/contrib/minitest/runner.rbs b/sig/datadog/ci/contrib/minitest/runner.rbs index 0af798e0..73cd5f73 100644 --- a/sig/datadog/ci/contrib/minitest/runner.rbs +++ b/sig/datadog/ci/contrib/minitest/runner.rbs @@ -10,11 +10,15 @@ module Datadog def init_plugins: (*untyped) -> (nil | untyped) + def run_one_method: (untyped klass, String method_name) -> untyped + private def datadog_configuration: () -> untyped def test_visibility_component: () -> Datadog::CI::TestVisibility::Component + + def test_retries_component: () -> Datadog::CI::TestRetries::Component end end end diff --git a/sig/datadog/ci/contrib/minitest/test.rbs b/sig/datadog/ci/contrib/minitest/test.rbs index ee391cbc..5d6e3d9e 100644 --- a/sig/datadog/ci/contrib/minitest/test.rbs +++ b/sig/datadog/ci/contrib/minitest/test.rbs @@ -13,6 +13,7 @@ module Datadog module InstanceMethods : ::Minitest::Test include ::Minitest::Test::LifecycleHooks + extend ClassMethods def before_setup: () -> (nil | untyped) diff --git a/sig/datadog/ci/test_retries/component.rbs b/sig/datadog/ci/test_retries/component.rbs index 29760aee..7a76adc6 100644 --- a/sig/datadog/ci/test_retries/component.rbs +++ b/sig/datadog/ci/test_retries/component.rbs @@ -16,13 +16,15 @@ module Datadog def configure: (Datadog::CI::Remote::LibrarySettings library_settings) -> void - def with_retries: () { (untyped) -> void } -> void + def with_retries: () { () -> 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 + + def test_visibility_component: () -> Datadog::CI::TestVisibility::Component end end end diff --git a/sig/datadog/ci/test_visibility/component.rbs b/sig/datadog/ci/test_visibility/component.rbs index 519302e8..9738bd04 100644 --- a/sig/datadog/ci/test_visibility/component.rbs +++ b/sig/datadog/ci/test_visibility/component.rbs @@ -7,6 +7,8 @@ module Datadog @codeowners: Datadog::CI::Codeowners::Matcher @context: Datadog::CI::TestVisibility::Context + FIBER_LOCAL_TEST_FINISHED_CALLBACK_KEY: Symbol + attr_reader test_suite_level_visibility_enabled: bool def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher) -> void @@ -39,6 +41,10 @@ module Datadog def deactivate_test_suite: (String test_suite_name) -> void + def set_test_finished_callback: (Proc callback) -> void + + def remove_test_finished_callback: () -> void + def itr_enabled?: () -> bool def shutdown!: () -> void diff --git a/sig/datadog/ci/test_visibility/null_component.rbs b/sig/datadog/ci/test_visibility/null_component.rbs index bb427d71..920effe4 100644 --- a/sig/datadog/ci/test_visibility/null_component.rbs +++ b/sig/datadog/ci/test_visibility/null_component.rbs @@ -24,6 +24,10 @@ module Datadog def active_span: () -> nil + def set_test_finished_callback: (Proc callback) -> void + + def remove_test_finished_callback: () -> void + def shutdown!: () -> nil def itr_enabled?: () -> bool diff --git a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb index 165f117f..d2d2e4a6 100644 --- a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb @@ -948,4 +948,246 @@ def test_with_background_thread ) end end + + context "with flaky test and test retries enabled" do + include_context "CI mode activated" do + let(:integration_name) { :minitest } + + let(:flaky_test_retries_enabled) { true } + end + + before do + Minitest.run([]) + end + + before(:context) do + Minitest::Runnable.reset + + class FlakyTestSuite < Minitest::Test + @@max_flaky_test_failures = 4 + @@flaky_test_failures = 0 + + def test_passed + assert true + end + + def test_flaky + if @@flaky_test_failures < @@max_flaky_test_failures + @@flaky_test_failures += 1 + assert 1 + 1 == 3 + else + assert 1 + 1 == 2 + end + end + end + end + + it "retries flaky test" do + # 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 + 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["test_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["test_passed"]).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 "with flaky test and test retries enabled with insufficient max retries" do + include_context "CI mode activated" do + let(:integration_name) { :minitest } + + let(:flaky_test_retries_enabled) { true } + let(:retry_failed_tests_max_attempts) { 3 } + end + + before do + Minitest.run([]) + end + + before(:context) do + Minitest::Runnable.reset + + class FlakyTestSuite2 < Minitest::Test + @@max_flaky_test_failures = 4 + @@flaky_test_failures = 0 + + def test_passed + assert true + end + + def test_flaky + if @@flaky_test_failures < @@max_flaky_test_failures + @@flaky_test_failures += 1 + assert 1 + 1 == 3 + else + assert 1 + 1 == 2 + end + end + end + end + + it "retries flaky test without success" do + # 1 initial run of flaky test + 3 retries without success + 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["test_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["test_passed"]).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 "with failed test, flaky test, test retries enabled, and low overall failed tests retry limit" do + include_context "CI mode activated" do + let(:integration_name) { :minitest } + + let(:flaky_test_retries_enabled) { true } + let(:retry_failed_tests_total_limit) { 1 } + end + + before do + Minitest.run([]) + end + + before(:context) do + Minitest::Runnable.reset + + class FailedAndFlakyTestSuite < Minitest::Test + # yep, this test is indeed order dependent! + i_suck_and_my_tests_are_order_dependent! + + @@max_flaky_test_failures = 4 + @@flaky_test_failures = 0 + + def test_failed + assert 1 + 1 == 4 + end + + def test_flaky + if @@flaky_test_failures < @@max_flaky_test_failures + @@flaky_test_failures += 1 + assert 1 + 1 == 3 + else + assert 1 + 1 == 2 + end + end + + def test_passed + assert true + end + end + end + + it "retries flaky test without success" do + # 1 initial run of failed test + 5 retries without success + 1 run of flaky test without retries + 1 passing test = 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") } + expect(test_spans_by_test_name["test_flaky"]).to have(1).item + expect(test_spans_by_test_name["test_failed"]).to have(6).items + expect(test_spans_by_test_name["test_passed"]).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) + + 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 "with flaky test, test retries enabled, and threading test runner" do + include_context "CI mode activated" do + let(:integration_name) { :minitest } + + let(:flaky_test_retries_enabled) { true } + end + + before do + Minitest.run([]) + end + + before(:context) do + Minitest::Runnable.reset + + class ParallelFlakyTestSuite < Minitest::Test + parallelize_me! + + @@max_flaky_test_failures = 4 + @@flaky_test_failures = 0 + + def test_passed + assert true + end + + def test_flaky + if @@flaky_test_failures < @@max_flaky_test_failures + @@flaky_test_failures += 1 + assert 1 + 1 == 3 + else + assert 1 + 1 == 2 + end + end + + def test_failed + assert 1 + 1 == 4 + end + end + end + + it "retries flaky test" do + # 1 initial run of flaky test + 4 retries until pass + 1 failed test run + 5 retries + 1 passing test = 12 spans + expect(test_spans).to have(12).items + + failed_spans, passed_spans = test_spans.partition { |span| span.get_tag("test.status") == "fail" } + expect(failed_spans).to have(10).items + 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["test_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(9) + + expect(test_spans_by_test_name["test_passed"]).to have(1).item + + expect(test_suite_spans).to have(12).items + + expect(test_session_span).to have_fail_status + end + end end diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 0f472772..17b201c0 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -837,7 +837,7 @@ def rspec_skipped_session_run 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(failed_spans).to have(4).items expect(passed_spans).to have(2).items test_spans_by_test_name = test_spans.group_by { |span| span.get_tag("test.name") } diff --git a/spec/datadog/ci/test_retries/component_spec.rb b/spec/datadog/ci/test_retries/component_spec.rb index 933e4e95..5ab2fa86 100644 --- a/spec/datadog/ci/test_retries/component_spec.rb +++ b/spec/datadog/ci/test_retries/component_spec.rb @@ -120,14 +120,30 @@ end describe "#with_retries" do + include_context "CI mode activated" do + let(:flaky_test_retries_enabled) { true } + end + let(:test_failed) { false } - let(:test_span) { instance_double(Datadog::CI::Test, failed?: test_failed, passed?: false, set_tag: true) } + let(:test_span) do + instance_double( + Datadog::CI::Test, + failed?: test_failed, + passed?: false, + set_tag: true, + get_tag: true, + skipped?: false, + type: "test" + ) + end subject(:runs_count) do runs_count = 0 - component.with_retries do |test_finished_callback| + component.with_retries do runs_count += 1 - test_finished_callback.call(test_span) + + # run callback manually + Datadog.send(:components).test_visibility.send(:on_test_finished, test_span) end runs_count diff --git a/spec/datadog/ci/test_visibility/component_spec.rb b/spec/datadog/ci/test_visibility/component_spec.rb index c7379fbd..6a43bd8f 100644 --- a/spec/datadog/ci/test_visibility/component_spec.rb +++ b/spec/datadog/ci/test_visibility/component_spec.rb @@ -797,4 +797,51 @@ end end end + + describe "#set_test_finished_callback" do + include_context "CI mode activated" + + let(:callback) { spy(:callback) } + + it "sets the test finished callback which will be executed when any test is finished" do + test_visibility.set_test_finished_callback(callback) + + ci_test = test_visibility.trace_test("my test", "my suite") + test_visibility.deactivate_test + + expect(callback).to have_received(:call).with(ci_test) + end + + it "only fires callback on the same thread where it was set" do + test_visibility.set_test_finished_callback(callback) + + t = Thread.new do + test_visibility.trace_test("my test", "my suite") + test_visibility.deactivate_test + end + t.join + + expect(callback).to_not have_received(:call) + end + end + + describe "#remove_test_finished_callback" do + include_context "CI mode activated" + + let(:callback) { spy(:callback) } + + it "removes the callback" do + test_visibility.set_test_finished_callback(callback) + + test_visibility.trace_test("my test", "my suite") + test_visibility.deactivate_test + + test_visibility.remove_test_finished_callback + + test_visibility.trace_test("my test", "my suite") + test_visibility.deactivate_test + + expect(callback).to have_received(:call).once + end + end end