diff --git a/lib/datadog/ci/ext/telemetry.rb b/lib/datadog/ci/ext/telemetry.rb index 41c24051..6fa5ccce 100644 --- a/lib/datadog/ci/ext/telemetry.rb +++ b/lib/datadog/ci/ext/telemetry.rb @@ -53,7 +53,6 @@ module Telemetry METRIC_CODE_COVERAGE_FINISHED = "code_coverage_finished" METRIC_CODE_COVERAGE_IS_EMPTY = "code_coverage.is_empty" METRIC_CODE_COVERAGE_FILES = "code_coverage.files" - METRIC_CODE_COVERAGE_ERRORS = "code_coverage.errors" METRIC_TEST_SESSION = "test_session" diff --git a/lib/datadog/ci/test_optimisation/component.rb b/lib/datadog/ci/test_optimisation/component.rb index ec39d93b..0c8923b2 100644 --- a/lib/datadog/ci/test_optimisation/component.rb +++ b/lib/datadog/ci/test_optimisation/component.rb @@ -13,6 +13,7 @@ require_relative "coverage/event" require_relative "skippable" +require_relative "telemetry" module Datadog module CI @@ -104,15 +105,20 @@ def code_coverage? def start_coverage(test) return if !enabled? || !code_coverage? - + Telemetry.code_coverage_started(test) coverage_collector&.start end def stop_coverage(test) return if !enabled? || !code_coverage? + Telemetry.code_coverage_finished(test) + coverage = coverage_collector&.stop - return if coverage.nil? || coverage.empty? + if coverage.nil? || coverage.empty? + Telemetry.code_coverage_is_empty + return + end return if test.skipped? @@ -121,6 +127,8 @@ def stop_coverage(test) # cucumber's gherkin files are not covered by the code coverage collector ensure_test_source_covered(test_source_file, coverage) unless test_source_file.nil? + Telemetry.code_coverage_files(coverage.size) + event = Coverage::Event.new( test_id: test.id.to_s, test_suite_id: test.test_suite_id.to_s, diff --git a/lib/datadog/ci/test_optimisation/telemetry.rb b/lib/datadog/ci/test_optimisation/telemetry.rb index d2be12bf..05ef6dde 100644 --- a/lib/datadog/ci/test_optimisation/telemetry.rb +++ b/lib/datadog/ci/test_optimisation/telemetry.rb @@ -25,10 +25,6 @@ def self.code_coverage_files(count) Utils::Telemetry.distribution(Ext::Telemetry::METRIC_CODE_COVERAGE_FILES, count.to_f) end - def self.code_coverage_errors - Utils::Telemetry.inc(Ext::Telemetry::METRIC_CODE_COVERAGE_ERRORS, 1) - end - def self.tags_for_test(test) { Ext::Telemetry::TAG_TEST_FRAMEWORK => test.get_tag(Ext::Test::TAG_FRAMEWORK), diff --git a/sig/datadog/ci/ext/telemetry.rbs b/sig/datadog/ci/ext/telemetry.rbs index 956a44c5..6a89b389 100644 --- a/sig/datadog/ci/ext/telemetry.rbs +++ b/sig/datadog/ci/ext/telemetry.rbs @@ -80,8 +80,6 @@ module Datadog METRIC_CODE_COVERAGE_FILES: "code_coverage.files" - METRIC_CODE_COVERAGE_ERRORS: "code_coverage.errors" - METRIC_TEST_SESSION: "test_session" TAG_TEST_FRAMEWORK: "test_framework" diff --git a/sig/datadog/ci/test_optimisation/telemetry.rbs b/sig/datadog/ci/test_optimisation/telemetry.rbs index 9dbeb9a4..3263719f 100644 --- a/sig/datadog/ci/test_optimisation/telemetry.rbs +++ b/sig/datadog/ci/test_optimisation/telemetry.rbs @@ -10,8 +10,6 @@ module Datadog def self.code_coverage_files: (Integer count) -> void - def self.code_coverage_errors: () -> void - def self.tags_for_test: (Datadog::CI::Test test) -> ::Hash[String, String] end end diff --git a/spec/datadog/ci/test_optimisation/component_spec.rb b/spec/datadog/ci/test_optimisation/component_spec.rb index 37c820bc..44aa7e62 100644 --- a/spec/datadog/ci/test_optimisation/component_spec.rb +++ b/spec/datadog/ci/test_optimisation/component_spec.rb @@ -3,6 +3,8 @@ require_relative "../../../../lib/datadog/ci/test_optimisation/component" RSpec.describe Datadog::CI::TestOptimisation::Component do + include_context "Telemetry spy" + let(:itr_enabled) { true } let(:api) { double("api") } @@ -12,8 +14,8 @@ let(:tracer_span) { Datadog::Tracing::SpanOperation.new("session") } let(:test_session) { Datadog::CI::TestSession.new(tracer_span) } - subject(:runner) { described_class.new(api: api, dd_env: "dd_env", coverage_writer: writer, enabled: itr_enabled) } - let(:configure) { runner.configure(remote_configuration, test_session: test_session, git_tree_upload_worker: git_worker) } + subject(:component) { described_class.new(api: api, dd_env: "dd_env", coverage_writer: writer, enabled: itr_enabled) } + let(:configure) { component.configure(remote_configuration, test_session: test_session, git_tree_upload_worker: git_worker) } before do allow(writer).to receive(:write) @@ -23,12 +25,12 @@ context "when remote configuration call failed" do let(:remote_configuration) { {"itr_enabled" => false} } - it "configures the runner and test session" do + it "configures the component and test session" do configure - expect(runner.enabled?).to be false - expect(runner.skipping_tests?).to be false - expect(runner.code_coverage?).to be false + expect(component.enabled?).to be false + expect(component.skipping_tests?).to be false + expect(component.code_coverage?).to be false end end @@ -39,10 +41,10 @@ configure end - it "configures the runner" do - expect(runner.enabled?).to be true - expect(runner.skipping_tests?).to be false - expect(runner.code_coverage?).to be(!PlatformHelpers.jruby?) # code coverage is not supported in JRuby + it "configures the component" do + expect(component.enabled?).to be true + expect(component.skipping_tests?).to be false + expect(component.code_coverage?).to be(!PlatformHelpers.jruby?) # code coverage is not supported in JRuby end it "sets test session tags" do @@ -72,12 +74,12 @@ configure end - it "configures the runner" do - expect(runner.enabled?).to be true - expect(runner.skipping_tests?).to be true + it "configures the component" do + expect(component.enabled?).to be true + expect(component.skipping_tests?).to be true - expect(runner.correlation_id).to eq("42") - expect(runner.skippable_tests).to eq(Set.new(["suite.test."])) + expect(component.correlation_id).to eq("42") + expect(component.skippable_tests).to eq(Set.new(["suite.test."])) expect(git_worker).to have_received(:wait_until_done) end @@ -86,29 +88,31 @@ context "when remote configuration call returned correct response with strings instead of bools" do let(:remote_configuration) { {"itr_enabled" => "true", "code_coverage" => "true", "tests_skipping" => "false"} } - it "configures the runner" do + it "configures the component" do configure - expect(runner.enabled?).to be true - expect(runner.skipping_tests?).to be false - expect(runner.code_coverage?).to be(!PlatformHelpers.jruby?) # code coverage is not supported in JRuby + expect(component.enabled?).to be true + expect(component.skipping_tests?).to be false + expect(component.code_coverage?).to be(!PlatformHelpers.jruby?) # code coverage is not supported in JRuby end end context "when remote configuration call returns empty hash" do let(:remote_configuration) { {} } - it "configures the runner" do + it "configures the component" do configure - expect(runner.enabled?).to be false - expect(runner.skipping_tests?).to be false - expect(runner.code_coverage?).to be false + expect(component.enabled?).to be false + expect(component.skipping_tests?).to be false + expect(component.code_coverage?).to be false end end end describe "#start_coverage" do + subject { component.start_coverage(test_span) } + let(:test_tracer_span) { Datadog::Tracing::SpanOperation.new("test") } let(:test_span) { Datadog::CI::Test.new(tracer_span) } @@ -120,10 +124,10 @@ let(:remote_configuration) { {"itr_enabled" => true, "code_coverage" => false, "tests_skipping" => false} } it "does not start coverage" do - expect(runner).not_to receive(:coverage_collector) + expect(component).not_to receive(:coverage_collector) - runner.start_coverage(test_span) - expect(runner.stop_coverage(test_span)).to be_nil + subject + expect(component.stop_coverage(test_span)).to be_nil end end @@ -131,10 +135,10 @@ let(:remote_configuration) { {"itr_enabled" => false, "code_coverage" => false, "tests_skipping" => false} } it "does not start coverage" do - expect(runner).not_to receive(:coverage_collector) + expect(component).not_to receive(:coverage_collector) - runner.start_coverage(test_span) - expect(runner.stop_coverage(test_span)).to be_nil + subject + expect(component.stop_coverage(test_span)).to be_nil end end @@ -146,13 +150,15 @@ end it "starts coverage" do - expect(runner).to receive(:coverage_collector).twice.and_call_original + expect(component).to receive(:coverage_collector).twice.and_call_original - runner.start_coverage(test_span) + subject expect(1 + 1).to eq(2) - coverage_event = runner.stop_coverage(test_span) + coverage_event = component.stop_coverage(test_span) expect(coverage_event.coverage.size).to be > 0 end + + it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_STARTED, 1 end context "when JRuby and code coverage is enabled" do @@ -163,16 +169,18 @@ end it "disables code coverage" do - expect(runner).not_to receive(:coverage_collector) - expect(runner.code_coverage?).to be(false) + expect(component).not_to receive(:coverage_collector) + expect(component.code_coverage?).to be(false) - runner.start_coverage(test_span) - expect(runner.stop_coverage(test_span)).to be_nil + component.start_coverage(test_span) + expect(component.stop_coverage(test_span)).to be_nil end end end describe "#stop_coverage" do + subject { component.stop_coverage(test_span) } + let(:test_tracer_span) { Datadog::Tracing::SpanOperation.new("test") } let(:test_span) { Datadog::CI::Test.new(tracer_span) } let(:remote_configuration) { {"itr_enabled" => true, "code_coverage" => true, "tests_skipping" => false} } @@ -187,27 +195,37 @@ allow(test_span).to receive(:test_session_id).and_return(3) end - it "creates coverage event and writes it" do - runner.start_coverage(test_span) - expect(1 + 1).to eq(2) - expect(runner.stop_coverage(test_span)).not_to be_nil + context "when coverage was collected" do + before do + component.start_coverage(test_span) + expect(1 + 1).to eq(2) + end + + it "creates coverage event and writes it" do + expect(subject).not_to be_nil - expect(writer).to have_received(:write) do |event| - expect(event.test_id).to eq("1") - expect(event.test_suite_id).to eq("2") - expect(event.test_session_id).to eq("3") + expect(writer).to have_received(:write) do |event| + expect(event.test_id).to eq("1") + expect(event.test_suite_id).to eq("2") + expect(event.test_session_id).to eq("3") - expect(event.coverage.size).to be > 0 + expect(event.coverage.size).to be > 0 + end 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 end context "when test is skipped" do - it "does not write coverage event" do - runner.start_coverage(test_span) + before do + component.start_coverage(test_span) expect(1 + 1).to eq(2) test_span.skipped! + end - expect(runner.stop_coverage(test_span)).to be_nil + it "does not write coverage event" do + expect(subject).to be_nil expect(writer).not_to have_received(:write) end end @@ -216,14 +234,16 @@ it "does not write coverage event" do expect(1 + 1).to eq(2) - expect(runner.stop_coverage(test_span)).to be_nil + expect(subject).to be_nil expect(writer).not_to have_received(:write) end + + it_behaves_like "emits telemetry metric", :inc, Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_IS_EMPTY, 1 end end describe "#mark_if_skippable" do - subject { runner.mark_if_skippable(test_span) } + subject { component.mark_if_skippable(test_span) } context "when skipping tests" do let(:remote_configuration) { {"itr_enabled" => true, "code_coverage" => true, "tests_skipping" => true} } @@ -294,7 +314,7 @@ end describe "#count_skipped_test" do - subject { runner.count_skipped_test(test_span) } + subject { component.count_skipped_test(test_span) } context "test is skipped by framework" do let(:test_span) do @@ -305,7 +325,7 @@ it "does not increment skipped tests count" do expect { subject } - .not_to change { runner.skipped_tests_count } + .not_to change { component.skipped_tests_count } end end @@ -318,7 +338,7 @@ it "increments skipped tests count" do expect { subject } - .to change { runner.skipped_tests_count } + .to change { component.skipped_tests_count } .from(0) .to(1) end @@ -333,7 +353,7 @@ it "does not increment skipped tests count" do expect { subject } - .not_to change { runner.skipped_tests_count } + .not_to change { component.skipped_tests_count } end end end @@ -346,10 +366,10 @@ end before do - runner.count_skipped_test(test_span) + component.count_skipped_test(test_span) end - subject { runner.write_test_session_tags(test_session_span) } + subject { component.write_test_session_tags(test_session_span) } let(:test_span) do Datadog::CI::Test.new( diff --git a/spec/datadog/ci/test_optimisation/telemetry_spec.rb b/spec/datadog/ci/test_optimisation/telemetry_spec.rb index 0077cf5e..5a76cb5d 100644 --- a/spec/datadog/ci/test_optimisation/telemetry_spec.rb +++ b/spec/datadog/ci/test_optimisation/telemetry_spec.rb @@ -82,15 +82,4 @@ it { code_coverage_files } end - - describe ".code_coverage_errors" do - subject(:code_coverage_errors) { described_class.code_coverage_errors } - - before do - expect(Datadog::CI::Utils::Telemetry).to receive(:inc) - .with(Datadog::CI::Ext::Telemetry::METRIC_CODE_COVERAGE_ERRORS, 1) - end - - it { code_coverage_errors } - end end