decouple API client that requests library settings from ITR

lib/datadog/ci/configuration/components.rb

@@ -71,14 +71,16 @@ def activate_ci!(settings)
           settings.tracing.test_mode.writer_options = writer_options
 
           itr = Datadog::CI::ITR::Runner.new(
-            enabled: settings.ci.enabled && settings.ci.itr_enabled,
-            api: test_visibility_api
+            enabled: settings.ci.enabled && settings.ci.itr_enabled
           )
 
+          api_client = Transport::ApiClient.new(api: test_visibility_api)
+
           # CI visibility recorder global instance
           @ci_recorder = TestVisibility::Recorder.new(
             test_suite_level_visibility_enabled: !settings.ci.force_test_level_visibility,
-            itr: itr
+            itr: itr,
+            api_client: api_client
           )
         end
 

lib/datadog/ci/itr/runner.rb

@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require_relative "client"
-
 module Datadog
   module CI
     module ITR
@@ -10,25 +8,17 @@ module ITR
       # skip tests that are not impacted by the changes
       class Runner
         def initialize(
-          enabled: false,
-          api: nil
+          enabled: false
         )
           @enabled = enabled
-          return unless enabled
-
-          @client = Client.new(api: api)
         end
 
         def enabled?
           @enabled
         end
 
-        def configure(service:)
-          return unless enabled?
-
-          # TODO: error handling when request failed
-          # TODO: need to pass runtime information
-          @client.fetch_settings(service: service)
+        def disable
+          @enabled = false
         end
       end
     end

lib/datadog/ci/test_visibility/recorder.rb

@@ -12,6 +12,7 @@
 require_relative "../ext/app_types"
 require_relative "../ext/test"
 require_relative "../ext/environment"
+require_relative "../transport/api_client"
 require_relative "../utils/git"
 
 require_relative "../span"
@@ -29,9 +30,8 @@ class Recorder
         attr_reader :environment_tags, :test_suite_level_visibility_enabled
 
         def initialize(
-          test_suite_level_visibility_enabled: false,
-          codeowners: Codeowners::Parser.new(Utils::Git.root).parse,
-          itr: nil
+          itr:, api_client:, test_suite_level_visibility_enabled: false,
+          codeowners: Codeowners::Parser.new(Utils::Git.root).parse
         )
           @test_suite_level_visibility_enabled = test_suite_level_visibility_enabled
 
@@ -41,24 +41,24 @@ def initialize(
 
           @codeowners = codeowners
 
-          raise ArgumentError, "ITR runner is required" unless itr
           @itr = itr
+          @api_client = api_client
         end
 
         def start_test_session(service: nil, tags: {})
           return skip_tracing unless test_suite_level_visibility_enabled
 
-          # TODO: pass runtime and environment information
-          # actually this is the correct place to configure ITR - when starting test session
-          @itr.configure(service: service)
-
           @global_context.fetch_or_activate_test_session do
             tracer_span = start_datadog_tracer_span(
               "test.session", build_span_options(service, Ext::AppTypes::TYPE_TEST_SESSION)
             )
             set_session_context(tags, tracer_span)
 
-            build_test_session(tracer_span, tags)
+            test_session = build_test_session(tracer_span, tags)
+
+            configure_library(service: service)
+
+            test_session
           end
         end
 
@@ -190,6 +190,14 @@ def itr_enabled?
 
         private
 
+        def configure_library(service:)
+          return unless itr_enabled?
+          # TODO: error handling when request failed - disable ITR runner
+          # TODO: need to pass runtime information
+          # TODO: disable ITR runner if itr_enabled is false in settings
+          @api_client.fetch_library_settings(service: service)
+        end
+
         def skip_tracing(block = nil)
           block.call(nil) if block
         end

lib/datadog/ci/itr/client.rb → lib/datadog/ci/transport/api_client.rb

@@ -1,28 +1,26 @@
 # frozen_string_literal: true
 
-require_relative "../ext/itr"
+require_relative "../ext/transport"
 
 module Datadog
   module CI
-    module ITR
-      # ITR API client
-      # communicates with the CI visibility backend
-      class Client
+    module Transport
+      # Datadog API client
+      # Calls settings endpoint to fetch library settings for given service and env
+      class ApiClient
         def initialize(api: nil)
-          raise ArgumentError, "Test visibility API is required" unless api
-
           @api = api
         end
 
-        def fetch_settings(service:)
-          # TODO: application/json support
+        def fetch_library_settings(service:)
+          # TODO: return error response if api is not present
+          return {} unless @api
           # TODO: id generation
           # TODO: runtime information is required for payload
           # TODO: return error response - use some wrapper from ddtrace as an example
-          @api.request(
-            path: Ext::ITR::API_PATH_SETTINGS,
-            payload: settings_payload(service: service),
-            headers: {Ext::Transport::HEADER_CONTENT_TYPE => Ext::Transport::CONTENT_TYPE_JSON}
+          @api.api_request(
+            path: Ext::Transport::DD_API_SETTINGS_PATH,
+            payload: settings_payload(service: service)
           )
         end
 
@@ -32,7 +30,7 @@ def settings_payload(service:)
           {
             data: {
               id: "change_me",
-              type: Ext::ITR::API_TYPE_SETTINGS,
+              type: Ext::Transport::DD_API_SETTINGS_TYPE,
               attributes: {
                 service: service
               }

sig/datadog/ci/itr/runner.rbs

@@ -3,13 +3,12 @@ module Datadog
     module ITR
       class Runner
         @enabled: bool
-        @client: Datadog::CI::ITR::Client
 
-        def initialize: (?enabled: bool, api: Datadog::CI::Transport::Api::Base?) -> void
+        def initialize: (?enabled: bool) -> void
 
         def enabled?: () -> bool
 
-        def configure: (service: String?) -> void
+        def disable: () -> void
       end
     end
   end

sig/datadog/ci/test_visibility/recorder.rbs

@@ -8,12 +8,13 @@ module Datadog
         @local_context: Datadog::CI::TestVisibility::Context::Local
         @global_context: Datadog::CI::TestVisibility::Context::Global
         @itr: Datadog::CI::ITR::Runner
+        @api_client: Datadog::CI::Transport::ApiClient
         @codeowners: Datadog::CI::Codeowners::Matcher
 
         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, ?itr: Datadog::CI::ITR::Runner?) -> void
+        def initialize: (?test_suite_level_visibility_enabled: bool, ?codeowners: Datadog::CI::Codeowners::Matcher, itr: Datadog::CI::ITR::Runner, api_client: Datadog::CI::Transport::ApiClient) -> void
 
         def trace_test: (String span_name, String test_suite_name, ?service: String?, ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Test span) -> untyped } -> untyped
 
@@ -47,6 +48,8 @@ module Datadog
 
         private
 
+        def configure_library: (service: String?) -> void
+
         def create_datadog_span: (String span_name, ?span_options: Hash[untyped, untyped], ?tags: Hash[untyped, untyped]) ?{ (Datadog::CI::Span span) -> untyped } -> untyped
 
         def set_trace_origin: (Datadog::Tracing::TraceOperation trace) -> untyped

sig/datadog/ci/itr/client.rbs → sig/datadog/ci/transport/api_client.rbs

@@ -1,12 +1,12 @@
 module Datadog
   module CI
-    module ITR
-      class Client
+    module Transport
+      class ApiClient
         @api: untyped
 
         def initialize: (?api: Datadog::CI::Transport::Api::Base?) -> void
 
-        def fetch_settings: (service: String?) -> untyped
+        def fetch_library_settings: (service: String?) -> untyped
 
         private
 

spec/datadog/ci/itr/runner_spec.rb

@@ -1,41 +1,15 @@
 # frozen_string_literal: true
 
-require_relative "../../../../lib/datadog/ci/itr/client"
 require_relative "../../../../lib/datadog/ci/itr/runner"
 
 RSpec.describe Datadog::CI::ITR::Runner do
   let(:itr_enabled) { true }
 
-  let(:api) { double("api") }
-  let(:client) { double("client") }
+  subject(:runner) { described_class.new(enabled: itr_enabled) }
 
-  subject(:runner) { described_class.new(enabled: itr_enabled, api: api) }
-
-  describe "#configure" do
-    let(:service) { "service" }
-
-    context "itr enabled" do
-      before do
-        expect(Datadog::CI::ITR::Client).to receive(:new).with(api: api).and_return(client)
-      end
-
-      it "fetches settings from backend" do
-        expect(client).to receive(:fetch_settings).with(service: service)
-
-        runner.configure(service: service)
-      end
-    end
-
-    context "itr disabled" do
-      let(:itr_enabled) { false }
-
-      before do
-        expect(Datadog::CI::ITR::Client).to_not receive(:new)
-      end
-
-      it "does nothing" do
-        expect(runner.configure(service: service)).to be_nil
-      end
+  describe "#disable" do
+    it "disables the runner" do
+      expect { runner.disable }.to change { runner.enabled? }.from(true).to(false)
     end
   end
 end

spec/datadog/ci/test_visibility/recorder_spec.rb

@@ -48,16 +48,6 @@
     end
   end
 
-  describe "#initialize" do
-    context "no ITR runner is provided" do
-      subject { described_class.new }
-
-      it "raises an error" do
-        expect { subject }.to raise_error(ArgumentError, "ITR runner is required")
-      end
-    end
-  end
-
   context "when test suite level visibility is disabled" do
     let(:service) { "my-service" }
     let(:tags) { {"test.framework" => "my-framework", "my.tag" => "my_value"} }

spec/datadog/ci/transport/api_client_spec.rb

@@ -0,0 +1,31 @@
+require_relative "../../../../lib/datadog/ci/transport/api_client"
+
+RSpec.describe Datadog::CI::Transport::ApiClient do
+  let(:api) { spy("api") }
+  subject { described_class.new(api: api) }
+
+  describe "#fetch_library_settings" do
+    let(:service) { "service" }
+    let(:path) { Datadog::CI::Ext::Transport::DD_API_SETTINGS_PATH }
+    let(:payload) do
+      {
+        data: {
+          id: "change_me",
+          type: Datadog::CI::Ext::Transport::DD_API_SETTINGS_TYPE,
+          attributes: {
+            service: service
+          }
+        }
+      }.to_json
+    end
+
+    it "requests the settings" do
+      subject.fetch_library_settings(service: service)
+
+      expect(api).to have_received(:api_request).with(
+        path: path,
+        payload: payload
+      )
+    end
+  end
+end