Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIVIS-9606] reduce ITR-induced code coverage overhead for default branch #157

Merged
merged 2 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions lib/datadog/ci/itr/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

require_relative "../git/local_repository"

require_relative "../utils/parsing"

require_relative "coverage/event"

module Datadog
Expand All @@ -32,13 +34,13 @@ def initialize(
def configure(remote_configuration, test_session)
Datadog.logger.debug("Configuring ITR Runner with remote configuration: #{remote_configuration}")

@enabled = convert_to_bool(
@enabled = Utils::Parsing.convert_to_bool(
remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY, false)
)
@test_skipping_enabled = @enabled && convert_to_bool(
@test_skipping_enabled = @enabled && Utils::Parsing.convert_to_bool(
remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY, false)
)
@code_coverage_enabled = @enabled && convert_to_bool(
@code_coverage_enabled = @enabled && Utils::Parsing.convert_to_bool(
remote_configuration.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY, false)
)

Expand Down Expand Up @@ -121,10 +123,6 @@ def load_datadog_cov!
@code_coverage_enabled = false
end

def convert_to_bool(value)
value.to_s == "true"
end

def ensure_test_source_covered(test_source_file, coverage)
absolute_test_source_file_path = File.join(Git::LocalRepository.root, test_source_file)
return if coverage.key?(absolute_test_source_file_path)
Expand Down
9 changes: 9 additions & 0 deletions lib/datadog/ci/test_visibility/recorder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,15 @@ def configure_library(test_session)
return unless itr_enabled?

remote_configuration = @remote_settings_api.fetch_library_settings(test_session)
# sometimes we can skip code coverage for default branch if there are no changes in the repository
# backend needs git metadata uploaded for this test session to check if we can skip code coverage
if remote_configuration.require_git?
Datadog.logger.debug { "Library configuration endpoint requires git upload to be finished, waiting..." }
@git_tree_upload_worker.wait_until_done

Datadog.logger.debug { "Requesting library configuration again..." }
remote_configuration = @remote_settings_api.fetch_library_settings(test_session)
end
@itr.configure(remote_configuration.payload, test_session)
end

Expand Down
5 changes: 5 additions & 0 deletions lib/datadog/ci/transport/remote_settings_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require "datadog/core/environment/identity"

require_relative "../ext/transport"
require_relative "../utils/parsing"

module Datadog
module CI
Expand Down Expand Up @@ -39,6 +40,10 @@ def payload
end
end

def require_git?
Utils::Parsing.convert_to_bool(payload[Ext::Transport::DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY])
end

private

def default_payload
Expand Down
16 changes: 16 additions & 0 deletions lib/datadog/ci/utils/parsing.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

require "open3"
require "pathname"

module Datadog
module CI
module Utils
module Parsing
def self.convert_to_bool(value)
value.to_s.downcase == "true"
end
end
end
end
end
2 changes: 0 additions & 2 deletions sig/datadog/ci/itr/runner.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ module Datadog

def coverage_collector: () -> Datadog::CI::ITR::Coverage::DDCov?

def convert_to_bool: (untyped value) -> bool

def load_datadog_cov!: () -> void

def write: (Datadog::CI::ITR::Coverage::Event event) -> void
Expand Down
2 changes: 2 additions & 0 deletions sig/datadog/ci/transport/remote_settings_api.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ module Datadog

def payload: () -> Hash[String, untyped]

def require_git?: () -> bool

private

def default_payload: () -> Hash[String, untyped]
Expand Down
9 changes: 9 additions & 0 deletions sig/datadog/ci/utils/parsing.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Datadog
module CI
module Utils
module Parsing
def self.convert_to_bool: (untyped value) -> bool
end
end
end
end
61 changes: 32 additions & 29 deletions spec/datadog/ci/test_visibility/recorder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -733,41 +733,44 @@
end

context "with ITR" do
include_context "CI mode activated" do
let(:itr_enabled) { true }
end
context "without require_git in settings response" do
include_context "CI mode activated" do
let(:itr_enabled) { true }
let(:tests_skipping_enabled) { true }
let(:code_coverage_enabled) { true }
end

describe "#start_test_session" do
let(:service) { "my-service" }
let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} }

subject { recorder.start_test_session(service: service, tags: tags) }

before do
settings_http_response = double(
"http-response",
ok?: true,
payload: {
"data" => {
"attributes" => {
"itr_enabled" => true,
"tests_skipping" => true,
"code_coverage" => true
}
}
}.to_json
)
allow_any_instance_of(Datadog::CI::Transport::RemoteSettingsApi).to receive(:fetch_library_settings).and_return(
Datadog::CI::Transport::RemoteSettingsApi::Response.new(settings_http_response)
)
it "returns a new CI test_session span with ITR tags" do
expect(subject).to be_kind_of(Datadog::CI::TestSession)
expect(subject.service).to eq(service)

expect(subject.skipping_tests?).to be true
expect(subject.code_coverage?).to be true
end
end
end

describe "#start_test_session" do
let(:service) { "my-service" }
let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} }
context "with require_git in settings response" do
include_context "CI mode activated" do
let(:itr_enabled) { true }
let(:tests_skipping_enabled) { true }
let(:code_coverage_enabled) { true }
let(:require_git) { true }
end

subject { recorder.start_test_session(service: service, tags: tags) }
describe "#start_test_session" do
let(:service) { "my-service" }
let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} }

it "returns a new CI test_session span with ITR tags" do
expect(subject).to be_kind_of(Datadog::CI::TestSession)
expect(subject.service).to eq(service)
subject { recorder.start_test_session(service: service, tags: tags) }

expect(subject.skipping_tests?).to be true
expect(subject.code_coverage?).to be true
it { is_expected.not_to be_skipping_tests }
end
end
end
Expand Down
18 changes: 16 additions & 2 deletions spec/datadog/ci/transport/remote_settings_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,21 +73,31 @@
"code_coverage" => true,
"tests_skipping" => false,
"itr_enabled" => true,
"require_git" => false
"require_git" => require_git
}
}
}.to_json
)
end
let(:require_git) { false }

it "parses the response" do
expect(response.ok?).to be true
expect(response.payload).to eq({
"code_coverage" => true,
"tests_skipping" => false,
"itr_enabled" => true,
"require_git" => false
"require_git" => require_git
})
expect(response.require_git?).to be false
end

context "when git is required" do
let(:require_git) { "True" }

it "parses the response" do
expect(response.require_git?).to be true
end
end
end

Expand All @@ -103,6 +113,7 @@
it "parses the response" do
expect(response.ok?).to be false
expect(response.payload).to eq("itr_enabled" => false)
expect(response.require_git?).to be false
end
end

Expand All @@ -122,6 +133,7 @@
it "parses the response" do
expect(response.ok?).to be true
expect(response.payload).to eq("itr_enabled" => false)
expect(response.require_git?).to be false
end
end

Expand All @@ -144,6 +156,7 @@
it "parses the response" do
expect(response.ok?).to be true
expect(response.payload).to eq("itr_enabled" => false)
expect(response.require_git?).to be false
end
end
end
Expand All @@ -154,6 +167,7 @@
it "returns an empty response" do
expect(response.ok?).to be false
expect(response.payload).to eq("itr_enabled" => false)
expect(response.require_git?).to be false
end
end
end
Expand Down
14 changes: 13 additions & 1 deletion spec/support/contexts/ci_mode.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
let(:code_coverage_enabled) { false }
let(:tests_skipping_enabled) { false }
let(:git_metadata_upload_enabled) { false }
let(:require_git) { false }

let(:recorder) { Datadog.send(:components).ci_recorder }

Expand All @@ -40,7 +41,18 @@
"itr_enabled" => itr_enabled,
"code_coverage" => code_coverage_enabled,
"tests_skipping" => tests_skipping_enabled
}
},
require_git?: require_git
),
# This is for the second call to fetch_library_settings
double(
"remote_settings_api_response",
payload: {
"itr_enabled" => itr_enabled,
"code_coverage" => !code_coverage_enabled,
"tests_skipping" => !tests_skipping_enabled
},
require_git?: !require_git
)
)

Expand Down
Loading