From 228bae60c2d7c5d125676b2b9df4fbaeae0f2739 Mon Sep 17 00:00:00 2001 From: Andrey Date: Thu, 29 Aug 2024 14:47:18 +0200 Subject: [PATCH] parse early_flake_detection_enabled? from remote library settings --- lib/datadog/ci/ext/telemetry.rb | 1 + lib/datadog/ci/ext/transport.rb | 1 + lib/datadog/ci/git/local_repository.rb | 4 ++ lib/datadog/ci/remote/library_settings.rb | 35 ++++++++++--- .../ci/remote/library_settings_client.rb | 3 +- lib/datadog/ci/test_retries/component.rb | 6 ++- sig/datadog/ci/ext/telemetry.rbs | 2 + sig/datadog/ci/ext/transport.rbs | 2 + sig/datadog/ci/remote/library_settings.rbs | 5 +- sig/datadog/ci/test_retries/component.rbs | 2 + spec/datadog/ci/git/packfiles_spec.rb | 3 ++ .../ci/remote/library_settings_client_spec.rb | 41 +++++++++------ .../datadog/ci/test_retries/component_spec.rb | 52 +++++++++++++------ spec/support/contexts/ci_mode.rb | 7 ++- 14 files changed, 118 insertions(+), 46 deletions(-) diff --git a/lib/datadog/ci/ext/telemetry.rb b/lib/datadog/ci/ext/telemetry.rb index ed6c338ca..959980e77 100644 --- a/lib/datadog/ci/ext/telemetry.rb +++ b/lib/datadog/ci/ext/telemetry.rb @@ -73,6 +73,7 @@ module Telemetry TAG_COMMAND = "command" TAG_COVERAGE_ENABLED = "coverage_enabled" TAG_ITR_SKIP_ENABLED = "itrskip_enabled" + TAG_EARLY_FLAKE_DETECTION_ENABLED = "early_flake_detection_enabled" TAG_PROVIDER = "provider" TAG_AUTO_INJECTED = "auto_injected" diff --git a/lib/datadog/ci/ext/transport.rb b/lib/datadog/ci/ext/transport.rb index 57fa7e0a5..25adb1eb6 100644 --- a/lib/datadog/ci/ext/transport.rb +++ b/lib/datadog/ci/ext/transport.rb @@ -37,6 +37,7 @@ module Transport DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY = "tests_skipping" DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY = "require_git" DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY = "flaky_test_retries_enabled" + DD_API_SETTINGS_RESPONSE_EARLY_FLAKE_DETECTION_KEY = "early_flake_detection" DD_API_SETTINGS_RESPONSE_DEFAULT = {DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY => false}.freeze DD_API_GIT_SEARCH_COMMITS_PATH = "/api/v2/git/repository/search_commits" diff --git a/lib/datadog/ci/git/local_repository.rb b/lib/datadog/ci/git/local_repository.rb index 8c2f1a6e7..b661ba818 100644 --- a/lib/datadog/ci/git/local_repository.rb +++ b/lib/datadog/ci/git/local_repository.rb @@ -147,12 +147,15 @@ def self.git_commit_users # returns maximum of 1000 latest commits in the last month def self.git_commits + p "COMMITS!!!!" Telemetry.git_command(Ext::Telemetry::Command::GET_LOCAL_COMMITS) output = nil duration_ms = Core::Utils::Time.measure(:float_millisecond) do output = exec_git_command("git log --format=%H -n 1000 --since=\"1 month ago\"") end + p "OUTPUT" + p output Telemetry.git_command_ms(Ext::Telemetry::Command::GET_LOCAL_COMMITS, duration_ms) @@ -161,6 +164,7 @@ def self.git_commits # @type var output: String output.split("\n") rescue => e + p e log_failure(e, "git commits") telemetry_track_error(e, Ext::Telemetry::Command::GET_LOCAL_COMMITS) [] diff --git a/lib/datadog/ci/remote/library_settings.rb b/lib/datadog/ci/remote/library_settings.rb index 63984e204..a1a853867 100644 --- a/lib/datadog/ci/remote/library_settings.rb +++ b/lib/datadog/ci/remote/library_settings.rb @@ -49,39 +49,58 @@ def payload def require_git? return @require_git if defined?(@require_git) - @require_git = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY) + @require_git = Utils::Parsing.convert_to_bool( + payload.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_REQUIRE_GIT_KEY, false) + ) end def itr_enabled? return @itr_enabled if defined?(@itr_enabled) - @itr_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY) + @itr_enabled = Utils::Parsing.convert_to_bool( + payload.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_ITR_ENABLED_KEY, false) + ) end def code_coverage_enabled? return @code_coverage_enabled if defined?(@code_coverage_enabled) - @code_coverage_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY) + @code_coverage_enabled = Utils::Parsing.convert_to_bool( + payload.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_CODE_COVERAGE_KEY, false) + ) end def tests_skipping_enabled? return @tests_skipping_enabled if defined?(@tests_skipping_enabled) - @tests_skipping_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY) + @tests_skipping_enabled = Utils::Parsing.convert_to_bool( + payload.fetch(Ext::Transport::DD_API_SETTINGS_RESPONSE_TESTS_SKIPPING_KEY, false) + ) end def flaky_test_retries_enabled? return @flaky_test_retries_enabled if defined?(@flaky_test_retries_enabled) - @flaky_test_retries_enabled = bool(Ext::Transport::DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY) + @flaky_test_retries_enabled = Utils::Parsing.convert_to_bool( + payload.fetch( + Ext::Transport::DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY, false + ) + ) end - private + def early_flake_detection_enabled? + return @early_flake_detection_enabled if defined?(@early_flake_detection_enabled) - def bool(key) - Utils::Parsing.convert_to_bool(payload.fetch(key, false)) + @early_flake_detection_enabled = Utils::Parsing.convert_to_bool( + payload.fetch( + Ext::Transport::DD_API_SETTINGS_RESPONSE_EARLY_FLAKE_DETECTION_KEY, + {} + ).fetch("enabled", false) + ) end + private + def default_payload Ext::Transport::DD_API_SETTINGS_RESPONSE_DEFAULT end diff --git a/lib/datadog/ci/remote/library_settings_client.rb b/lib/datadog/ci/remote/library_settings_client.rb index 43c701315..9b50c0e6e 100644 --- a/lib/datadog/ci/remote/library_settings_client.rb +++ b/lib/datadog/ci/remote/library_settings_client.rb @@ -58,7 +58,8 @@ def fetch(test_session) 1, { Ext::Telemetry::TAG_COVERAGE_ENABLED => library_settings.code_coverage_enabled?.to_s, - Ext::Telemetry::TAG_ITR_SKIP_ENABLED => library_settings.tests_skipping_enabled?.to_s + Ext::Telemetry::TAG_ITR_SKIP_ENABLED => library_settings.tests_skipping_enabled?.to_s, + Ext::Telemetry::TAG_EARLY_FLAKE_DETECTION_ENABLED => library_settings.early_flake_detection_enabled?.to_s } ) diff --git a/lib/datadog/ci/test_retries/component.rb b/lib/datadog/ci/test_retries/component.rb index b80b6b0b5..3efd9521a 100644 --- a/lib/datadog/ci/test_retries/component.rb +++ b/lib/datadog/ci/test_retries/component.rb @@ -11,7 +11,8 @@ module TestRetries # - retrying new tests - detect flaky tests as early as possible to prevent them from being merged class Component attr_reader :retry_failed_tests_enabled, :retry_failed_tests_max_attempts, - :retry_failed_tests_total_limit, :retry_failed_tests_count + :retry_failed_tests_total_limit, :retry_failed_tests_count, + :retry_new_tests_enabled def initialize( retry_failed_tests_enabled:, @@ -24,11 +25,14 @@ def initialize( # counter that store the current number of failed tests retried @retry_failed_tests_count = 0 + @retry_new_tests_enabled = true + @mutex = Mutex.new end def configure(library_settings) @retry_failed_tests_enabled &&= library_settings.flaky_test_retries_enabled? + @retry_new_tests_enabled &&= library_settings.early_flake_detection_enabled? end def with_retries(&block) diff --git a/sig/datadog/ci/ext/telemetry.rbs b/sig/datadog/ci/ext/telemetry.rbs index 34c4141bf..33e892268 100644 --- a/sig/datadog/ci/ext/telemetry.rbs +++ b/sig/datadog/ci/ext/telemetry.rbs @@ -116,6 +116,8 @@ module Datadog TAG_ITR_SKIP_ENABLED: "itrskip_enabled" + TAG_EARLY_FLAKE_DETECTION_ENABLED: "early_flake_detection_enabled" + TAG_PROVIDER: "provider" TAG_AUTO_INJECTED: "auto_injected" diff --git a/sig/datadog/ci/ext/transport.rbs b/sig/datadog/ci/ext/transport.rbs index e701817e8..88625092b 100644 --- a/sig/datadog/ci/ext/transport.rbs +++ b/sig/datadog/ci/ext/transport.rbs @@ -50,6 +50,8 @@ module Datadog DD_API_SETTINGS_RESPONSE_FLAKY_TEST_RETRIES_KEY: "flaky_test_retries_enabled" + DD_API_SETTINGS_RESPONSE_EARLY_FLAKE_DETECTION_KEY: "early_flake_detection" + DD_API_SETTINGS_RESPONSE_DEFAULT: Hash[String, untyped] DD_API_GIT_SEARCH_COMMITS_PATH: "/api/v2/git/repository/search_commits" diff --git a/sig/datadog/ci/remote/library_settings.rbs b/sig/datadog/ci/remote/library_settings.rbs index 68791562a..e4b2b555b 100644 --- a/sig/datadog/ci/remote/library_settings.rbs +++ b/sig/datadog/ci/remote/library_settings.rbs @@ -10,6 +10,7 @@ module Datadog @code_coverage_enabled: bool @tests_skipping_enabled: bool @flaky_test_retries_enabled: bool + @early_flake_detection_enabled: bool def initialize: (Datadog::CI::Transport::Adapters::Net::Response? http_response) -> void @@ -27,9 +28,9 @@ module Datadog def flaky_test_retries_enabled?: () -> bool - private + def early_flake_detection_enabled?: () -> bool - def bool: (String key) -> bool + private def default_payload: () -> Hash[String, untyped] end diff --git a/sig/datadog/ci/test_retries/component.rbs b/sig/datadog/ci/test_retries/component.rbs index 1144687b5..604f89ac6 100644 --- a/sig/datadog/ci/test_retries/component.rbs +++ b/sig/datadog/ci/test_retries/component.rbs @@ -10,6 +10,8 @@ module Datadog attr_reader retry_failed_tests_count: Integer + attr_reader retry_new_tests_enabled: bool + @mutex: Thread::Mutex def initialize: (retry_failed_tests_enabled: bool, retry_failed_tests_max_attempts: Integer, retry_failed_tests_total_limit: Integer) -> void diff --git a/spec/datadog/ci/git/packfiles_spec.rb b/spec/datadog/ci/git/packfiles_spec.rb index 07842e5ac..b30c5949e 100644 --- a/spec/datadog/ci/git/packfiles_spec.rb +++ b/spec/datadog/ci/git/packfiles_spec.rb @@ -12,6 +12,9 @@ describe ".generate" do it "yields packfile" do + p Datadog::CI::Git::LocalRepository.git_commits + p included_commits + expect do |b| described_class.generate(included_commits: included_commits, excluded_commits: excluded_commits, &b) end.to yield_with_args(/\/.+\h{8}-\h{40}\.pack$/) diff --git a/spec/datadog/ci/remote/library_settings_client_spec.rb b/spec/datadog/ci/remote/library_settings_client_spec.rb index 31c01a584..1fc4ba6c5 100644 --- a/spec/datadog/ci/remote/library_settings_client_spec.rb +++ b/spec/datadog/ci/remote/library_settings_client_spec.rb @@ -70,6 +70,26 @@ end context "when response is OK" do + let(:attributes) do + { + "code_coverage" => "1", + "tests_skipping" => "false", + "itr_enabled" => "True", + "require_git" => require_git, + "flaky_test_retries_enabled" => "true", + "early_flake_detection" => { + "enabled" => "true", + "slow_test_retries" => { + "5s" => 10, + "10s" => 5, + "30s" => 3, + "5m" => 2 + }, + "faulty_session_threshold" => 30 + } + } + end + let(:http_response) do double( "http_response", @@ -78,13 +98,7 @@ "data" => { "id" => "123", "type" => Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE, - "attributes" => { - "code_coverage" => "1", - "tests_skipping" => "false", - "itr_enabled" => "True", - "require_git" => require_git, - "flaky_test_retries_enabled" => "true" - } + "attributes" => attributes } }.to_json, request_compressed: false, @@ -95,21 +109,18 @@ it "parses the response" do expect(response.ok?).to be true - expect(response.payload).to eq({ - "code_coverage" => "1", - "tests_skipping" => "false", - "itr_enabled" => "True", - "require_git" => require_git, - "flaky_test_retries_enabled" => "true" - }) + expect(response.payload).to eq(attributes) expect(response.require_git?).to be false expect(response.itr_enabled?).to be true expect(response.code_coverage_enabled?).to be true expect(response.tests_skipping_enabled?).to be false expect(response.flaky_test_retries_enabled?).to be true + expect(response.early_flake_detection_enabled?).to be true metric = telemetry_metric(:inc, "git_requests.settings_response") - expect(metric.tags).to eq("coverage_enabled" => "true", "itrskip_enabled" => "false") + expect(metric.tags).to eq( + "coverage_enabled" => "true", "itrskip_enabled" => "false", "early_flake_detection_enabled" => "true" + ) end it_behaves_like "emits telemetry metric", :inc, "git_requests.settings", 1 diff --git a/spec/datadog/ci/test_retries/component_spec.rb b/spec/datadog/ci/test_retries/component_spec.rb index 27e6407da..a5a5da14e 100644 --- a/spec/datadog/ci/test_retries/component_spec.rb +++ b/spec/datadog/ci/test_retries/component_spec.rb @@ -1,12 +1,21 @@ require_relative "../../../../lib/datadog/ci/test_retries/component" RSpec.describe Datadog::CI::TestRetries::Component do - let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings) } + let(:library_settings) do + instance_double( + Datadog::CI::Remote::LibrarySettings, + flaky_test_retries_enabled?: remote_flaky_test_retries_enabled, + early_flake_detection_enabled?: remote_early_flake_detection_enabled + ) + end let(:retry_failed_tests_enabled) { true } let(:retry_failed_tests_max_attempts) { 1 } let(:retry_failed_tests_total_limit) { 12 } + let(:remote_flaky_test_retries_enabled) { false } + let(:remote_early_flake_detection_enabled) { false } + subject(:component) do described_class.new( retry_failed_tests_enabled: retry_failed_tests_enabled, @@ -19,9 +28,7 @@ subject { component.configure(library_settings) } context "when flaky test retries are enabled" do - before do - allow(library_settings).to receive(:flaky_test_retries_enabled?).and_return(true) - end + let(:remote_flaky_test_retries_enabled) { true } it "enables retrying failed tests" do subject @@ -31,9 +38,7 @@ end context "when flaky test retries are disabled" do - before do - allow(library_settings).to receive(:flaky_test_retries_enabled?).and_return(false) - end + let(:remote_flaky_test_retries_enabled) { false } it "disables retrying failed tests" do subject @@ -44,10 +49,7 @@ context "when flaky test retries are disabled in local settings" do let(:retry_failed_tests_enabled) { false } - - before do - allow(library_settings).to receive(:flaky_test_retries_enabled?).and_return(true) - end + let(:remote_flaky_test_retries_enabled) { true } it "disables retrying failed tests even if it's enabled remotely" do subject @@ -55,6 +57,26 @@ expect(component.retry_failed_tests_enabled).to be false end end + + context "when early flake detecion is enabled" do + let(:remote_early_flake_detection_enabled) { true } + + it "enables retrying failed tests" do + subject + + expect(component.retry_new_tests_enabled).to be true + end + end + + context "when early flake detecion is disabled" do + let(:remote_early_flake_detection_enabled) { false } + + it "disables retrying failed tests" do + subject + + expect(component.retry_new_tests_enabled).to be false + end + end end describe "#retry_failed_tests_max_attempts" do @@ -80,7 +102,7 @@ end context "when retry failed tests is enabled" do - let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: true) } + let(:remote_flaky_test_retries_enabled) { true } context "when test span is failed" do let(:test_failed) { true } @@ -130,8 +152,6 @@ end context "when retry failed tests is disabled" do - let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: false) } - it { is_expected.to be_a(Datadog::CI::TestRetries::Strategy::NoRetry) } end end @@ -171,13 +191,11 @@ end context "when no retries strategy is used" do - let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: false) } - it { is_expected.to eq(1) } end context "when retried failed tests strategy is used" do - let(:library_settings) { instance_double(Datadog::CI::Remote::LibrarySettings, flaky_test_retries_enabled?: true) } + let(:remote_flaky_test_retries_enabled) { true } context "when test span is failed" do let(:test_failed) { true } diff --git a/spec/support/contexts/ci_mode.rb b/spec/support/contexts/ci_mode.rb index 2354c53c9..33ec12e34 100644 --- a/spec/support/contexts/ci_mode.rb +++ b/spec/support/contexts/ci_mode.rb @@ -27,6 +27,7 @@ let(:bundle_path) { nil } let(:use_single_threaded_coverage) { false } let(:flaky_test_retries_enabled) { false } + let(:early_flake_detection_enabled) { false } let(:retry_failed_tests_max_attempts) { 5 } let(:retry_failed_tests_total_limit) { 100 } @@ -66,7 +67,8 @@ itr_enabled?: itr_enabled, code_coverage_enabled?: code_coverage_enabled, tests_skipping_enabled?: tests_skipping_enabled, - flaky_test_retries_enabled?: flaky_test_retries_enabled + flaky_test_retries_enabled?: flaky_test_retries_enabled, + early_flake_detection_enabled?: early_flake_detection_enabled ), # This is for the second call to fetch_library_settings instance_double( @@ -80,7 +82,8 @@ itr_enabled?: itr_enabled, code_coverage_enabled?: !code_coverage_enabled, tests_skipping_enabled?: !tests_skipping_enabled, - flaky_test_retries_enabled?: flaky_test_retries_enabled + flaky_test_retries_enabled?: flaky_test_retries_enabled, + early_flake_detection_enabled?: early_flake_detection_enabled ) ) allow_any_instance_of(Datadog::CI::TestOptimisation::Skippable).to receive(:fetch_skippable_tests).and_return(skippable_tests_response)