From c7117c1510635e71053a7278864ce37afb003f88 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 28 Aug 2024 13:47:47 +0200 Subject: [PATCH 1/5] add additional tests to make sure that telemtry copunts skipped tests correctly --- spec/datadog/ci/test_optimisation/component_spec.rb | 4 ++++ spec/support/contexts/telemetry_spy.rb | 9 +++++++++ 2 files changed, 13 insertions(+) diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index 58fde782..f3ca2744 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -333,6 +333,8 @@ expect { subject } .not_to change { component.skipped_tests_count } end + + it_behaves_like "emits no metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_ITR_SKIPPED end context "test is skipped by ITR" do @@ -363,6 +365,8 @@ expect { subject } .not_to change { component.skipped_tests_count } end + + it_behaves_like "emits no metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_ITR_SKIPPED end end diff --git a/spec/support/contexts/telemetry_spy.rb b/spec/support/contexts/telemetry_spy.rb index 3409c2bf..0991e072 100644 --- a/spec/support/contexts/telemetry_spy.rb +++ b/spec/support/contexts/telemetry_spy.rb @@ -38,6 +38,15 @@ def telemetry_spy_value_suffix(value) end end + shared_examples_for "emits no metric" do |metric_type, metric_name| + it "emits no :#{metric_type} metric #{metric_name}" do + subject + + metric = telemetry_metric(metric_type, metric_name) + expect(metric).to be_nil + end + end + def telemetry_metric(type, name) @metrics[type].find { |m| m.name == name } end From d6841004db64c7a7d759cdcca4bcbc3606eeddf9 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 28 Aug 2024 13:54:52 +0200 Subject: [PATCH 2/5] don't send telemetry event indicating empty coverage for skipped tests --- lib/datadog/ci/test_optimisation/component.rb | 6 ++++-- .../ci/test_optimisation/component_spec.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/datadog/ci/test_optimisation/component.rb b/lib/datadog/ci/test_optimisation/component.rb index cb25ea2c..465fe5c1 100644 --- a/lib/datadog/ci/test_optimisation/component.rb +++ b/lib/datadog/ci/test_optimisation/component.rb @@ -109,13 +109,15 @@ def stop_coverage(test) Telemetry.code_coverage_finished(test) coverage = coverage_collector&.stop + + # if test was skipped, we discard coverage data + return if test.skipped? + if coverage.nil? || coverage.empty? Telemetry.code_coverage_is_empty return end - return if test.skipped? - test_source_file = test.source_file # cucumber's gherkin files are not covered by the code coverage collector diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index f3ca2744..1b946405 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -235,6 +235,24 @@ expect(subject).to be_nil expect(writer).not_to have_received(:write) end + + it_behaves_like "emits no metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_IS_EMPTY + end + + context "when test is skipped and coverage is empty" do + before do + allow_any_instance_of(Datadog::CI::TestOptimisation::Coverage::DDCov).to receive(:stop).and_return(nil) + + component.start_coverage(test_span) + test_span.skipped! + end + + it "does not write coverage event" do + expect(subject).to be_nil + expect(writer).not_to have_received(:write) + end + + it_behaves_like "emits no metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_IS_EMPTY end context "when coverage was not collected" do From a503ec21cf0f5317616e3e64cc799693a6cc9ec8 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 28 Aug 2024 14:07:33 +0200 Subject: [PATCH 3/5] don't collect code coverage for retried tests --- lib/datadog/ci/test.rb | 6 ++++++ lib/datadog/ci/test_optimisation/component.rb | 3 +++ sig/datadog/ci/test.rbs | 1 + .../ci/test_optimisation/component_spec.rb | 13 ++++++++++++ spec/datadog/ci/test_spec.rb | 20 +++++++++++++++++++ 5 files changed, 43 insertions(+) diff --git a/lib/datadog/ci/test.rb b/lib/datadog/ci/test.rb index 513a228d..97744c59 100644 --- a/lib/datadog/ci/test.rb +++ b/lib/datadog/ci/test.rb @@ -70,6 +70,12 @@ def skipped_by_itr? get_tag(Ext::Test::TAG_ITR_SKIPPED_BY_ITR) == "true" end + # Returns "true" if test span represents a retry. + # @return [Boolean] true if this test is a retry, false otherwise. + def is_retry? + get_tag(Ext::Test::TAG_IS_RETRY) == "true" + end + # Marks this test as unskippable by the intelligent test runner. # This must be done before the test execution starts. # diff --git a/lib/datadog/ci/test_optimisation/component.rb b/lib/datadog/ci/test_optimisation/component.rb index 465fe5c1..1920b3e7 100644 --- a/lib/datadog/ci/test_optimisation/component.rb +++ b/lib/datadog/ci/test_optimisation/component.rb @@ -99,12 +99,15 @@ def code_coverage? def start_coverage(test) return if !enabled? || !code_coverage? + return if test.is_retry? + Telemetry.code_coverage_started(test) coverage_collector&.start end def stop_coverage(test) return if !enabled? || !code_coverage? + return if test.is_retry? Telemetry.code_coverage_finished(test) diff --git a/sig/datadog/ci/test.rbs b/sig/datadog/ci/test.rbs index 62f7b534..d264178c 100644 --- a/sig/datadog/ci/test.rbs +++ b/sig/datadog/ci/test.rbs @@ -11,6 +11,7 @@ module Datadog def itr_unskippable!: () -> void def source_file: () -> String? def parameters: () -> String? + def is_retry?: () -> bool private diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index 1b946405..c7d462fe 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -166,6 +166,19 @@ end it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_STARTED, 1 + + context "when test is a retry" do + before do + test_span.set_tag(Datadog::CI::Ext::Test::TAG_IS_RETRY, "true") + end + + it "does not start coverage" do + expect(component).not_to receive(:coverage_collector) + + subject + expect(component.stop_coverage(test_span)).to be_nil + end + end end context "when JRuby and code coverage is enabled" do diff --git a/spec/datadog/ci/test_spec.rb b/spec/datadog/ci/test_spec.rb index 0b5924ee..98c16a70 100644 --- a/spec/datadog/ci/test_spec.rb +++ b/spec/datadog/ci/test_spec.rb @@ -322,4 +322,24 @@ expect(ci_test.parameters).to eq(parameters) end end + + describe "#is_retry?" do + subject(:is_retry) { ci_test.is_retry? } + + context "when tag is set" do + before do + allow(tracer_span).to( + receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_IS_RETRY).and_return("true") + ) + end + + it { is_expected.to be true } + end + + context "when tag is not set" do + before { allow(tracer_span).to receive(:get_tag).with(Datadog::CI::Ext::Test::TAG_IS_RETRY).and_return(nil) } + + it { is_expected.to be false } + end + end end From 8171ad4c19415a4972d0e3f6b2ef82555428f318 Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 28 Aug 2024 14:26:37 +0200 Subject: [PATCH 4/5] fix tests --- spec/datadog/ci/test_optimisation/component_spec.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index c7d462fe..73e9fcb1 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -234,7 +234,7 @@ end it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FINISHED, 1 - it_behaves_like "emits telemetry metric", :distribution, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FILES, 5.0 + it_behaves_like "emits telemetry metric", :distribution, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FILES, 6.0 end context "when test is skipped" do @@ -252,11 +252,8 @@ it_behaves_like "emits no metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_IS_EMPTY end - context "when test is skipped and coverage is empty" do + context "when test is skipped and coverage is not collected" do before do - allow_any_instance_of(Datadog::CI::TestOptimisation::Coverage::DDCov).to receive(:stop).and_return(nil) - - component.start_coverage(test_span) test_span.skipped! end From c82db0df802a1aa89422cec16b5608b40adc9c5f Mon Sep 17 00:00:00 2001 From: Andrey Date: Wed, 28 Aug 2024 14:39:25 +0200 Subject: [PATCH 5/5] do not skip code coverage for retried tests --- lib/datadog/ci/test_optimisation/component.rb | 2 -- .../ci/test_optimisation/component_spec.rb | 15 +-------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/lib/datadog/ci/test_optimisation/component.rb b/lib/datadog/ci/test_optimisation/component.rb index 1920b3e7..729febc2 100644 --- a/lib/datadog/ci/test_optimisation/component.rb +++ b/lib/datadog/ci/test_optimisation/component.rb @@ -99,7 +99,6 @@ def code_coverage? def start_coverage(test) return if !enabled? || !code_coverage? - return if test.is_retry? Telemetry.code_coverage_started(test) coverage_collector&.start @@ -107,7 +106,6 @@ def start_coverage(test) def stop_coverage(test) return if !enabled? || !code_coverage? - return if test.is_retry? Telemetry.code_coverage_finished(test) diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index 73e9fcb1..44dad042 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -166,19 +166,6 @@ end it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_STARTED, 1 - - context "when test is a retry" do - before do - test_span.set_tag(Datadog::CI::Ext::Test::TAG_IS_RETRY, "true") - end - - it "does not start coverage" do - expect(component).not_to receive(:coverage_collector) - - subject - expect(component.stop_coverage(test_span)).to be_nil - end - end end context "when JRuby and code coverage is enabled" do @@ -234,7 +221,7 @@ end it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FINISHED, 1 - it_behaves_like "emits telemetry metric", :distribution, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FILES, 6.0 + it_behaves_like "emits telemetry metric", :distribution, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_FILES, 5.0 end context "when test is skipped" do