From 44686007237bbb1406318248761012b12d34a421 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 13:45:48 +0100 Subject: [PATCH 01/10] convert type param for Datadog::CI.trace method to keyword and make it optional --- lib/datadog/ci.rb | 26 +++++++++---------- lib/datadog/ci/contrib/cucumber/formatter.rb | 2 +- lib/datadog/ci/test_visibility/recorder.rb | 2 +- sig/datadog/ci.rbs | 2 +- sig/datadog/ci/test_visibility/recorder.rbs | 2 +- .../ci/test_visibility/recorder_spec.rb | 8 +++--- spec/datadog/ci_spec.rb | 4 +-- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index 689dc2ad..d42b304a 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -32,8 +32,8 @@ class << self # # @param [String] service the service name for this session (optional, defaults to DD_SERVICE) # @param [Hash] tags extra tags which should be added to the test session. - # @return [Datadog::CI::TestSession] returns the active, running {Datadog::CI::TestSession}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is + # @return [Datadog::CI::TestSession] the active, running {Datadog::CI::TestSession}. + # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is # detected and test suite level visibility cannot be supported. def start_test_session(service: nil, tags: {}) service ||= Datadog.configuration.service @@ -88,8 +88,8 @@ def active_test_session # @param [String] test_module_name the name for this module # @param [String] service the service name for this session (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test module (optional, some tags are inherited from test session). - # @return [Datadog::CI::TestModule] returns the active, running {Datadog::CI::TestModule}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is + # @return [Datadog::CI::TestModule] the active, running {Datadog::CI::TestModule}. + # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is # detected and test suite level visibility cannot be supported. def start_test_module(test_module_name, service: nil, tags: {}) recorder.start_test_module(test_module_name, service: service, tags: tags) @@ -141,8 +141,8 @@ def active_test_module # @param [String] test_suite_name the name of the test suite # @param [String] service the service name for this test suite (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test module (optional, some tags are inherited from test session) - # @return [Datadog::CI::TestSuite] returns the active, running {Datadog::CI::TestSuite}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled or if old Datadog agent is + # @return [Datadog::CI::TestSuite] the active, running {Datadog::CI::TestSuite}. + # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is # detected and test suite level visibility cannot be supported. def start_test_suite(test_suite_name, service: nil, tags: {}) recorder.start_test_suite(test_suite_name, service: service, tags: tags) @@ -245,8 +245,8 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) # @param [String] test_suite_name name of test suite this test belongs to (example: "CalculatorTest"). # @param [String] service the service name for this span (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test. - # @return [Datadog::CI::Test] Returns the active, unfinished {Datadog::CI::Test}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @return [Datadog::CI::Test] the active, unfinished {Datadog::CI::Test}. + # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled def start_test(test_name, test_suite_name, service: nil, tags: {}) recorder.trace_test(test_name, test_suite_name, service: service, tags: tags) end @@ -260,8 +260,8 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # # ``` # Datadog::CI.trace( - # "step", # "Given I have 42 cucumbers", + # type: "step", # tags: {} # ) do # run_operation @@ -271,8 +271,8 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # The {.trace} method can also be used without a block in this way: # ``` # ci_span = Datadog::CI.trace( - # "step", # "Given I have 42 cucumbers", + # type: "step", # tags: {} # ) # # ... run test here ... @@ -280,8 +280,8 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # ``` # Remember that in this case, calling {Datadog::CI::Span#finish} is mandatory. # - # @param [String] type custom, user-defined span type (for example "step" or "query"). # @param [String] span_name the resource this span refers, or `test` if it's missing + # @param [String] type custom, user-defined span type (for example "step" or "query"). # @param [Hash] tags extra tags which should be added to the span. # @return [Object] If a block is provided, returns the result of the block execution. # @return [Datadog::CI::Span] If no block is provided, returns the active, @@ -290,8 +290,8 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # @yield Optional block where newly created {Datadog::CI::Span} captures the execution. # @yieldparam [Datadog::CI::Span] ci_span the newly created and active [Datadog::CI::Span] # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled - def trace(type, span_name, tags: {}, &block) - recorder.trace(type, span_name, tags: tags, &block) + def trace(span_name, type: "span", tags: {}, &block) + recorder.trace(span_name, type: type, tags: tags, &block) end # The active, unfinished custom span if it matches given type. diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index bd6bb173..62885f9d 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -94,7 +94,7 @@ def on_test_case_finished(event) end def on_test_step_started(event) - CI.trace(Ext::STEP_SPAN_TYPE, event.test_step.to_s) + CI.trace(event.test_step.to_s, type: Ext::STEP_SPAN_TYPE) end def on_test_step_finished(event) diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index e875f302..e46b3083 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -121,7 +121,7 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) end end - def trace(type, span_name, tags: {}, &block) + def trace(span_name, type: "span", tags: {}, &block) span_options = build_span_options( nil, # service name is completely optional for custom spans type, diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index f9495c5f..dca363ca 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -10,7 +10,7 @@ module Datadog def self.start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span - def self.trace: (String type, String span_name, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped + def self.trace: (String span_name, ?type: String, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped def self.active_test_session: () -> Datadog::CI::TestSession? diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index bd9d0335..f9ae8c3e 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -18,7 +18,7 @@ module Datadog def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped - def trace: (String type, String span_name, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped + def trace: (String span_name, ?type: String, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index d720bffd..f3c4a3a2 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -93,7 +93,7 @@ context "when given a block" do before do - recorder.trace(type, span_name, tags: tags) do |span| + recorder.trace(span_name, type: type, tags: tags) do |span| span.set_metric("my.metric", 42) end end @@ -117,7 +117,7 @@ context "when given a block" do before do - recorder.trace(type, span_name, tags: tags) do |span| + recorder.trace(span_name, type: type, tags: tags) do |span| span.set_metric("my.metric", 42) end end @@ -143,7 +143,7 @@ end context "without a block" do - subject { recorder.trace("step", "my test step", tags: tags) } + subject { recorder.trace("my test step", type: type, tags: tags) } it "returns a new CI span" do expect(subject).to be_kind_of(Datadog::CI::Span) @@ -590,7 +590,7 @@ end context "when span is started" do - let(:ci_span) { recorder.trace("step", "my test step") } + let(:ci_span) { recorder.trace("my test step", type: "step") } before do ci_span diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 5aeacbef..8b52341e 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -50,7 +50,7 @@ end describe "::trace" do - subject(:trace) { described_class.trace(type, span_name, **options, &block) } + subject(:trace) { described_class.trace(span_name, type: type, **options, &block) } let(:type) { "span type" } let(:span_name) { "span name" } @@ -60,7 +60,7 @@ let(:ci_span) { instance_double(Datadog::CI::Span) } before do - allow(recorder).to receive(:trace).with(type, span_name, **options, &block).and_return(ci_span) + allow(recorder).to receive(:trace).with(span_name, type: type, **options, &block).and_return(ci_span) end it { is_expected.to be(ci_span) } From e4c227cdb769e2cfb28ac4f7a5883e6293ef3d35 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:13:05 +0100 Subject: [PATCH 02/10] use Utils::Configuration to fetch default service name for session if none is provided --- lib/datadog/ci.rb | 6 +++--- sig/datadog/ci.rbs | 2 +- spec/datadog/ci_spec.rb | 8 ++++---- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index d42b304a..d20e6d67 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative "ci/version" +require_relative "ci/utils/configuration" require "datadog/core" @@ -30,13 +31,12 @@ class << self # # Remember that calling {Datadog::CI::TestSession#finish} is mandatory. # - # @param [String] service the service name for this session (optional, defaults to DD_SERVICE) + # @param [String] service the service name for this session (optional, defaults to DD_SERVICE or repository name) # @param [Hash] tags extra tags which should be added to the test session. # @return [Datadog::CI::TestSession] the active, running {Datadog::CI::TestSession}. # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is # detected and test suite level visibility cannot be supported. - def start_test_session(service: nil, tags: {}) - service ||= Datadog.configuration.service + def start_test_session(service: Utils::Configuration.fetch_service_name("test"), tags: {}) recorder.start_test_session(service: service, tags: tags) end diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index dca363ca..82a12b4d 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -4,7 +4,7 @@ module Datadog def self.start_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span - def self.start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def self.start_test_session: (?service: String, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span def self.start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 8b52341e..5e68575e 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -101,13 +101,11 @@ end describe "::start_test_session" do - let(:service) { nil } - subject(:start_test_session) { described_class.start_test_session(service: service) } - let(:ci_test_session) { instance_double(Datadog::CI::TestSession) } context "when service is provided" do let(:service) { "my-service" } + subject(:start_test_session) { described_class.start_test_session(service: service) } before do allow(recorder).to receive(:start_test_session).with(service: service, tags: {}).and_return(ci_test_session) @@ -117,9 +115,11 @@ end context "when service is not provided" do + subject(:start_test_session) { described_class.start_test_session } + context "when service is configured on library level" do before do - allow(Datadog.configuration).to receive(:service).and_return("configured-service") + allow(Datadog.configuration).to receive(:service_without_fallback).and_return("configured-service") allow(recorder).to receive(:start_test_session).with( service: "configured-service", tags: {} ).and_return(ci_test_session) From f314beb3b707227799491f0e5ad7e18e15342360 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:19:45 +0100 Subject: [PATCH 03/10] remove type parameter from Datadog::CI.active_span method --- lib/datadog/ci.rb | 10 +++++----- lib/datadog/ci/contrib/cucumber/formatter.rb | 2 +- sig/datadog/ci.rbs | 2 +- spec/datadog/ci_spec.rb | 8 ++++---- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index d20e6d67..71c9d9e6 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -2,6 +2,7 @@ require_relative "ci/version" require_relative "ci/utils/configuration" +require_relative "ci/ext/app_types" require "datadog/core" @@ -294,8 +295,8 @@ def trace(span_name, type: "span", tags: {}, &block) recorder.trace(span_name, type: type, tags: tags, &block) end - # The active, unfinished custom span if it matches given type. - # If no span is active, or if the active span is not a custom span with given type, returns nil. + # The active, unfinished custom (i.e. not test/suite/module/session) span. + # If no span is active, or if the active span is not a custom span, returns nil. # # The active span belongs to an {.active_test}. # @@ -314,12 +315,11 @@ def trace(span_name, type: "span", tags: {}, &block) # step_span.finish() # ``` # - # @param [String] type type of the span to retrieve (for example "step" or "query") that was provided to {.trace} # @return [Datadog::CI::Span] the active span # @return [nil] if no span is active, or if the active span is not a custom span with given type - def active_span(type) + def active_span span = recorder.active_span - span if span && span.type == type + span if span && !Ext::AppTypes::CI_SPAN_TYPES.include?(span.type) end # The active, unfinished test span. diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 62885f9d..1bfb1300 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -98,7 +98,7 @@ def on_test_step_started(event) end def on_test_step_finished(event) - current_step_span = CI.active_span(Ext::STEP_SPAN_TYPE) + current_step_span = CI.active_span return if current_step_span.nil? finish_test(current_step_span, event.result) diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index 82a12b4d..d32ec57b 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -20,7 +20,7 @@ module Datadog def self.active_test_suite: (String test_suite_name) -> Datadog::CI::TestSuite? - def self.active_span: (String type) -> Datadog::CI::Span? + def self.active_span: () -> Datadog::CI::Span? def self.deactivate_test: (Datadog::CI::Test test) -> void diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 5e68575e..2fad9e8e 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -67,11 +67,11 @@ end describe "::active_span" do - subject(:active_span) { described_class.active_span(type) } + subject(:active_span) { described_class.active_span } let(:type) { "span type" } - context "when span type matches current active span" do + context "when current active span has custom type" do let(:ci_span) { instance_double(Datadog::CI::Span, type: type) } before do @@ -81,8 +81,8 @@ it { is_expected.to be(ci_span) } end - context "when span type does not match current active span" do - let(:ci_span) { instance_double(Datadog::CI::Span, type: "other span type") } + context "when current active span is a test" do + let(:ci_span) { instance_double(Datadog::CI::Span, type: "test") } before do allow(recorder).to receive(:active_span).and_return(ci_span) From f23ace0e7a4e449d17ae03488cc8da26eba51fa2 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:28:05 +0100 Subject: [PATCH 04/10] raise when Datadog::CI.trace is used to trace span types that are reserved for CI visibility --- lib/datadog/ci.rb | 11 +++++++++++ sig/datadog/ci.rbs | 3 +++ spec/datadog/ci_spec.rb | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index 71c9d9e6..f1988868 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -11,6 +11,8 @@ module Datadog # # @public_api module CI + class ReservedTypeError < StandardError; end + class << self # Starts a {Datadog::CI::TestSession ci_test_session} that represents the whole test session run. # @@ -288,10 +290,19 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # @return [Datadog::CI::Span] If no block is provided, returns the active, # unfinished {Datadog::CI::Span}. # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @raise [ReservedTypeError] if provided type is reserved for Datadog CI visibility # @yield Optional block where newly created {Datadog::CI::Span} captures the execution. # @yieldparam [Datadog::CI::Span] ci_span the newly created and active [Datadog::CI::Span] # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled def trace(span_name, type: "span", tags: {}, &block) + if Ext::AppTypes::CI_SPAN_TYPES.include?(type) + raise( + ReservedTypeError, + "Span type #{type} is reserved for Datadog CI visibility. " \ + "Reserved types are: #{Ext::AppTypes::CI_SPAN_TYPES}" + ) + end + recorder.trace(span_name, type: type, tags: tags, &block) end diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index d32ec57b..3e54f313 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -1,5 +1,8 @@ module Datadog module CI + class ReservedTypeError < StandardError + end + def self.trace_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span test) -> untyped } -> untyped def self.start_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 2fad9e8e..6a98a232 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -64,6 +64,14 @@ end it { is_expected.to be(ci_span) } + + context "when using reserved type" do + let(:type) { Datadog::CI::Ext::AppTypes::TYPE_TEST } + + it "raises error" do + expect { trace }.to raise_error(Datadog::CI::ReservedTypeError) + end + end end describe "::active_span" do From cbaaa45f2504faf1a4c7c14e9d1ae20265092c08 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:28:58 +0100 Subject: [PATCH 05/10] Ext::AppTypes is part of public API --- lib/datadog/ci/ext/app_types.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/datadog/ci/ext/app_types.rb b/lib/datadog/ci/ext/app_types.rb index a54ab5b3..274ce1da 100644 --- a/lib/datadog/ci/ext/app_types.rb +++ b/lib/datadog/ci/ext/app_types.rb @@ -3,6 +3,8 @@ module Datadog module CI module Ext + # Defines span types for CI visibility + # @public_api module AppTypes TYPE_TEST = "test" TYPE_TEST_SESSION = "test_session_end" From cff5bb1533eca87893a8bd4395565b1ea0b0e787 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:39:38 +0100 Subject: [PATCH 06/10] remove unncessary parameter from CI.deactivate_test --- lib/datadog/ci.rb | 4 +- lib/datadog/ci/test.rb | 2 +- .../ci/test_visibility/context/local.rb | 12 ++---- lib/datadog/ci/test_visibility/recorder.rb | 8 ++-- sig/datadog/ci.rbs | 2 +- .../ci/test_visibility/context/local.rbs | 4 +- sig/datadog/ci/test_visibility/recorder.rbs | 2 +- spec/datadog/ci/test_spec.rb | 2 +- .../ci/test_visibility/context/local_spec.rb | 41 ++++++++----------- .../ci/test_visibility/recorder_spec.rb | 15 +------ 10 files changed, 32 insertions(+), 60 deletions(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index f1988868..39ad566d 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -360,8 +360,8 @@ def active_test # Internal only, to finish a test use {Datadog::CI::Test#finish} # @private - def deactivate_test(test) - recorder.deactivate_test(test) + def deactivate_test + recorder.deactivate_test end # Internal only, to finish a test session use {Datadog::CI::TestSession#finish} diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index 8b9a3f99..af79d30e 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -20,7 +20,7 @@ def name def finish super - CI.deactivate_test(self) + CI.deactivate_test end # Running test suite that this test is part of (if any). diff --git a/lib/datadog/ci/test_visibility/context/local.rb b/lib/datadog/ci/test_visibility/context/local.rb index 4950d54a..83f87a8e 100644 --- a/lib/datadog/ci/test_visibility/context/local.rb +++ b/lib/datadog/ci/test_visibility/context/local.rb @@ -11,7 +11,7 @@ def initialize self.active_test = nil end - def activate_test!(test) + def activate_test(test) raise "Nested tests are not supported. Currently active test: #{active_test}" unless active_test.nil? if block_given? @@ -26,14 +26,8 @@ def activate_test!(test) end end - def deactivate_test!(test) - return if active_test.nil? - - if active_test == test - self.active_test = nil - else - raise "Trying to deactivate test #{test}, but currently active test is #{active_test}" - end + def deactivate_test + self.active_test = nil end def active_test diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index e46b3083..ca7c87e6 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -108,7 +108,7 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) start_datadog_tracer_span(test_name, span_options) do |tracer_span| test = build_test(tracer_span, tags) - @local_context.activate_test!(test) do + @local_context.activate_test(test) do block.call(test) end end @@ -116,7 +116,7 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) tracer_span = start_datadog_tracer_span(test_name, span_options) test = build_test(tracer_span, tags) - @local_context.activate_test!(test) + @local_context.activate_test(test) test end end @@ -160,8 +160,8 @@ def active_test_suite(test_suite_name) @global_context.active_test_suite(test_suite_name) end - def deactivate_test(test) - @local_context.deactivate_test!(test) + def deactivate_test + @local_context.deactivate_test end def deactivate_test_session diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index 3e54f313..b5df8d65 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -25,7 +25,7 @@ module Datadog def self.active_span: () -> Datadog::CI::Span? - def self.deactivate_test: (Datadog::CI::Test test) -> void + def self.deactivate_test: () -> void def self.deactivate_test_session: () -> void diff --git a/sig/datadog/ci/test_visibility/context/local.rbs b/sig/datadog/ci/test_visibility/context/local.rbs index 28f6d1c6..3ce7c880 100644 --- a/sig/datadog/ci/test_visibility/context/local.rbs +++ b/sig/datadog/ci/test_visibility/context/local.rbs @@ -7,9 +7,9 @@ module Datadog def initialize: () -> void - def activate_test!: (Datadog::CI::Test test) ?{ () -> untyped } -> void + def activate_test: (Datadog::CI::Test test) ?{ () -> untyped } -> void - def deactivate_test!: (Datadog::CI::Test test) -> void + def deactivate_test: () -> void def active_test: () -> Datadog::CI::Test? diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index f9ae8c3e..c5855cae 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -36,7 +36,7 @@ module Datadog def active_span: () -> Datadog::CI::Span? - def deactivate_test: (Datadog::CI::Test test) -> void + def deactivate_test: () -> void def deactivate_test_session: () -> void diff --git a/spec/datadog/ci/test_spec.rb b/spec/datadog/ci/test_spec.rb index 05fc8ed1..7084522b 100644 --- a/spec/datadog/ci/test_spec.rb +++ b/spec/datadog/ci/test_spec.rb @@ -19,7 +19,7 @@ it "deactivates the test" do ci_test.finish - expect(Datadog::CI).to have_received(:deactivate_test).with(ci_test) + expect(Datadog::CI).to have_received(:deactivate_test) end end diff --git a/spec/datadog/ci/test_visibility/context/local_spec.rb b/spec/datadog/ci/test_visibility/context/local_spec.rb index 0c6aedd0..48273bc7 100644 --- a/spec/datadog/ci/test_visibility/context/local_spec.rb +++ b/spec/datadog/ci/test_visibility/context/local_spec.rb @@ -5,12 +5,12 @@ let(:ci_test) { Datadog::CI::Test.new(tracer_span) } let(:ci_test2) { Datadog::CI::Test.new(tracer_span) } - describe "#activate_test!" do + describe "#activate_test" do context "when a test is already active" do it "raises an error" do - subject.activate_test!(Datadog::CI::Test.new(tracer_span)) + subject.activate_test(Datadog::CI::Test.new(tracer_span)) - expect { subject.activate_test!(ci_test) }.to( + expect { subject.activate_test(ci_test) }.to( raise_error( RuntimeError, "Nested tests are not supported. Currently active test: " \ @@ -23,7 +23,7 @@ context "when no test is active" do context "when a block is given" do it "activates the test for the duration of the block" do - subject.activate_test!(ci_test) do + subject.activate_test(ci_test) do expect(subject.active_test).to be(ci_test) end @@ -33,7 +33,7 @@ context "when no block is given" do it "activates the test" do - subject.activate_test!(ci_test) + subject.activate_test(ci_test) expect(subject.active_test).to be(ci_test) end end @@ -41,10 +41,10 @@ context "with multiple fibers" do it "create one fiber-local variable per fiber" do - subject.activate_test!(ci_test) + subject.activate_test(ci_test) Fiber.new do - subject.activate_test!(ci_test2) + subject.activate_test(ci_test2) expect(subject.active_test).to be(ci_test2) end.resume @@ -55,10 +55,10 @@ context "with multiple threads" do it "create one thread-local variable per thread" do - subject.activate_test!(ci_test) + subject.activate_test(ci_test) Thread.new do - subject.activate_test!(ci_test2) + subject.activate_test(ci_test2) expect(subject.active_test).to be(ci_test2) end.join @@ -68,28 +68,19 @@ end end - describe "#deactivate_test!" do + describe "#deactivate_test" do context "when no test is active" do it "does nothing" do - expect { subject.deactivate_test!(ci_test) }.not_to raise_error + expect { subject.deactivate_test }.not_to raise_error end end context "when a test is active" do - before { subject.activate_test!(ci_test) } + before { subject.activate_test(ci_test) } - context "when the test is the active test" do - it "deactivates the test" do - subject.deactivate_test!(ci_test) - expect(subject.active_test).to be_nil - end - end - - context "when the test is not the active test" do - it "raises an error" do - expect { subject.deactivate_test!(Datadog::CI::Test.new(tracer_span)) } - .to raise_error(RuntimeError, /Trying to deactivate test/) - end + it "deactivates the test" do + subject.deactivate_test + expect(subject.active_test).to be_nil end end end @@ -102,7 +93,7 @@ end context "when a test is active" do - before { subject.activate_test!(ci_test) } + before { subject.activate_test(ci_test) } it "returns the active test" do expect(subject.active_test).to be(ci_test) diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index f3c4a3a2..c82c7cc4 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -604,7 +604,7 @@ end describe "#deactivate_test" do - subject { recorder.deactivate_test(ci_test) } + subject { recorder.deactivate_test } context "when there is no active test" do let(:ci_test) { Datadog::CI::Test.new(double("tracer span")) } @@ -621,19 +621,6 @@ expect(recorder.active_test).to be_nil end end - - context "when deactivating a different test from the one that is running right now" do - let(:ci_test) { Datadog::CI::Test.new(double("tracer span", get_tag: "wrong test")) } - - before do - recorder.trace_test("my test", "my suite") - end - - it "raises an error" do - expect { subject }.to raise_error(/Trying to deactivate test Datadog::CI::Test\(name:wrong test/) - expect(recorder.active_test).not_to be_nil - end - end end describe "#deactivate_test_session" do From b954c9b87abc47edc2320cffa96142e6b73a64fd Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Wed, 17 Jan 2024 14:53:23 +0100 Subject: [PATCH 07/10] remove deactivation methods from Datadog::CI, call them on Recorder directly to avoid exposing via public API --- lib/datadog/ci.rb | 24 -------------- lib/datadog/ci/span.rb | 7 +++++ lib/datadog/ci/test.rb | 2 +- lib/datadog/ci/test_module.rb | 2 +- lib/datadog/ci/test_session.rb | 2 +- lib/datadog/ci/test_suite.rb | 2 +- .../ci/test_visibility/null_recorder.rb | 2 +- sig/datadog/ci.rbs | 8 +---- sig/datadog/ci/span.rbs | 4 +++ .../ci/test_visibility/null_recorder.rbs | 2 +- spec/datadog/ci/test_module_spec.rb | 7 +++-- spec/datadog/ci/test_session_spec.rb | 7 +++-- spec/datadog/ci/test_spec.rb | 7 +++-- spec/datadog/ci/test_suite_spec.rb | 7 +++-- .../ci/test_visibility/null_recorder_spec.rb | 4 +-- spec/datadog/ci_spec.rb | 31 ------------------- 16 files changed, 35 insertions(+), 83 deletions(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index 39ad566d..452b659b 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -358,30 +358,6 @@ def active_test recorder.active_test end - # Internal only, to finish a test use {Datadog::CI::Test#finish} - # @private - def deactivate_test - recorder.deactivate_test - end - - # Internal only, to finish a test session use {Datadog::CI::TestSession#finish} - # @private - def deactivate_test_session - recorder.deactivate_test_session - end - - # Internal only, to finish a test module use {Datadog::CI::TestModule#finish} - # @private - def deactivate_test_module - recorder.deactivate_test_module - end - - # Internal only, to finish a test suite use {Datadog::CI::TestSuite#finish} - # @private - def deactivate_test_suite(test_suite_name) - recorder.deactivate_test_suite(test_suite_name) - end - private def components diff --git a/lib/datadog/ci/span.rb b/lib/datadog/ci/span.rb index 9ff7e300..aa3cb71c 100644 --- a/lib/datadog/ci/span.rb +++ b/lib/datadog/ci/span.rb @@ -155,6 +155,13 @@ def set_parameters(arguments, metadata = {}) def to_s "#{self.class}(name:#{name},tracer_span:#{@tracer_span})" end + + private + + # provides access to global CI recorder for CI models to deactivate themselves + def recorder + Datadog.send(:components).ci_recorder + end end end end diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index af79d30e..e2017ec5 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -20,7 +20,7 @@ def name def finish super - CI.deactivate_test + recorder.deactivate_test end # Running test suite that this test is part of (if any). diff --git a/lib/datadog/ci/test_module.rb b/lib/datadog/ci/test_module.rb index 95f13c88..4fb8678b 100644 --- a/lib/datadog/ci/test_module.rb +++ b/lib/datadog/ci/test_module.rb @@ -16,7 +16,7 @@ class TestModule < ConcurrentSpan def finish super - CI.deactivate_test_module + recorder.deactivate_test_module end end end diff --git a/lib/datadog/ci/test_session.rb b/lib/datadog/ci/test_session.rb index e60e1902..c3216f5e 100644 --- a/lib/datadog/ci/test_session.rb +++ b/lib/datadog/ci/test_session.rb @@ -17,7 +17,7 @@ class TestSession < ConcurrentSpan def finish super - CI.deactivate_test_session + recorder.deactivate_test_session end # Return the test session's name which is equal to test command used diff --git a/lib/datadog/ci/test_suite.rb b/lib/datadog/ci/test_suite.rb index 295ea91a..3a7f7213 100644 --- a/lib/datadog/ci/test_suite.rb +++ b/lib/datadog/ci/test_suite.rb @@ -18,7 +18,7 @@ class TestSuite < ConcurrentSpan def finish super - CI.deactivate_test_suite(name) + recorder.deactivate_test_suite(name) end end end diff --git a/lib/datadog/ci/test_visibility/null_recorder.rb b/lib/datadog/ci/test_visibility/null_recorder.rb index 258524f9..35529a38 100644 --- a/lib/datadog/ci/test_visibility/null_recorder.rb +++ b/lib/datadog/ci/test_visibility/null_recorder.rb @@ -42,7 +42,7 @@ def active_test_module def active_test_suite(test_suite_name) end - def deactivate_test(test) + def deactivate_test end def deactivate_test_session diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index b5df8d65..4a2c1b04 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -25,13 +25,7 @@ module Datadog def self.active_span: () -> Datadog::CI::Span? - def self.deactivate_test: () -> void - - def self.deactivate_test_session: () -> void - - def self.deactivate_test_module: () -> void - - def self.deactivate_test_suite: (String test_suite_name) -> void + private def self.components: () -> Datadog::CI::Configuration::Components diff --git a/sig/datadog/ci/span.rbs b/sig/datadog/ci/span.rbs index 34b5481d..7dcf3f5d 100644 --- a/sig/datadog/ci/span.rbs +++ b/sig/datadog/ci/span.rbs @@ -44,6 +44,10 @@ module Datadog def set_default_tags: () -> void def set_parameters: (Hash[String, Object] arguments, ?Hash[String, Object] metadata) -> void + + private + + def recorder: () -> Datadog::CI::TestVisibility::Recorder end end end diff --git a/sig/datadog/ci/test_visibility/null_recorder.rbs b/sig/datadog/ci/test_visibility/null_recorder.rbs index 3526765e..81ef0993 100644 --- a/sig/datadog/ci/test_visibility/null_recorder.rbs +++ b/sig/datadog/ci/test_visibility/null_recorder.rbs @@ -26,7 +26,7 @@ module Datadog def active_span: () -> Datadog::CI::Span? - def deactivate_test: (Datadog::CI::Test test) -> void + def deactivate_test: () -> void def deactivate_test_session: () -> void diff --git a/spec/datadog/ci/test_module_spec.rb b/spec/datadog/ci/test_module_spec.rb index 98d6d08d..9b4d97ff 100644 --- a/spec/datadog/ci/test_module_spec.rb +++ b/spec/datadog/ci/test_module_spec.rb @@ -2,16 +2,17 @@ RSpec.describe Datadog::CI::TestModule do let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true) } + let(:recorder) { spy("recorder") } + + before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } describe "#finish" do subject(:ci_test_module) { described_class.new(tracer_span) } - before { allow(Datadog::CI).to receive(:deactivate_test_module) } - it "deactivates the test module" do ci_test_module.finish - expect(Datadog::CI).to have_received(:deactivate_test_module) + expect(recorder).to have_received(:deactivate_test_module) end end end diff --git a/spec/datadog/ci/test_session_spec.rb b/spec/datadog/ci/test_session_spec.rb index 47a244b3..52ce638e 100644 --- a/spec/datadog/ci/test_session_spec.rb +++ b/spec/datadog/ci/test_session_spec.rb @@ -2,16 +2,17 @@ RSpec.describe Datadog::CI::TestSession do let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true) } + let(:recorder) { spy("recorder") } + + before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } describe "#finish" do subject(:ci_test_session) { described_class.new(tracer_span) } - before { allow(Datadog::CI).to receive(:deactivate_test_session) } - it "deactivates the test session" do ci_test_session.finish - expect(Datadog::CI).to have_received(:deactivate_test_session) + expect(recorder).to have_received(:deactivate_test_session) end end diff --git a/spec/datadog/ci/test_spec.rb b/spec/datadog/ci/test_spec.rb index 7084522b..89139a37 100644 --- a/spec/datadog/ci/test_spec.rb +++ b/spec/datadog/ci/test_spec.rb @@ -2,6 +2,9 @@ RSpec.describe Datadog::CI::Test do let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true) } + let(:recorder) { spy("recorder") } + + before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } describe "#name" do subject(:name) { ci_test.name } @@ -15,11 +18,9 @@ describe "#finish" do subject(:ci_test) { described_class.new(tracer_span) } - before { allow(Datadog::CI).to receive(:deactivate_test) } - it "deactivates the test" do ci_test.finish - expect(Datadog::CI).to have_received(:deactivate_test) + expect(recorder).to have_received(:deactivate_test) end end diff --git a/spec/datadog/ci/test_suite_spec.rb b/spec/datadog/ci/test_suite_spec.rb index d6019ccf..46d854e5 100644 --- a/spec/datadog/ci/test_suite_spec.rb +++ b/spec/datadog/ci/test_suite_spec.rb @@ -3,16 +3,17 @@ RSpec.describe Datadog::CI::TestSuite do let(:test_suite_name) { "my.suite" } let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true, name: test_suite_name) } + let(:recorder) { spy("recorder") } + + before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } describe "#finish" do subject(:ci_test_suite) { described_class.new(tracer_span) } - before { allow(Datadog::CI).to receive(:deactivate_test_suite) } - it "deactivates the test suite" do ci_test_suite.finish - expect(Datadog::CI).to have_received(:deactivate_test_suite).with(test_suite_name) + expect(recorder).to have_received(:deactivate_test_suite).with(test_suite_name) end end end diff --git a/spec/datadog/ci/test_visibility/null_recorder_spec.rb b/spec/datadog/ci/test_visibility/null_recorder_spec.rb index 2f8b1c81..b013ae60 100644 --- a/spec/datadog/ci/test_visibility/null_recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/null_recorder_spec.rb @@ -115,9 +115,7 @@ end describe "#deactivate_test" do - let(:ci_test) { double } - - subject { recorder.deactivate_test(ci_test) } + subject { recorder.deactivate_test } it { is_expected.to be_nil } end diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 6a98a232..7c58a296 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -150,16 +150,6 @@ it { is_expected.to be(ci_test_session) } end - describe "::deactivate_test_session" do - subject(:deactivate_test_session) { described_class.deactivate_test_session } - - before do - allow(recorder).to receive(:deactivate_test_session) - end - - it { is_expected.to be_nil } - end - describe "::start_test_module" do subject(:start_test_module) { described_class.start_test_module("my-module") } @@ -186,16 +176,6 @@ it { is_expected.to be(ci_test_module) } end - describe "::deactivate_test_module" do - subject(:deactivate_test_module) { described_class.deactivate_test_module } - - before do - allow(recorder).to receive(:deactivate_test_module) - end - - it { is_expected.to be_nil } - end - describe "::start_test_suite" do subject(:start_test_suite) { described_class.start_test_suite("my-suite") } @@ -222,17 +202,6 @@ it { is_expected.to be(ci_test_suite) } end - - describe "::deactivate_test_suite" do - let(:test_suite_name) { "my-suite" } - subject(:deactivate_test_suite) { described_class.deactivate_test_suite(test_suite_name) } - - before do - allow(recorder).to receive(:deactivate_test_suite).with(test_suite_name) - end - - it { is_expected.to be_nil } - end end context "integration testing the manual API" do From 20918ba62b76fe898b9bdc339d6693474867a1ff Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 18 Jan 2024 13:00:39 +0100 Subject: [PATCH 08/10] use module instance variables for caching in Utils::Git --- lib/datadog/ci/utils/git.rb | 14 +++++++------- sig/datadog/ci/utils/git.rbs | 3 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lib/datadog/ci/utils/git.rb b/lib/datadog/ci/utils/git.rb index 4d23e1d3..859ca049 100644 --- a/lib/datadog/ci/utils/git.rb +++ b/lib/datadog/ci/utils/git.rb @@ -21,14 +21,14 @@ def self.is_git_tag?(ref) end def self.root - return @@root if defined?(@@root) + return @root if defined?(@root) - @@root = exec_git_command("git rev-parse --show-toplevel") + @root = exec_git_command("git rev-parse --show-toplevel") rescue => e Datadog.logger.debug( "Unable to read git root: #{e.class.name} #{e.message} at #{Array(e.backtrace).first}" ) - @@root = nil + @root = nil end def self.relative_to_root(path) @@ -44,19 +44,19 @@ def self.relative_to_root(path) end def self.repository_name - return @@repository_name if defined?(@@repository_name) + return @repository_name if defined?(@repository_name) git_remote_url = exec_git_command("git ls-remote --get-url origin") # return git repository name from remote url without .git extension last_path_segment = git_remote_url.split("/").last if git_remote_url - @@repository_name = last_path_segment.gsub(".git", "") if last_path_segment - @@repository_name ||= current_folder_name + @repository_name = last_path_segment.gsub(".git", "") if last_path_segment + @repository_name ||= current_folder_name rescue => e Datadog.logger.debug( "Unable to get git remote: #{e.class.name} #{e.message} at #{Array(e.backtrace).first}" ) - @@repository_name = current_folder_name + @repository_name = current_folder_name end def self.current_folder_name diff --git a/sig/datadog/ci/utils/git.rbs b/sig/datadog/ci/utils/git.rbs index 04e96f78..9b33a668 100644 --- a/sig/datadog/ci/utils/git.rbs +++ b/sig/datadog/ci/utils/git.rbs @@ -2,7 +2,8 @@ module Datadog module CI module Utils module Git - @@root: String? + @root: String? + @repository_name: String? def self.normalize_ref: (String? name) -> String? From 9e9410e657ee8e59d362262722c8baf90bdee801 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 18 Jan 2024 14:34:09 +0100 Subject: [PATCH 09/10] remove NullSpan and get rid of NullObject completely because it interfered with class hierarchy (see #set_parameters method - now it can be moved down to Datadog::CI::Test model) --- lib/datadog/ci.rb | 19 +++-- lib/datadog/ci/contrib/cucumber/formatter.rb | 6 +- lib/datadog/ci/contrib/minitest/plugin.rb | 2 +- lib/datadog/ci/contrib/minitest/runnable.rb | 3 +- lib/datadog/ci/contrib/rspec/example.rb | 20 +++--- lib/datadog/ci/contrib/rspec/example_group.rb | 1 + lib/datadog/ci/contrib/rspec/runner.rb | 3 +- lib/datadog/ci/null_span.rb | 66 ------------------ lib/datadog/ci/span.rb | 19 ----- lib/datadog/ci/test.rb | 20 ++++++ lib/datadog/ci/test_session.rb | 2 +- .../ci/test_visibility/null_recorder.rb | 22 +----- lib/datadog/ci/test_visibility/recorder.rb | 11 +-- sig/datadog/ci.rbs | 10 +-- sig/datadog/ci/null_span.rbs | 37 ---------- .../ci/test_visibility/null_recorder.rbs | 34 +++------ sig/datadog/ci/test_visibility/recorder.rbs | 10 ++- spec/datadog/ci/null_span_spec.rb | 69 ------------------- spec/datadog/ci/span_spec.rb | 12 ---- spec/datadog/ci/test_spec.rb | 22 +++--- .../ci/test_visibility/null_recorder_spec.rb | 43 +++--------- .../ci/test_visibility/recorder_spec.rb | 6 +- spec/datadog/ci_spec.rb | 6 +- spec/support/tracer_helpers.rb | 23 ++++--- 24 files changed, 115 insertions(+), 351 deletions(-) delete mode 100644 lib/datadog/ci/null_span.rb delete mode 100644 sig/datadog/ci/null_span.rbs delete mode 100644 spec/datadog/ci/null_span_spec.rb diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index 452b659b..c05a5bae 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -37,8 +37,7 @@ class << self # @param [String] service the service name for this session (optional, defaults to DD_SERVICE or repository name) # @param [Hash] tags extra tags which should be added to the test session. # @return [Datadog::CI::TestSession] the active, running {Datadog::CI::TestSession}. - # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is - # detected and test suite level visibility cannot be supported. + # @return [nil] if test suite level visibility is disabled or CI mode is disabled. def start_test_session(service: Utils::Configuration.fetch_service_name("test"), tags: {}) recorder.start_test_session(service: service, tags: tags) end @@ -92,8 +91,7 @@ def active_test_session # @param [String] service the service name for this session (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test module (optional, some tags are inherited from test session). # @return [Datadog::CI::TestModule] the active, running {Datadog::CI::TestModule}. - # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is - # detected and test suite level visibility cannot be supported. + # @return [nil] if test suite level visibility is disabled or CI mode is disabled. def start_test_module(test_module_name, service: nil, tags: {}) recorder.start_test_module(test_module_name, service: service, tags: tags) end @@ -145,8 +143,7 @@ def active_test_module # @param [String] service the service name for this test suite (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test module (optional, some tags are inherited from test session) # @return [Datadog::CI::TestSuite] the active, running {Datadog::CI::TestSuite}. - # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled or if old Datadog agent is - # detected and test suite level visibility cannot be supported. + # @return [nil] if test suite level visibility is disabled or CI mode is disabled. def start_test_suite(test_suite_name, service: nil, tags: {}) recorder.start_test_suite(test_suite_name, service: service, tags: tags) end @@ -220,10 +217,10 @@ def active_test_suite(test_suite_name) # @return [Object] If a block is provided, returns the result of the block execution. # @return [Datadog::CI::Test] If no block is provided, returns the active, # unfinished {Datadog::CI::Test}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @return [nil] if no block is provided and CI mode is disabled. # @yield Optional block where newly created {Datadog::CI::Test} captures the execution. # @yieldparam [Datadog::CI::Test] ci_test the newly created and active [Datadog::CI::Test] - # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @yieldparam [nil] if CI mode is disabled def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) recorder.trace_test(test_name, test_suite_name, service: service, tags: tags, &block) end @@ -249,7 +246,7 @@ def trace_test(test_name, test_suite_name, service: nil, tags: {}, &block) # @param [String] service the service name for this span (optional, inherited from test session if not provided) # @param [Hash] tags extra tags which should be added to the test. # @return [Datadog::CI::Test] the active, unfinished {Datadog::CI::Test}. - # @return [Datadog::CI::NullSpan] null object if CI visibility is disabled + # @return [nil] if CI mode is disabled. def start_test(test_name, test_suite_name, service: nil, tags: {}) recorder.trace_test(test_name, test_suite_name, service: service, tags: tags) end @@ -289,11 +286,11 @@ def start_test(test_name, test_suite_name, service: nil, tags: {}) # @return [Object] If a block is provided, returns the result of the block execution. # @return [Datadog::CI::Span] If no block is provided, returns the active, # unfinished {Datadog::CI::Span}. - # @return [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @return [nil] if CI visibility is disabled # @raise [ReservedTypeError] if provided type is reserved for Datadog CI visibility # @yield Optional block where newly created {Datadog::CI::Span} captures the execution. # @yieldparam [Datadog::CI::Span] ci_span the newly created and active [Datadog::CI::Span] - # @yieldparam [Datadog::CI::NullSpan] ci_span null object if CI visibility is disabled + # @yieldparam [nil] ci_span if CI visibility is disabled def trace(span_name, type: "span", tags: {}, &block) if Ext::AppTypes::CI_SPAN_TYPES.include?(type) raise( diff --git a/lib/datadog/ci/contrib/cucumber/formatter.rb b/lib/datadog/ci/contrib/cucumber/formatter.rb index 1bfb1300..d5cd2dc9 100644 --- a/lib/datadog/ci/contrib/cucumber/formatter.rb +++ b/lib/datadog/ci/contrib/cucumber/formatter.rb @@ -41,7 +41,7 @@ def on_test_run_started(event) }, service: configuration[:service_name] ) - CI.start_test_module(test_session.name) + CI.start_test_module(test_session.name) if test_session end def on_test_run_finished(event) @@ -71,7 +71,7 @@ def on_test_case_started(event) service: configuration[:service_name] ) - if (parameters = extract_parameters_hash(event.test_case)) + if test_span && (parameters = extract_parameters_hash(event.test_case)) test_span.set_parameters(parameters) end end @@ -157,7 +157,7 @@ def start_test_suite(test_suite_name) test_suite = CI.start_test_suite(test_suite_name) # will be overridden if any test fails - test_suite.passed! + test_suite.passed! if test_suite @current_test_suite = test_suite end diff --git a/lib/datadog/ci/contrib/minitest/plugin.rb b/lib/datadog/ci/contrib/minitest/plugin.rb index 0595dcbf..3234cf19 100644 --- a/lib/datadog/ci/contrib/minitest/plugin.rb +++ b/lib/datadog/ci/contrib/minitest/plugin.rb @@ -54,7 +54,7 @@ def plugin_datadog_ci_init(*) }, service: datadog_configuration[:service_name] ) - CI.start_test_module(test_session.name) + CI.start_test_module(test_session.name) if test_session reporter.reporters << DatadogReporter.new(reporter) end diff --git a/lib/datadog/ci/contrib/minitest/runnable.rb b/lib/datadog/ci/contrib/minitest/runnable.rb index cd239ae0..e3aa2272 100644 --- a/lib/datadog/ci/contrib/minitest/runnable.rb +++ b/lib/datadog/ci/contrib/minitest/runnable.rb @@ -20,9 +20,10 @@ def run(*) test_suite_name = Helpers.test_suite_name(self, method) test_suite = Datadog::CI.start_test_suite(test_suite_name) - test_suite.passed! # will be overridden if any test fails + test_suite.passed! if test_suite # will be overridden if any test fails results = super + return results unless test_suite test_suite.finish diff --git a/lib/datadog/ci/contrib/rspec/example.rb b/lib/datadog/ci/contrib/rspec/example.rb index 3b34f5a3..598cc860 100644 --- a/lib/datadog/ci/contrib/rspec/example.rb +++ b/lib/datadog/ci/contrib/rspec/example.rb @@ -41,17 +41,19 @@ def run(*) }, service: datadog_configuration[:service_name] ) do |test_span| - test_span.set_parameters({}, {"scoped_id" => metadata[:scoped_id]}) - result = super - case execution_result.status - when :passed - test_span.passed! - when :failed - test_span.failed!(exception: execution_result.exception) - else - test_span.skipped!(exception: execution_result.exception) if execution_result.example_skipped? + if test_span + test_span.set_parameters({}, {"scoped_id" => metadata[:scoped_id]}) + + case execution_result.status + when :passed + test_span.passed! + when :failed + test_span.failed!(exception: execution_result.exception) + else + test_span.skipped!(exception: execution_result.exception) if execution_result.example_skipped? + end end result diff --git a/lib/datadog/ci/contrib/rspec/example_group.rb b/lib/datadog/ci/contrib/rspec/example_group.rb index cd3725e9..4da06e3a 100644 --- a/lib/datadog/ci/contrib/rspec/example_group.rb +++ b/lib/datadog/ci/contrib/rspec/example_group.rb @@ -23,6 +23,7 @@ def run(*) test_suite = Datadog::CI.start_test_suite(suite_name) result = super + return result unless test_suite if result test_suite.passed! diff --git a/lib/datadog/ci/contrib/rspec/runner.rb b/lib/datadog/ci/contrib/rspec/runner.rb index 75af699f..b9aed9c1 100644 --- a/lib/datadog/ci/contrib/rspec/runner.rb +++ b/lib/datadog/ci/contrib/rspec/runner.rb @@ -25,9 +25,10 @@ def run_specs(*) service: datadog_configuration[:service_name] ) - test_module = CI.start_test_module(test_session.name) + test_module = CI.start_test_module(test_session.name) if test_session result = super + return result unless test_module && test_session if result != 0 # TODO: repeating this twice feels clunky, we need to remove test_module API before GA diff --git a/lib/datadog/ci/null_span.rb b/lib/datadog/ci/null_span.rb deleted file mode 100644 index c9d1f285..00000000 --- a/lib/datadog/ci/null_span.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -require "datadog/tracing/span_operation" - -module Datadog - module CI - # Represents an ignored span when CI visibility is disabled. - # Replaces all methods with no-op. - # - # @public_api - class NullSpan < Span - def initialize - super(Datadog::Tracing::SpanOperation.new("null.span")) - end - - def id - end - - def name - end - - def service - end - - def type - end - - def passed! - end - - def failed!(exception: nil) - end - - def skipped!(exception: nil, reason: nil) - end - - def get_tag(key) - end - - def set_tag(key, value) - end - - def set_metric(key, value) - end - - def finish - end - - def set_tags(tags) - end - - def set_environment_runtime_tags - end - - def set_default_tags - end - - def set_parameters(arguments, metadata = {}) - end - - def to_s - self.class.to_s - end - end - end -end diff --git a/lib/datadog/ci/span.rb b/lib/datadog/ci/span.rb index aa3cb71c..e09c9b52 100644 --- a/lib/datadog/ci/span.rb +++ b/lib/datadog/ci/span.rb @@ -133,25 +133,6 @@ def set_default_tags tracer_span.set_tag(Ext::Test::TAG_SPAN_KIND, Ext::Test::SPAN_KIND_TEST) end - # Sets the parameters for this span for parametrized tests (e.g. Cucumber examples or RSpec shared specs). - # Parameters are needed to compute test fingerprint to distinguish between different tests having same names. - # @param [Hash] arguments the arguments that test accepts as key-value hash - # @param [Hash] metadata optional metadata - # @return [void] - def set_parameters(arguments, metadata = {}) - return if arguments.nil? - - set_tag( - Ext::Test::TAG_PARAMETERS, - JSON.generate( - { - arguments: arguments, - metadata: metadata - } - ) - ) - end - def to_s "#{self.class}(name:#{name},tracer_span:#{@tracer_span})" end diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index e2017ec5..58edf1cc 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -59,6 +59,26 @@ def test_session_id def source_file get_tag(Ext::Test::TAG_SOURCE_FILE) 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. + # + # @param [Hash] arguments the arguments that test accepts as key-value hash + # @param [Hash] metadata optional metadata + # @return [void] + def set_parameters(arguments, metadata = {}) + return if arguments.nil? + + set_tag( + Ext::Test::TAG_PARAMETERS, + JSON.generate( + { + arguments: arguments, + metadata: metadata + } + ) + ) + end end end end diff --git a/lib/datadog/ci/test_session.rb b/lib/datadog/ci/test_session.rb index c3216f5e..354abf1b 100644 --- a/lib/datadog/ci/test_session.rb +++ b/lib/datadog/ci/test_session.rb @@ -32,7 +32,7 @@ def inheritable_tags return @inheritable_tags if defined?(@inheritable_tags) # this method is not synchronized because it does not iterate over the tags collection, but rather - # uses synchronized method to get each tag value + # uses synchronized method #get_tag to get each tag value res = {} Ext::Test::INHERITABLE_TAGS.each do |tag| res[tag] = get_tag(tag) diff --git a/lib/datadog/ci/test_visibility/null_recorder.rb b/lib/datadog/ci/test_visibility/null_recorder.rb index 35529a38..87e1ebbb 100644 --- a/lib/datadog/ci/test_visibility/null_recorder.rb +++ b/lib/datadog/ci/test_visibility/null_recorder.rb @@ -42,30 +42,10 @@ def active_test_module def active_test_suite(test_suite_name) end - def deactivate_test - end - - def deactivate_test_session - end - - def deactivate_test_module - end - - def deactivate_test_suite(test_suite_name) - end - private def skip_tracing(block = nil) - if block - block.call(null_span) - else - null_span - end - end - - def null_span - @null_span ||= NullSpan.new + block.call(nil) if block end end end diff --git a/lib/datadog/ci/test_visibility/recorder.rb b/lib/datadog/ci/test_visibility/recorder.rb index ca7c87e6..c274590f 100644 --- a/lib/datadog/ci/test_visibility/recorder.rb +++ b/lib/datadog/ci/test_visibility/recorder.rb @@ -15,7 +15,6 @@ require_relative "../utils/git" require_relative "../span" -require_relative "../null_span" require_relative "../test" require_relative "../test_session" require_relative "../test_module" @@ -179,11 +178,7 @@ def deactivate_test_suite(test_suite_name) private def skip_tracing(block = nil) - if block - block.call(null_span) - else - null_span - end + block.call(nil) if block end # Sets trace's origin to ciapp-test @@ -295,10 +290,6 @@ def start_datadog_tracer_span(span_name, span_options, &block) end end - def null_span - @null_span ||= NullSpan.new - end - def validate_test_suite_level_visibility_correctness(test) return unless test_suite_level_visibility_enabled diff --git a/sig/datadog/ci.rbs b/sig/datadog/ci.rbs index 4a2c1b04..34f91bf1 100644 --- a/sig/datadog/ci.rbs +++ b/sig/datadog/ci.rbs @@ -3,15 +3,15 @@ module Datadog class ReservedTypeError < StandardError end - def self.trace_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span test) -> untyped } -> untyped + def self.trace_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test? test) -> untyped } -> untyped - def self.start_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def self.start_test: (String test_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Test? - def self.start_test_session: (?service: String, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def self.start_test_session: (?service: String, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestSession? - def self.start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def self.start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestModule? - def self.start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def self.start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestSuite? def self.trace: (String span_name, ?type: String, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped diff --git a/sig/datadog/ci/null_span.rbs b/sig/datadog/ci/null_span.rbs deleted file mode 100644 index d7a806b5..00000000 --- a/sig/datadog/ci/null_span.rbs +++ /dev/null @@ -1,37 +0,0 @@ -module Datadog - module CI - class NullSpan < Span - def initialize: () -> void - - def id: () -> nil - - def name: () -> nil - - def service: () -> nil - - def type: () -> nil - - def passed!: () -> nil - - def failed!: (?exception: untyped?) -> nil - - def skipped!: (?exception: untyped?, ?reason: untyped?) -> nil - - def get_tag: (untyped key) -> nil - - def set_tag: (untyped key, untyped value) -> nil - - def set_metric: (untyped key, untyped value) -> nil - - def finish: () -> nil - - def set_tags: (untyped tags) -> nil - - def set_environment_runtime_tags: () -> nil - - def set_default_tags: () -> nil - - def to_s: () -> untyped - end - end -end diff --git a/sig/datadog/ci/test_visibility/null_recorder.rbs b/sig/datadog/ci/test_visibility/null_recorder.rbs index 81ef0993..2f30a0c3 100644 --- a/sig/datadog/ci/test_visibility/null_recorder.rbs +++ b/sig/datadog/ci/test_visibility/null_recorder.rbs @@ -2,43 +2,31 @@ module Datadog module CI module TestVisibility class NullRecorder - @null_span: Datadog::CI::NullSpan - def initialize: (?untyped args) -> void - def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped - - def trace: (String type, String span_name, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped - - def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (nil span) -> untyped } -> untyped - def start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def trace: (String type, String span_name, ?tags: Hash[untyped, untyped]) ?{ (nil) -> untyped } -> untyped - def start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> nil - def active_test_session: () -> Datadog::CI::TestSession? + def start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> nil - def active_test_module: () -> Datadog::CI::TestModule? + def start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> nil - def active_test_suite: (String test_suite_name) -> Datadog::CI::TestSuite? + def active_test_session: () -> nil - def active_test: () -> Datadog::CI::Test? + def active_test_module: () -> nil - def active_span: () -> Datadog::CI::Span? + def active_test_suite: (String test_suite_name) -> nil - def deactivate_test: () -> void + def active_test: () -> nil - def deactivate_test_session: () -> void - - def deactivate_test_module: () -> void - - def deactivate_test_suite: (String test_suite_name) -> void + def active_span: () -> nil private - def null_span: () -> Datadog::CI::Span - - def skip_tracing: (?untyped block) -> untyped + def skip_tracing: (?untyped block) -> nil end end end diff --git a/sig/datadog/ci/test_visibility/recorder.rbs b/sig/datadog/ci/test_visibility/recorder.rbs index c5855cae..490f7302 100644 --- a/sig/datadog/ci/test_visibility/recorder.rbs +++ b/sig/datadog/ci/test_visibility/recorder.rbs @@ -9,22 +9,20 @@ module Datadog @global_context: Datadog::CI::TestVisibility::Context::Global @codeowners: Datadog::CI::Codeowners::Matcher - @null_span: Datadog::CI::NullSpan - attr_reader environment_tags: Hash[String, String] attr_reader test_suite_level_visibility_enabled: bool def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher) -> void - def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped + def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test span) -> untyped } -> untyped def trace: (String span_name, ?type: String, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped - def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def start_test_session: (?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestSession - def start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def start_test_module: (String test_module_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestModule - def start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::Span + def start_test_suite: (String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) -> Datadog::CI::TestSuite def active_test_session: () -> Datadog::CI::TestSession? diff --git a/spec/datadog/ci/null_span_spec.rb b/spec/datadog/ci/null_span_spec.rb deleted file mode 100644 index 5cbc80f8..00000000 --- a/spec/datadog/ci/null_span_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -RSpec.describe Datadog::CI::NullSpan do - subject(:span) { described_class.new } - - describe "#name" do - it "returns nil" do - expect(span.name).to be_nil - end - end - - describe "#passed!" do - it "returns nil" do - expect(span.passed!).to be_nil - end - end - - describe "#failed!" do - it "returns nil" do - expect(span.failed!).to be_nil - end - end - - describe "#skipped!" do - it "returns nil" do - expect(span.skipped!).to be_nil - end - end - - describe "#set_tag" do - it "returns nil" do - expect(span.set_tag("foo", "bar")).to be_nil - end - end - - describe "#set_tags" do - it "returns nil" do - expect(span.set_tags("foo" => "bar", "baz" => "qux")).to be_nil - end - end - - describe "#set_metric" do - it "returns nil" do - expect(span.set_metric("foo", "bar")).to be_nil - end - end - - describe "#set_default_tags" do - it "returns nil" do - expect(span.set_default_tags).to be_nil - end - end - - describe "#set_environment_runtime_tags" do - it "returns nil" do - expect(span.set_environment_runtime_tags).to be_nil - end - end - - describe "#finish" do - it "returns nil" do - expect(span.finish).to be_nil - end - end - - describe "#type" do - it "returns nil" do - expect(span.type).to be_nil - end - end -end diff --git a/spec/datadog/ci/span_spec.rb b/spec/datadog/ci/span_spec.rb index 22b9ad5b..362ced9f 100644 --- a/spec/datadog/ci/span_spec.rb +++ b/spec/datadog/ci/span_spec.rb @@ -228,16 +228,4 @@ expect(span.type).to eq("test") end end - - describe "#set_parameters" do - let(:parameters) { {"foo" => "bar", "baz" => "qux"} } - - it "sets the parameters" do - expect(tracer_span).to receive(:set_tag).with( - "test.parameters", JSON.generate({arguments: parameters, metadata: {}}) - ) - - span.set_parameters(parameters) - end - end end diff --git a/spec/datadog/ci/test_spec.rb b/spec/datadog/ci/test_spec.rb index 89139a37..3149126a 100644 --- a/spec/datadog/ci/test_spec.rb +++ b/spec/datadog/ci/test_spec.rb @@ -3,12 +3,12 @@ RSpec.describe Datadog::CI::Test do let(:tracer_span) { instance_double(Datadog::Tracing::SpanOperation, finish: true) } let(:recorder) { spy("recorder") } + subject(:ci_test) { described_class.new(tracer_span) } before { allow_any_instance_of(described_class).to receive(:recorder).and_return(recorder) } describe "#name" do subject(:name) { ci_test.name } - let(:ci_test) { described_class.new(tracer_span) } before { allow(ci_test).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_NAME).and_return("test name") } @@ -16,8 +16,6 @@ end describe "#finish" do - subject(:ci_test) { described_class.new(tracer_span) } - it "deactivates the test" do ci_test.finish expect(recorder).to have_received(:deactivate_test) @@ -26,7 +24,6 @@ describe "#test_suite_id" do subject(:test_suite_id) { ci_test.test_suite_id } - let(:ci_test) { described_class.new(tracer_span) } before do allow(tracer_span).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_TEST_SUITE_ID).and_return("test suite id") @@ -37,7 +34,6 @@ describe "#test_suite_name" do subject(:test_suite_name) { ci_test.test_suite_name } - let(:ci_test) { described_class.new(tracer_span) } before do allow(tracer_span).to( @@ -50,7 +46,6 @@ describe "#test_suite" do subject(:test_suite) { ci_test.test_suite } - let(:ci_test) { described_class.new(tracer_span) } context "when test suite name is set" do before do @@ -70,7 +65,6 @@ describe "#test_module_id" do subject(:test_module_id) { ci_test.test_module_id } - let(:ci_test) { described_class.new(tracer_span) } before do allow(tracer_span).to( @@ -83,7 +77,6 @@ describe "#test_session_id" do subject(:test_session_id) { ci_test.test_session_id } - let(:ci_test) { described_class.new(tracer_span) } before do allow(tracer_span).to( @@ -96,7 +89,6 @@ describe "#source_file" do subject(:source_file) { ci_test.source_file } - let(:ci_test) { described_class.new(tracer_span) } before do allow(tracer_span).to( @@ -106,4 +98,16 @@ it { is_expected.to eq("foo/bar.rb") } end + + describe "#set_parameters" do + let(:parameters) { {"foo" => "bar", "baz" => "qux"} } + + it "sets the parameters" do + expect(tracer_span).to receive(:set_tag).with( + "test.parameters", JSON.generate({arguments: parameters, metadata: {}}) + ) + + ci_test.set_parameters(parameters) + end + end end diff --git a/spec/datadog/ci/test_visibility/null_recorder_spec.rb b/spec/datadog/ci/test_visibility/null_recorder_spec.rb index b013ae60..4e89a59e 100644 --- a/spec/datadog/ci/test_visibility/null_recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/null_recorder_spec.rb @@ -1,34 +1,34 @@ RSpec.describe Datadog::CI::TestVisibility::NullRecorder do let(:recorder) { described_class.new } - describe "#trace_test_session" do + describe "#start_test_session" do subject { recorder.start_test_session } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate session" do expect(recorder.active_test_session).to be_nil end end - describe "#trace_test_module" do + describe "#start_test_module" do let(:module_name) { "my-module" } subject { recorder.start_test_module(module_name) } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate module" do expect(recorder.active_test_module).to be_nil end end - describe "#trace_test_suite" do + describe "#start_test_suite" do let(:suite_name) { "my-module" } subject { recorder.start_test_suite(suite_name) } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate test suite" do expect(recorder.active_test_suite(suite_name)).to be_nil @@ -43,7 +43,7 @@ recorder.trace_test("my test", "my suite") do |test_span| spy_under_test.call - test_span.passed! + test_span.passed! if test_span end end @@ -59,7 +59,7 @@ context "without a block" do subject { recorder.trace_test("my test", "my suite") } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } end end @@ -71,7 +71,7 @@ recorder.trace("step", "my step") do |span| spy_under_test.call - span.set_metric("my.metric", 42) + span.set_metric("my.metric", 42) if span end end @@ -87,7 +87,7 @@ context "without a block" do subject { recorder.trace("step", "my step") } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } end end @@ -113,27 +113,4 @@ it { is_expected.to be_nil } end - - describe "#deactivate_test" do - subject { recorder.deactivate_test } - - it { is_expected.to be_nil } - end - - describe "#deactivate_test_session" do - subject { recorder.deactivate_test_session } - it { is_expected.to be_nil } - end - - describe "#deactivate_test_module" do - subject { recorder.deactivate_test_module } - - it { is_expected.to be_nil } - end - - describe "#deactivate_test_suite" do - subject { recorder.deactivate_test_suite("my suite") } - - it { is_expected.to be_nil } - end end diff --git a/spec/datadog/ci/test_visibility/recorder_spec.rb b/spec/datadog/ci/test_visibility/recorder_spec.rb index c82c7cc4..6695dc6d 100644 --- a/spec/datadog/ci/test_visibility/recorder_spec.rb +++ b/spec/datadog/ci/test_visibility/recorder_spec.rb @@ -55,7 +55,7 @@ describe "#trace_test_session" do subject { recorder.start_test_session(service: service, tags: tags) } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate session" do expect(recorder.active_test_session).to be_nil @@ -67,7 +67,7 @@ subject { recorder.start_test_module(module_name, service: service, tags: tags) } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate module" do expect(recorder.active_test_module).to be_nil @@ -79,7 +79,7 @@ subject { recorder.start_test_suite(suite_name, service: service, tags: tags) } - it { is_expected.to be_kind_of(Datadog::CI::NullSpan) } + it { is_expected.to be_nil } it "does not activate test suite" do expect(recorder.active_test_suite(suite_name)).to be_nil diff --git a/spec/datadog/ci_spec.rb b/spec/datadog/ci_spec.rb index 7c58a296..ba176ab2 100644 --- a/spec/datadog/ci_spec.rb +++ b/spec/datadog/ci_spec.rb @@ -216,7 +216,7 @@ end it "doesn't record spans via Datadog::CI interface" do - expect(spans.count).to eq(1) # http span only + expect(spans).to have(1).item # http span only end end @@ -232,7 +232,7 @@ end it "records test suite level spans" do - expect(spans.count).to eq(5) # session + module + suite + test + http span + expect(spans).to have(5).items # session + module + suite + test + http span expect(test_session_span).not_to be_nil end end @@ -248,7 +248,7 @@ end it "does not record test suite level spans" do - expect(spans.count).to eq(2) # test + http span + expect(spans).to have(2).items # test + http span expect(test_session_span).to be_nil end end diff --git a/spec/support/tracer_helpers.rb b/spec/support/tracer_helpers.rb index 8b236264..b208fd61 100644 --- a/spec/support/tracer_helpers.rb +++ b/spec/support/tracer_helpers.rb @@ -41,7 +41,7 @@ def produce_test_trace( Datadog::CI.active_test.set_tag("test_owner", "my_team") if Datadog::CI.active_test Datadog::CI.active_test.set_metric("memory_allocations", 16) if Datadog::CI.active_test - set_result(test, result: result, exception: exception, skip_reason: skip_reason) + set_result(test, result: result, exception: exception, skip_reason: skip_reason) if test Timecop.travel(start_time + duration_seconds) end @@ -70,7 +70,7 @@ def produce_test_session_trace( test_module = Datadog::CI.start_test_module(test_module_name) - test_suite_span = Datadog::CI.start_test_suite(test_suite) + tracked_test_suite = Datadog::CI.start_test_suite(test_suite) tests_count.times do |num| produce_test_trace( @@ -84,13 +84,20 @@ def produce_test_session_trace( ) end - set_result(test_suite_span, result: result, exception: exception, skip_reason: skip_reason) - set_result(test_module, result: result, exception: exception, skip_reason: skip_reason) - set_result(test_session, result: result, exception: exception, skip_reason: skip_reason) + if tracked_test_suite + set_result(tracked_test_suite, result: result, exception: exception, skip_reason: skip_reason) + tracked_test_suite.finish + end + + if test_module + set_result(test_module, result: result, exception: exception, skip_reason: skip_reason) + test_module.finish + end - test_suite_span.finish - test_module.finish - test_session.finish + if test_session + set_result(test_session, result: result, exception: exception, skip_reason: skip_reason) + test_session.finish + end end def test_session_span From c89c711d4f0f0e736537488b64f98abcd80aa009 Mon Sep 17 00:00:00 2001 From: Andrey Marchenko Date: Thu, 18 Jan 2024 15:44:29 +0100 Subject: [PATCH 10/10] minor docs fix --- lib/datadog/ci.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/datadog/ci.rb b/lib/datadog/ci.rb index c05a5bae..dee62b23 100644 --- a/lib/datadog/ci.rb +++ b/lib/datadog/ci.rb @@ -324,7 +324,7 @@ def trace(span_name, type: "span", tags: {}, &block) # ``` # # @return [Datadog::CI::Span] the active span - # @return [nil] if no span is active, or if the active span is not a custom span with given type + # @return [nil] if no span is active, or if the active span is not a custom span def active_span span = recorder.active_span span if span && !Ext::AppTypes::CI_SPAN_TYPES.include?(span.type)