From bba2c38c9066f1a984bac6d95f432c68af270010 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 23 Jan 2024 16:20:22 +0100 Subject: [PATCH 01/13] cucumber: better support for pending and undefined scenario: report them as skipped, not passed --- lib/datadog/ci/contrib/cucumber/formatter.rb | 12 ++++--- .../contrib/cucumber/features/passing.feature | 16 ++++++++- .../features/step_definitions/steps.rb | 8 +++++ .../contrib/cucumber/instrumentation_spec.rb | 35 +++++++++++++++++++ vendor/rbs/cucumber/0/cucumber.rbs | 3 ++ 5 files changed, 69 insertions(+), 5 deletions(-) diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index d5cd2dc9..1f759ec9 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -122,12 +122,16 @@ def test_suite_name(test_case) end def finish_test(span, result) - if result.skipped? - span.skipped! - elsif result.ok? - span.passed! + if result.skipped? || result.pending? || result.undefined? + span.skipped!(reason: result.message) elsif result.failed? span.failed!(exception: result.exception) + elsif result.ok? + span.passed! + else + Datadog.logger.warn do + "Unknown test result #{result.class.name} for test #{span.name}" + end end span.finish end diff --git a/spec/datadog/ci/contrib/cucumber/features/passing.feature b/spec/datadog/ci/contrib/cucumber/features/passing.feature index 38be690a..67814aa1 100644 --- a/spec/datadog/ci/contrib/cucumber/features/passing.feature +++ b/spec/datadog/ci/contrib/cucumber/features/passing.feature @@ -3,4 +3,18 @@ Feature: Datadog integration Scenario: cucumber scenario Given datadog And datadog - Then datadog \ No newline at end of file + Then datadog + + Scenario: undefined scenario + Given datadog + And undefined + Then undefined + + Scenario: pending scenario + Given datadog + Then pending + + Scenario: skipped scenario + Given datadog + And skip + Then datadog diff --git a/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps.rb b/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps.rb index 9aec38f9..095fb0ca 100644 --- a/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps.rb +++ b/spec/datadog/ci/contrib/cucumber/features/step_definitions/steps.rb @@ -6,6 +6,14 @@ 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 diff --git a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb index a0050612..7e846efe 100644 --- a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb @@ -70,6 +70,8 @@ let(:feature_file_to_run) { "passing.feature" } it "creates spans for each scenario and step" do + expect(test_spans).to have(4).items + scenario_span = spans.find { |s| s.resource == "cucumber scenario" } expect(scenario_span.type).to eq(Datadog::CI::Ext::AppTypes::TYPE_TEST) @@ -108,6 +110,39 @@ end end + it "marks undefined cucumber scenario as skipped" do + undefined_scenario_span = spans.find { |s| s.resource == "undefined scenario" } + expect(undefined_scenario_span).not_to be_nil + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::SKIP + ) + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( + 'Undefined step: "undefined"' + ) + end + + it "marks pending cucumber scenario as skipped" do + undefined_scenario_span = spans.find { |s| s.resource == "pending scenario" } + expect(undefined_scenario_span).not_to be_nil + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::SKIP + ) + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( + "implementation" + ) + end + + it "marks skipped cucumber scenario as skipped" do + undefined_scenario_span = spans.find { |s| s.resource == "skipped scenario" } + expect(undefined_scenario_span).not_to be_nil + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::SKIP + ) + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( + "Scenario skipped" + ) + end + it "creates test session span" do expect(test_session_span).not_to be_nil expect(test_session_span.service).to eq("jalapenos") diff --git a/vendor/rbs/cucumber/0/cucumber.rbs b/vendor/rbs/cucumber/0/cucumber.rbs index 6e643260..e6cf0f73 100644 --- a/vendor/rbs/cucumber/0/cucumber.rbs +++ b/vendor/rbs/cucumber/0/cucumber.rbs @@ -18,6 +18,9 @@ class Cucumber::Core::Test::Result def failed?: () -> bool def ok?: () -> bool def skipped?: () -> bool + def pending?: () -> bool + def undefined?: () -> bool + def message: () -> String def exception: () -> untyped end From ecea3cf7a7fc8cf0000ff5ea6900ca3ce00b6fc8 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 23 Jan 2024 16:45:42 +0100 Subject: [PATCH 02/13] better handling of cucumber scenario status that takes into account strict mode --- lib/datadog/ci/contrib/cucumber/formatter.rb | 10 +++------- vendor/rbs/cucumber/0/cucumber.rbs | 3 ++- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 1f759ec9..52680a24 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -122,16 +122,12 @@ def test_suite_name(test_case) end def finish_test(span, result) - if result.skipped? || result.pending? || result.undefined? + if !result.passed? && result.ok?(@config.strict) span.skipped!(reason: result.message) - elsif result.failed? - span.failed!(exception: result.exception) - elsif result.ok? + elsif result.passed? span.passed! else - Datadog.logger.warn do - "Unknown test result #{result.class.name} for test #{span.name}" - end + span.failed!(exception: result.exception) end span.finish end diff --git a/vendor/rbs/cucumber/0/cucumber.rbs b/vendor/rbs/cucumber/0/cucumber.rbs index e6cf0f73..891f2d16 100644 --- a/vendor/rbs/cucumber/0/cucumber.rbs +++ b/vendor/rbs/cucumber/0/cucumber.rbs @@ -16,8 +16,9 @@ end class Cucumber::Core::Test::Result def failed?: () -> bool - def ok?: () -> bool + def ok?: (?untyped strict) -> bool def skipped?: () -> bool + def passed?: () -> bool def pending?: () -> bool def undefined?: () -> bool def message: () -> String From 3ef881fdcc90f5cef8436bbfba1f792fe4427d24 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 23 Jan 2024 16:56:50 +0100 Subject: [PATCH 03/13] test cucumber strict mode and fix a bug with session status --- lib/datadog/ci/contrib/cucumber/formatter.rb | 3 +- .../contrib/cucumber/instrumentation_spec.rb | 59 ++++++++++++++++--- 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 52680a24..e367f0d4 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -84,8 +84,6 @@ def on_test_case_finished(event) # TestRunFinished event does not have a success attribute before 8.0. # if event.result.failed? - @failed_tests_count += 1 - test_suite = @current_test_suite test_suite.failed! if test_suite end @@ -128,6 +126,7 @@ def finish_test(span, result) span.passed! else span.failed!(exception: result.exception) + @failed_tests_count += 1 end span.finish end diff --git a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb index 7e846efe..604699ed 100644 --- a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb @@ -122,23 +122,23 @@ end it "marks pending cucumber scenario as skipped" do - undefined_scenario_span = spans.find { |s| s.resource == "pending scenario" } - expect(undefined_scenario_span).not_to be_nil - expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + pending_scenario_span = spans.find { |s| s.resource == "pending scenario" } + expect(pending_scenario_span).not_to be_nil + expect(pending_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( Datadog::CI::Ext::Test::Status::SKIP ) - expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( + expect(pending_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( "implementation" ) end it "marks skipped cucumber scenario as skipped" do - undefined_scenario_span = spans.find { |s| s.resource == "skipped scenario" } - expect(undefined_scenario_span).not_to be_nil - expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + skipped_scenario_span = spans.find { |s| s.resource == "skipped scenario" } + expect(skipped_scenario_span).not_to be_nil + expect(skipped_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( Datadog::CI::Ext::Test::Status::SKIP ) - expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( + expect(skipped_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq( "Scenario skipped" ) end @@ -299,4 +299,47 @@ ) end end + + context "executing a feature with undefined steps in strict mode" do + let(:expected_test_run_code) { 2 } + let(:feature_file_to_run) { "passing.feature" } + let(:args) do + [ + "--strict", + "-r", + steps_file_for_run_path, + features_path + ] + end + + it "marks test session as failed" do + expect(test_session_span).not_to be_nil + expect(test_session_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::FAIL) + end + + it "marks undefined cucumber scenario as failed" do + undefined_scenario_span = spans.find { |s| s.resource == "undefined scenario" } + expect(undefined_scenario_span).not_to be_nil + expect(undefined_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::FAIL + ) + expect(undefined_scenario_span).to have_error_message("Undefined step: \"undefined\"") + end + + it "marks pending cucumber scenario as failed" do + pending_scenario_span = spans.find { |s| s.resource == "pending scenario" } + expect(pending_scenario_span).not_to be_nil + expect(pending_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::FAIL + ) + end + + it "marks skipped cucumber scenario as skipped" do + skipped_scenario_span = spans.find { |s| s.resource == "skipped scenario" } + expect(skipped_scenario_span).not_to be_nil + expect(skipped_scenario_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( + Datadog::CI::Ext::Test::Status::SKIP + ) + end + end end From f569eda658fb0b320be647d9848702636f2ee248 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Tue, 23 Jan 2024 17:00:09 +0100 Subject: [PATCH 04/13] fix a bug with test suite status in strict mode --- lib/datadog/ci/contrib/cucumber/formatter.rb | 11 +++-------- .../ci/contrib/cucumber/instrumentation_spec.rb | 5 +++++ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index e367f0d4..0618ce97 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -80,14 +80,6 @@ def on_test_case_finished(event) test_span = CI.active_test return if test_span.nil? - # We need to track overall test failures manually if we are using cucumber < 8.0 because - # TestRunFinished event does not have a success attribute before 8.0. - # - if event.result.failed? - test_suite = @current_test_suite - test_suite.failed! if test_suite - end - finish_test(test_span, event.result) end @@ -127,6 +119,9 @@ def finish_test(span, result) else span.failed!(exception: result.exception) @failed_tests_count += 1 + + test_suite = @current_test_suite + test_suite.failed! if test_suite end span.finish end diff --git a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb index 604699ed..96271dda 100644 --- a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb @@ -317,6 +317,11 @@ expect(test_session_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::FAIL) end + it "marks test suite as failed" do + expect(test_suite_span).not_to be_nil + expect(test_suite_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::FAIL) + end + it "marks undefined cucumber scenario as failed" do undefined_scenario_span = spans.find { |s| s.resource == "undefined scenario" } expect(undefined_scenario_span).not_to be_nil From 97d76424e9a9d6573bb76f027b1dad757e1fc271 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 14:14:24 +0100 Subject: [PATCH 05/13] cucumber test suite where all tests are skipped now marked as skipped --- lib/datadog/ci/contrib/cucumber/formatter.rb | 42 ++++++++++++------- sig/datadog/ci/contrib/cucumber/formatter.rbs | 10 +++-- .../contrib/cucumber/features/skipped.feature | 11 +++++ .../contrib/cucumber/instrumentation_spec.rb | 27 ++++++++++-- 4 files changed, 70 insertions(+), 20 deletions(-) create mode 100644 spec/datadog/ci/contrib/cucumber/features/skipped.feature diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 0618ce97..2544f506 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -15,12 +15,13 @@ class Formatter def initialize(config) @ast_lookup = ::Cucumber::Formatter::AstLookup.new(config) if defined?(::Cucumber::Formatter::AstLookup) - @config = config - @failed_tests_count = 0 @current_test_suite = nil + @failed_tests_count = 0 + @test_suite_stats = Hash.new(0) + bind_events(config) end @@ -80,7 +81,9 @@ def on_test_case_finished(event) test_span = CI.active_test return if test_span.nil? - finish_test(test_span, event.result) + datadog_status = finish_span(test_span, event.result) + @test_suite_stats[datadog_status] += 1 + @failed_tests_count += 1 if datadog_status == :failed end def on_test_step_started(event) @@ -91,7 +94,7 @@ def on_test_step_finished(event) current_step_span = CI.active_span return if current_step_span.nil? - finish_test(current_step_span, event.result) + finish_span(current_step_span, event.result) end private @@ -111,19 +114,23 @@ def test_suite_name(test_case) end end - def finish_test(span, result) - if !result.passed? && result.ok?(@config.strict) + def finish_span(span, result) + datadog_status = if !result.passed? && result.ok?(@config.strict) span.skipped!(reason: result.message) + + :skipped elsif result.passed? span.passed! + + :passed else span.failed!(exception: result.exception) - @failed_tests_count += 1 - test_suite = @current_test_suite - test_suite.failed! if test_suite + :failed end span.finish + + datadog_status end def finish_session(result) @@ -149,18 +156,25 @@ def finish_session(result) def start_test_suite(test_suite_name) finish_current_test_suite - test_suite = CI.start_test_suite(test_suite_name) - # will be overridden if any test fails - test_suite.passed! if test_suite - - @current_test_suite = test_suite + @current_test_suite = CI.start_test_suite(test_suite_name) end def finish_current_test_suite test_suite = @current_test_suite return unless test_suite + if @test_suite_stats[:failed] > 0 + test_suite.failed! + elsif @test_suite_stats[:passed] > 0 + test_suite.passed! + else + test_suite.skipped! + end + test_suite.finish + + @test_suite_stats = Hash.new(0) + @current_test_suite = nil end def same_test_suite_as_current?(test_suite_name) diff --git a/sig/datadog/ci/contrib/cucumber/formatter.rbs b/sig/datadog/ci/contrib/cucumber/formatter.rbs index 523246ae..7d0b8fb9 100644 --- a/sig/datadog/ci/contrib/cucumber/formatter.rbs +++ b/sig/datadog/ci/contrib/cucumber/formatter.rbs @@ -4,9 +4,13 @@ module Datadog module Cucumber class Formatter private - @failed_tests_count: Integer - @current_test_suite: Datadog::CI::Span? @ast_lookup: ::Cucumber::Formatter::AstLookup + @config: untyped + + @current_test_suite: Datadog::CI::TestSuite? + + @failed_tests_count: Integer + @test_suite_stats: Hash[Symbol, Integer] attr_reader config: untyped @@ -40,7 +44,7 @@ module Datadog def finish_session: (bool result) -> void - def finish_test: (Datadog::CI::Span test, Cucumber::Core::Test::Result result) -> void + def finish_span: (Datadog::CI::Span span, Cucumber::Core::Test::Result result) -> Symbol def extract_parameters_hash: (untyped test_case) -> Hash[String, String]? diff --git a/spec/datadog/ci/contrib/cucumber/features/skipped.feature b/spec/datadog/ci/contrib/cucumber/features/skipped.feature new file mode 100644 index 00000000..98c3f9c5 --- /dev/null +++ b/spec/datadog/ci/contrib/cucumber/features/skipped.feature @@ -0,0 +1,11 @@ +Feature: All scenarios are skipped + + Scenario: undefined scenario + Given datadog + And undefined + Then undefined + + Scenario: skipped scenario + Given datadog + And skip + Then datadog diff --git a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb index 96271dda..c97dfe38 100644 --- a/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb @@ -235,8 +235,8 @@ let(:feature_file_to_run) { "with_parameters.feature" } it "a single test suite, and a test span for each example with parameters JSON" do - expect(test_spans.count).to eq(3) - expect(test_suite_spans.count).to eq(1) + expect(test_spans).to have(3).items + expect(test_suite_spans).to have(1).item test_spans.each_with_index do |span, index| # test parameters are available since cucumber 4 @@ -269,7 +269,7 @@ let(:failing_test_suite) { test_suite_spans.find { |span| span.name =~ /failing/ } } it "creates a test suite span for each feature" do - expect(test_suite_spans.count).to eq(3) + expect(test_suite_spans).to have(4).items expect(passing_test_suite.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq( Datadog::CI::Ext::Test::Status::PASS ) @@ -347,4 +347,25 @@ ) end end + + context "executing a feature where all scenarios are skipped" do + let(:feature_file_to_run) { "skipped.feature" } + + it "marks all test spans as skipped" do + expect(test_spans).to have(2).items + expect(test_spans.map { |span| span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS) }.uniq).to eq( + [Datadog::CI::Ext::Test::Status::SKIP] + ) + end + + it "marks test session as passed" do + expect(test_session_span).not_to be_nil + expect(test_session_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::PASS) + end + + it "marks test suite as skipped" do + expect(test_suite_span).not_to be_nil + expect(test_suite_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::SKIP) + end + end end From e7a8356ab65356f13300ad96601e757660a749db Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 14:42:38 +0100 Subject: [PATCH 06/13] rspec: simplify pending specs logic and always send message and exception --- lib/datadog/ci/contrib/rspec/example.rb | 12 +++++------- .../datadog/ci/contrib/rspec/instrumentation_spec.rb | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index f495a724..48dd8a38 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -60,13 +60,11 @@ def run(*) test_suite_span.failed! if test_suite_span else # :pending or nil - if execution_result.pending_message - test_span.skipped!(reason: execution_result.pending_message) - elsif execution_result.example_skipped? - test_span.skipped!(exception: execution_result.exception) - else - test_span.skipped!(exception: execution_result.pending_exception) - end + test_span.skipped!( + reason: execution_result.pending_message, + exception: execution_result.pending_exception + ) + test_suite_span.skipped! if test_suite_span end end diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index 211bae93..febe942e 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -343,7 +343,7 @@ def expect_failure expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::SKIP) expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_NAME)).to eq("foo") expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq("did not fix the math yet") - expect(first_test_span).not_to have_error + expect(first_test_span).to have_error end it "with pending keyword and passing" do @@ -374,7 +374,7 @@ def expect_failure expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::SKIP) expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_NAME)).to eq("foo") expect(first_test_span.get_tag(Datadog::CI::Ext::Test::TAG_SKIP_REASON)).to eq("did not fix the math yet") - expect(first_test_span).not_to have_error + expect(first_test_span).to have_error end end From a386287532d36cba8a3e5f286f648b10db3838b2 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 16:03:12 +0100 Subject: [PATCH 07/13] rspec: count examples and pending examples before and after test suite to see if there are some that weren't skipped --- lib/datadog/ci/contrib/rspec/example_group.rb | 58 +++++++++++++++++-- .../ci/contrib/rspec/example_group.rbs | 21 +++++++ .../ci/contrib/rspec/instrumentation_spec.rb | 48 +++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index 4da06e3a..54e45305 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -7,6 +7,45 @@ module Datadog module CI module Contrib module RSpec + class ExampleResultsCounter + def initialize(reporter) + @reporter = reporter + @examples_start_count = current_examples_count + @pending_examples_start_count = current_pending_examples_count + end + + def all_pending_since_start? + return false unless reporter_counts_examples? + + examples_since_start_count = current_examples_count - @examples_start_count + pending_examples_since_start_count = current_pending_examples_count - @pending_examples_start_count + + examples_since_start_count == pending_examples_since_start_count + end + + private + + attr_reader :reporter + + def reporter_counts_examples? + return @reporter_counts_examples if defined?(@reporter_counts_examples) + + @reporter_counts_examples = reporter.respond_to?(:examples) && !reporter.examples.nil? && + reporter.examples.is_a?(Array) && reporter.respond_to?(:pending_examples) && + !reporter.pending_examples.nil? && reporter.pending_examples.is_a?(Array) + end + + def current_examples_count + return 0 unless reporter_counts_examples? + reporter.examples.count + end + + def current_pending_examples_count + return 0 unless reporter_counts_examples? + reporter.pending_examples.count + end + end + # Instrument RSpec::Core::ExampleGroup module ExampleGroup def self.included(base) @@ -15,24 +54,31 @@ def self.included(base) # Instance methods for configuration module ClassMethods - def run(*) + def run(reporter = ::RSpec::Core::NullReporter) return super unless datadog_configuration[:enabled] return super unless top_level? suite_name = "#{description} at #{file_path}" test_suite = Datadog::CI.start_test_suite(suite_name) - result = super - return result unless test_suite + counter = ExampleResultsCounter.new(reporter) - if result - test_suite.passed! + success = super + return success unless test_suite + + if success + if counter.all_pending_since_start? + test_suite.skipped! + else + test_suite.passed! + end else test_suite.failed! end + test_suite.finish - result + success end private diff --git a/sig/datadog/ci/contrib/rspec/example_group.rbs b/sig/datadog/ci/contrib/rspec/example_group.rbs index dec0f3f2..bda04fd5 100644 --- a/sig/datadog/ci/contrib/rspec/example_group.rbs +++ b/sig/datadog/ci/contrib/rspec/example_group.rbs @@ -2,6 +2,27 @@ module Datadog module CI module Contrib module RSpec + class ExampleResultsCounter + @reporter: untyped + @examples_start_count: Integer + @pending_examples_start_count: Integer + @reporter_counts_examples: bool + + + def initialize: (untyped reporter) -> void + + def all_pending_since_start?: () -> bool + + private + attr_reader reporter: untyped + + def reporter_counts_examples?: () -> bool + + def current_examples_count: () -> Integer + + def current_pending_examples_count: () -> Integer + end + module ExampleGroup def self.included: (untyped base) -> untyped diff --git a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb index febe942e..43748763 100644 --- a/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/rspec/instrumentation_spec.rb @@ -553,5 +553,53 @@ def rspec_session_run(with_failed_test: false, with_shared_test: false) end end end + + context "with skipped test suite" do + def rspec_skipped_session_run + with_new_rspec_environment do + RSpec.describe "SomeTest" do + it "foo" do + # DO NOTHING + end + end + + spec = RSpec.describe "SkippedTest" do + context "nested" do + it "skipped foo", skip: true do + # DO NOTHING + end + + it "pending fails" do + pending("did not fix the math yet") + expect(1).to eq(2) + end + end + end + + options = ::RSpec::Core::ConfigurationOptions.new(%w[--pattern none]) + ::RSpec::Core::Runner.new(options).run(devnull, devnull) + + spec + end + end + + before do + rspec_skipped_session_run + end + + it "marks test session as passed" do + expect(test_session_span).not_to be_nil + expect(test_session_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::PASS) + end + + it "marks test suite as skipped" do + skipped_suite = test_suite_spans.find do |suite_span| + suite_span.get_tag(Datadog::CI::Ext::Test::TAG_SUITE).include?("SkippedTest") + end + + expect(skipped_suite).not_to be_nil + expect(skipped_suite.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::SKIP) + end + end end end From 085e06963930cc49c6d3e120f3fa15101cf5d4f7 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 16:50:41 +0100 Subject: [PATCH 08/13] Datadog::CI::TestSuite now automatically collects stats about tests and is able to derive status based on these stats. Minitest instrumentation correctly marks test suites as skipped. --- lib/datadog/ci/concurrent_span.rb | 3 +- lib/datadog/ci/contrib/minitest/hooks.rb | 22 ++------ lib/datadog/ci/contrib/minitest/runnable.rb | 1 - lib/datadog/ci/contrib/rspec/example_group.rb | 2 + lib/datadog/ci/test.rb | 34 ++++++++++++ lib/datadog/ci/test_suite.rb | 55 ++++++++++++++++++- sig/datadog/ci/concurrent_span.rbs | 2 +- sig/datadog/ci/contrib/minitest/hooks.rbs | 6 +- sig/datadog/ci/test.rbs | 4 ++ sig/datadog/ci/test_suite.rbs | 11 ++++ .../contrib/minitest/instrumentation_spec.rb | 33 +++++++++++ spec/datadog/ci/test_suite_spec.rb | 6 ++ 12 files changed, 151 insertions(+), 28 deletions(-) diff --git a/lib/datadog/ci/concurrent_span.rb b/lib/datadog/ci/concurrent_span.rb index 994e5038..641adc20 100644 --- a/lib/datadog/ci/concurrent_span.rb +++ b/lib/datadog/ci/concurrent_span.rb @@ -12,7 +12,8 @@ class ConcurrentSpan < Span def initialize(tracer_span) super - @mutex = Mutex.new + # we use Monitor instead of Mutex because it is reentrant + @mutex = Monitor.new end # Gets tag value by key. This method is thread-safe. diff --git a/lib/datadog/ci/contrib/minitest/hooks.rb b/lib/datadog/ci/contrib/minitest/hooks.rb index e68b657d..293efe6f 100644 --- a/lib/datadog/ci/contrib/minitest/hooks.rb +++ b/lib/datadog/ci/contrib/minitest/hooks.rb @@ -41,9 +41,9 @@ def after_teardown test_span = CI.active_test return super unless test_span - finish_test(test_span, result_code) + finish_with_result(test_span, result_code) if Helpers.parallel?(self.class) - finish_test_suite(test_span.test_suite, result_code) + finish_with_result(test_span.test_suite, result_code) end super @@ -51,23 +51,9 @@ def after_teardown private - def finish_test(test_span, result_code) - finish_with_result(test_span, result_code) - - # mark test suite as failed if any test failed - if test_span.failed? - test_suite = test_span.test_suite - test_suite.failed! if test_suite - end - end - - def finish_test_suite(test_suite, result_code) - return unless test_suite - - finish_with_result(test_suite, result_code) - end - def finish_with_result(span, result_code) + return unless span + case result_code when "." span.passed! diff --git a/lib/datadog/ci/contrib/minitest/runnable.rb b/lib/datadog/ci/contrib/minitest/runnable.rb index e3aa2272..581a22fa 100644 --- a/lib/datadog/ci/contrib/minitest/runnable.rb +++ b/lib/datadog/ci/contrib/minitest/runnable.rb @@ -20,7 +20,6 @@ def run(*) test_suite_name = Helpers.test_suite_name(self, method) test_suite = Datadog::CI.start_test_suite(test_suite_name) - test_suite.passed! if test_suite # will be overridden if any test fails results = super return results unless test_suite diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index 54e45305..ad70a99a 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -37,11 +37,13 @@ def reporter_counts_examples? def current_examples_count return 0 unless reporter_counts_examples? + reporter.examples.count end def current_pending_examples_count return 0 unless reporter_counts_examples? + reporter.pending_examples.count end end diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index dbde3b31..c7e896f9 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -62,6 +62,33 @@ def source_file get_tag(Ext::Test::TAG_SOURCE_FILE) end + # Sets the status of the span to "pass". + # @return [void] + def passed! + super + + record_test_result(Ext::Test::Status::PASS) + end + + # Sets the status of the span to "fail". + # @param [Exception] exception the exception that caused the test to fail. + # @return [void] + def failed!(exception: nil) + super + + record_test_result(Ext::Test::Status::FAIL) + end + + # Sets the status of the span to "skip". + # @param [Exception] exception the exception that caused the test to fail. + # @param [String] reason the reason why the test was skipped. + # @return [void] + def skipped!(exception: nil, reason: nil) + super + + record_test_result(Ext::Test::Status::SKIP) + end + # Sets the parameters for this test (e.g. Cucumber example or RSpec shared specs). # Parameters are needed to compute test fingerprint to distinguish between different tests having same names. # @@ -81,6 +108,13 @@ def set_parameters(arguments, metadata = {}) ) ) end + + private + + def record_test_result(datadog_status) + suite = test_suite + suite.record_test_result(datadog_status) if suite + end end end end diff --git a/lib/datadog/ci/test_suite.rb b/lib/datadog/ci/test_suite.rb index 3a7f7213..97b58197 100644 --- a/lib/datadog/ci/test_suite.rb +++ b/lib/datadog/ci/test_suite.rb @@ -13,12 +13,63 @@ module CI # # @public_api class TestSuite < ConcurrentSpan + def initialize(tracer_span) + super + + @test_suite_stats = Hash.new(0) + end + # Finishes this test suite. # @return [void] def finish - super + synchronize do + # we need to derive test suite status from execution stats if no status was set explicitly + set_status_from_stats! if undefined? + + super + + recorder.deactivate_test_suite(name) + end + end + + # @internal + def record_test_result(datadog_test_status) + synchronize do + @test_suite_stats[datadog_test_status] += 1 + end + end + + # @internal + def passed_tests_count + synchronize do + @test_suite_stats[Ext::Test::Status::PASS] + end + end + + # @internal + def skipped_tests_count + synchronize do + @test_suite_stats[Ext::Test::Status::SKIP] + end + end + + # @internal + def failed_tests_count + synchronize do + @test_suite_stats[Ext::Test::Status::FAIL] + end + end + + private - recorder.deactivate_test_suite(name) + def set_status_from_stats! + if failed_tests_count > 0 + failed! + elsif passed_tests_count == 0 + skipped! + else + passed! + end end end end diff --git a/sig/datadog/ci/concurrent_span.rbs b/sig/datadog/ci/concurrent_span.rbs index ff834140..d7712436 100644 --- a/sig/datadog/ci/concurrent_span.rbs +++ b/sig/datadog/ci/concurrent_span.rbs @@ -1,7 +1,7 @@ module Datadog module CI class ConcurrentSpan < Span - @mutex: Thread::Mutex + @mutex: Monitor def initialize: (Datadog::Tracing::SpanOperation tracer_span) -> void def passed!: () -> void diff --git a/sig/datadog/ci/contrib/minitest/hooks.rbs b/sig/datadog/ci/contrib/minitest/hooks.rbs index 4cad85e4..225f7d12 100644 --- a/sig/datadog/ci/contrib/minitest/hooks.rbs +++ b/sig/datadog/ci/contrib/minitest/hooks.rbs @@ -13,11 +13,7 @@ module Datadog def datadog_configuration: () -> untyped - def finish_test: (Datadog::CI::Test test_span, String result_code) -> void - - def finish_test_suite: (Datadog::CI::TestSuite? test_suite, String result_code) -> void - - def finish_with_result: (Datadog::CI::Span span, String result_code) -> void + def finish_with_result: (Datadog::CI::Span? span, String result_code) -> void def self.test_order: () -> (nil | :parallel | :sorted | :random | :alpha) end diff --git a/sig/datadog/ci/test.rbs b/sig/datadog/ci/test.rbs index 41b086d2..ab77f7d0 100644 --- a/sig/datadog/ci/test.rbs +++ b/sig/datadog/ci/test.rbs @@ -8,6 +8,10 @@ module Datadog def test_module_id: () -> String? def test_session_id: () -> String? def source_file: () -> String? + + private + + def record_test_result: (String datadog_status) -> void end end end diff --git a/sig/datadog/ci/test_suite.rbs b/sig/datadog/ci/test_suite.rbs index 361b599c..b3d2bf8e 100644 --- a/sig/datadog/ci/test_suite.rbs +++ b/sig/datadog/ci/test_suite.rbs @@ -1,6 +1,17 @@ module Datadog module CI class TestSuite < ConcurrentSpan + @test_suite_stats: Hash[String, Integer] + + def record_test_result: (String datadog_test_status) -> void + + def passed_tests_count: () -> Integer + def skipped_tests_count: () -> Integer + def failed_tests_count: () -> Integer + + private + + def set_status_from_stats!: () -> void end end end diff --git a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb index 712cd7c2..69c14b70 100644 --- a/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb +++ b/spec/datadog/ci/contrib/minitest/instrumentation_spec.rb @@ -675,6 +675,39 @@ def test_b_2 ) end end + + context "skipped suite" do + before(:context) do + Minitest::Runnable.reset + + class SkippedTest < Minitest::Test + def test_1 + skip + end + + def test_2 + skip + end + end + end + + it "marks all test spans as skipped" do + expect(test_spans).to have(2).items + expect(test_spans.map { |span| span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS) }.uniq).to eq( + [Datadog::CI::Ext::Test::Status::SKIP] + ) + end + + it "marks test session as passed" do + expect(test_session_span).not_to be_nil + expect(test_session_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::PASS) + end + + it "marks test suite as skipped" do + expect(test_suite_span).not_to be_nil + expect(test_suite_span.get_tag(Datadog::CI::Ext::Test::TAG_STATUS)).to eq(Datadog::CI::Ext::Test::Status::SKIP) + end + end end end end diff --git a/spec/datadog/ci/test_suite_spec.rb b/spec/datadog/ci/test_suite_spec.rb index 46d854e5..ea80fac5 100644 --- a/spec/datadog/ci/test_suite_spec.rb +++ b/spec/datadog/ci/test_suite_spec.rb @@ -10,6 +10,12 @@ describe "#finish" do subject(:ci_test_suite) { described_class.new(tracer_span) } + before do + expect(tracer_span).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_STATUS).and_return( + Datadog::CI::Ext::Test::Status::PASS + ) + end + it "deactivates the test suite" do ci_test_suite.finish From 2380ee3357b5ad69e53ed9860ecc4c6e396ddc74 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 16:51:43 +0100 Subject: [PATCH 09/13] rspec - custom code is no longer needed to track executed examples --- lib/datadog/ci/contrib/rspec/example_group.rb | 53 ------------------- .../ci/contrib/rspec/example_group.rbs | 21 -------- 2 files changed, 74 deletions(-) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index ad70a99a..e54b108f 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -7,47 +7,6 @@ module Datadog module CI module Contrib module RSpec - class ExampleResultsCounter - def initialize(reporter) - @reporter = reporter - @examples_start_count = current_examples_count - @pending_examples_start_count = current_pending_examples_count - end - - def all_pending_since_start? - return false unless reporter_counts_examples? - - examples_since_start_count = current_examples_count - @examples_start_count - pending_examples_since_start_count = current_pending_examples_count - @pending_examples_start_count - - examples_since_start_count == pending_examples_since_start_count - end - - private - - attr_reader :reporter - - def reporter_counts_examples? - return @reporter_counts_examples if defined?(@reporter_counts_examples) - - @reporter_counts_examples = reporter.respond_to?(:examples) && !reporter.examples.nil? && - reporter.examples.is_a?(Array) && reporter.respond_to?(:pending_examples) && - !reporter.pending_examples.nil? && reporter.pending_examples.is_a?(Array) - end - - def current_examples_count - return 0 unless reporter_counts_examples? - - reporter.examples.count - end - - def current_pending_examples_count - return 0 unless reporter_counts_examples? - - reporter.pending_examples.count - end - end - # Instrument RSpec::Core::ExampleGroup module ExampleGroup def self.included(base) @@ -63,21 +22,9 @@ def run(reporter = ::RSpec::Core::NullReporter) suite_name = "#{description} at #{file_path}" test_suite = Datadog::CI.start_test_suite(suite_name) - counter = ExampleResultsCounter.new(reporter) - success = super return success unless test_suite - if success - if counter.all_pending_since_start? - test_suite.skipped! - else - test_suite.passed! - end - else - test_suite.failed! - end - test_suite.finish success diff --git a/sig/datadog/ci/contrib/rspec/example_group.rbs b/sig/datadog/ci/contrib/rspec/example_group.rbs index bda04fd5..dec0f3f2 100644 --- a/sig/datadog/ci/contrib/rspec/example_group.rbs +++ b/sig/datadog/ci/contrib/rspec/example_group.rbs @@ -2,27 +2,6 @@ module Datadog module CI module Contrib module RSpec - class ExampleResultsCounter - @reporter: untyped - @examples_start_count: Integer - @pending_examples_start_count: Integer - @reporter_counts_examples: bool - - - def initialize: (untyped reporter) -> void - - def all_pending_since_start?: () -> bool - - private - attr_reader reporter: untyped - - def reporter_counts_examples?: () -> bool - - def current_examples_count: () -> Integer - - def current_pending_examples_count: () -> Integer - end - module ExampleGroup def self.included: (untyped base) -> untyped From 93f966661a695fea40f054fa243c625c60571431 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 16:55:19 +0100 Subject: [PATCH 10/13] simplify cucumber test suite status logic --- lib/datadog/ci/contrib/cucumber/formatter.rb | 25 +++---------------- sig/datadog/ci/contrib/cucumber/formatter.rbs | 3 +-- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 2544f506..1ab2e4e1 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -20,7 +20,6 @@ def initialize(config) @current_test_suite = nil @failed_tests_count = 0 - @test_suite_stats = Hash.new(0) bind_events(config) end @@ -81,9 +80,8 @@ def on_test_case_finished(event) test_span = CI.active_test return if test_span.nil? - datadog_status = finish_span(test_span, event.result) - @test_suite_stats[datadog_status] += 1 - @failed_tests_count += 1 if datadog_status == :failed + finish_span(test_span, event.result) + @failed_tests_count += 1 if test_span.failed? end def on_test_step_started(event) @@ -115,22 +113,14 @@ def test_suite_name(test_case) end def finish_span(span, result) - datadog_status = if !result.passed? && result.ok?(@config.strict) + if !result.passed? && result.ok?(@config.strict) span.skipped!(reason: result.message) - - :skipped elsif result.passed? span.passed! - - :passed else span.failed!(exception: result.exception) - - :failed end span.finish - - datadog_status end def finish_session(result) @@ -163,17 +153,8 @@ def finish_current_test_suite test_suite = @current_test_suite return unless test_suite - if @test_suite_stats[:failed] > 0 - test_suite.failed! - elsif @test_suite_stats[:passed] > 0 - test_suite.passed! - else - test_suite.skipped! - end - test_suite.finish - @test_suite_stats = Hash.new(0) @current_test_suite = nil end diff --git a/sig/datadog/ci/contrib/cucumber/formatter.rbs b/sig/datadog/ci/contrib/cucumber/formatter.rbs index 7d0b8fb9..5dd1f869 100644 --- a/sig/datadog/ci/contrib/cucumber/formatter.rbs +++ b/sig/datadog/ci/contrib/cucumber/formatter.rbs @@ -10,7 +10,6 @@ module Datadog @current_test_suite: Datadog::CI::TestSuite? @failed_tests_count: Integer - @test_suite_stats: Hash[Symbol, Integer] attr_reader config: untyped @@ -44,7 +43,7 @@ module Datadog def finish_session: (bool result) -> void - def finish_span: (Datadog::CI::Span span, Cucumber::Core::Test::Result result) -> Symbol + def finish_span: (Datadog::CI::Span span, Cucumber::Core::Test::Result result) -> void def extract_parameters_hash: (untyped test_case) -> Hash[String, String]? From b21b74d4a5b364af4d1e3cbcc238a59de8ecfda9 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 18:29:03 +0100 Subject: [PATCH 11/13] rspec: take into account what framework reported about example group success when setting test suite status --- lib/datadog/ci/contrib/rspec/example_group.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index e54b108f..b13f2173 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -25,6 +25,14 @@ def run(reporter = ::RSpec::Core::NullReporter) success = super return success unless test_suite + if success && test_suite.passed_tests_count > 0 + test_suite.passed! + elsif success + test_suite.skipped! + else + test_suite.failed! + end + test_suite.finish success From 4153dd8085a4c9e41a247a59019c9d30c228c36d Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 18:35:24 +0100 Subject: [PATCH 12/13] spec for test suite statistics --- lib/datadog/ci/test_suite.rb | 2 +- spec/datadog/ci/test_suite_spec.rb | 89 ++++++++++++++++++++++++++++-- 2 files changed, 85 insertions(+), 6 deletions(-) diff --git a/lib/datadog/ci/test_suite.rb b/lib/datadog/ci/test_suite.rb index 97b58197..59b8e43d 100644 --- a/lib/datadog/ci/test_suite.rb +++ b/lib/datadog/ci/test_suite.rb @@ -23,7 +23,7 @@ def initialize(tracer_span) # @return [void] def finish synchronize do - # we need to derive test suite status from execution stats if no status was set explicitly + # we try to derive test suite status from execution stats if no status was set explicitly set_status_from_stats! if undefined? super diff --git a/spec/datadog/ci/test_suite_spec.rb b/spec/datadog/ci/test_suite_spec.rb index ea80fac5..82e53519 100644 --- a/spec/datadog/ci/test_suite_spec.rb +++ b/spec/datadog/ci/test_suite_spec.rb @@ -6,20 +6,99 @@ let(:recorder) { spy("recorder") } before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } + subject(:ci_test_suite) { described_class.new(tracer_span) } + + describe "#record_test_result" do + let(:failed_tests_count) { 2 } + let(:skipped_tests_count) { 3 } + let(:passed_tests_count) { 5 } + + before do + failed_tests_count.times do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::FAIL) + end + skipped_tests_count.times do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::SKIP) + end + passed_tests_count.times do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::PASS) + end + end + + it "records the test results" do + expect(ci_test_suite.failed_tests_count).to eq(failed_tests_count) + expect(ci_test_suite.skipped_tests_count).to eq(skipped_tests_count) + expect(ci_test_suite.passed_tests_count).to eq(passed_tests_count) + end + end describe "#finish" do - subject(:ci_test_suite) { described_class.new(tracer_span) } + subject(:finish) { ci_test_suite.finish } before do expect(tracer_span).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_STATUS).and_return( - Datadog::CI::Ext::Test::Status::PASS + test_suite_status ) end - it "deactivates the test suite" do - ci_test_suite.finish + context "when test suite has status" do + let(:test_suite_status) { Datadog::CI::Ext::Test::Status::PASS } + + it "deactivates the test suite" do + finish + + expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) + end + end + + context "when test suite has no status" do + let(:test_suite_status) { nil } + + context "and there are test failures" do + before do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::FAIL) + end + + it "sets the status to fail" do + expect(tracer_span).to receive(:set_tag).with( + Datadog::CI::Ext::Test::TAG_STATUS, Datadog::CI::Ext::Test::Status::FAIL + ) + expect(tracer_span).to receive(:status=).with(1) + + finish + end + end + + context "and there are only skipped tests" do + before do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::SKIP) + end + + it "sets the status to skip" do + expect(tracer_span).to receive(:set_tag).with( + Datadog::CI::Ext::Test::TAG_STATUS, Datadog::CI::Ext::Test::Status::SKIP + ) + + finish + end + end + + context "and there are some passed tests" do + before do + 2.times do + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::SKIP) + end + ci_test_suite.record_test_result(Datadog::CI::Ext::Test::Status::PASS) + end + + it "sets the status to pass" do + expect(tracer_span).to receive(:set_tag).with( + Datadog::CI::Ext::Test::TAG_STATUS, Datadog::CI::Ext::Test::Status::PASS + ) - expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) + finish + end + end end end end From a63799527e1f5c63043b1ea18418c3847c99f86e Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 24 Jan 2024 19:00:04 +0100 Subject: [PATCH 13/13] specs for Datadog::CI::Test tracking test results in the test_suite --- spec/datadog/ci/test_spec.rb | 79 ++++++++++++++++++++++++++++++ spec/datadog/ci/test_suite_spec.rb | 6 +++ 2 files changed, 85 insertions(+) diff --git a/spec/datadog/ci/test_spec.rb b/spec/datadog/ci/test_spec.rb index 3149126a..f474145f 100644 --- a/spec/datadog/ci/test_spec.rb +++ b/spec/datadog/ci/test_spec.rb @@ -110,4 +110,83 @@ ci_test.set_parameters(parameters) end end + + describe "#passed!" do + before { allow(ci_test).to receive(:test_suite).and_return(test_suite) } + + context "when test suite is set" do + let(:test_suite) { instance_double(Datadog::CI::TestSuite, record_test_result: true) } + + it "records the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "pass") + ci_test.passed! + + expect(test_suite).to have_received(:record_test_result).with("pass") + end + end + + context "when test suite is not set" do + let(:test_suite) { nil } + + it "does not record the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "pass") + + ci_test.passed! + end + end + end + + describe "#skipped!" do + before { allow(ci_test).to receive(:test_suite).and_return(test_suite) } + + context "when test suite is set" do + let(:test_suite) { instance_double(Datadog::CI::TestSuite, record_test_result: true) } + + it "records the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "skip") + + ci_test.skipped! + + expect(test_suite).to have_received(:record_test_result).with("skip") + end + end + + context "when test suite is not set" do + let(:test_suite) { nil } + + it "does not record the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "skip") + + ci_test.skipped! + end + end + end + + describe "#failed!" do + before { allow(ci_test).to receive(:test_suite).and_return(test_suite) } + + context "when test suite is set" do + let(:test_suite) { instance_double(Datadog::CI::TestSuite, record_test_result: true) } + + it "records the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "fail") + expect(tracer_span).to receive(:status=).with(1) + + ci_test.failed! + + expect(test_suite).to have_received(:record_test_result).with("fail") + end + end + + context "when test suite is not set" do + let(:test_suite) { nil } + + it "does not record the test result in the test suite" do + expect(tracer_span).to receive(:set_tag).with("test.status", "fail") + expect(tracer_span).to receive(:status=).with(1) + + ci_test.failed! + end + end + end end diff --git a/spec/datadog/ci/test_suite_spec.rb b/spec/datadog/ci/test_suite_spec.rb index 82e53519..5a144f68 100644 --- a/spec/datadog/ci/test_suite_spec.rb +++ b/spec/datadog/ci/test_suite_spec.rb @@ -66,6 +66,8 @@ expect(tracer_span).to receive(:status=).with(1) finish + + expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) end end @@ -80,6 +82,8 @@ ) finish + + expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) end end @@ -97,6 +101,8 @@ ) finish + + expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) end end end