Skip to content

Commit

Permalink
Merge pull request #174 from DataDog/anmarchenko/skip_per_test_covera…
Browse files Browse the repository at this point in the history
…ge_for_bundled_gems

[CIVIS-9951] add settings option to ignore code coverage for bundled gems location
  • Loading branch information
anmarchenko authored May 13, 2024
2 parents 8d3ce5c + 6eddb65 commit 0c97a8c
Show file tree
Hide file tree
Showing 21 changed files with 236 additions and 20 deletions.
1 change: 1 addition & 0 deletions Steepfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,5 @@ target :lib do
library "msgpack"
library "ci_queue"
library "knapsack_pro"
library "bundler"
end
46 changes: 36 additions & 10 deletions ext/datadog_cov/datadog_cov.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,35 @@
#define DD_COV_TARGET_FILES 1
#define DD_COV_TARGET_LINES 2

static int is_prefix(VALUE prefix, const char *str)
{
if (prefix == Qnil)
{
return 0;
}

const char *c_prefix = RSTRING_PTR(prefix);
if (c_prefix == NULL)
{
return 0;
}

long prefix_len = RSTRING_LEN(prefix);
if (strncmp(c_prefix, str, prefix_len) == 0)
{
return 1;
}
else
{
return 0;
}
}

// Data structure
struct dd_cov_data
{
VALUE root;
VALUE ignored_path;
int mode;
VALUE coverage;
};
Expand All @@ -18,6 +43,7 @@ static void dd_cov_mark(void *ptr)
struct dd_cov_data *dd_cov_data = ptr;
rb_gc_mark_movable(dd_cov_data->coverage);
rb_gc_mark_movable(dd_cov_data->root);
rb_gc_mark_movable(dd_cov_data->ignored_path);
}

static void dd_cov_free(void *ptr)
Expand All @@ -32,6 +58,7 @@ static void dd_cov_compact(void *ptr)
struct dd_cov_data *dd_cov_data = ptr;
dd_cov_data->coverage = rb_gc_location(dd_cov_data->coverage);
dd_cov_data->root = rb_gc_location(dd_cov_data->root);
dd_cov_data->ignored_path = rb_gc_location(dd_cov_data->ignored_path);
}

const rb_data_type_t dd_cov_data_type = {
Expand All @@ -49,6 +76,7 @@ static VALUE dd_cov_allocate(VALUE klass)
VALUE obj = TypedData_Make_Struct(klass, struct dd_cov_data, &dd_cov_data_type, dd_cov_data);
dd_cov_data->coverage = rb_hash_new();
dd_cov_data->root = Qnil;
dd_cov_data->ignored_path = Qnil;
dd_cov_data->mode = DD_COV_TARGET_FILES;
return obj;
}
Expand All @@ -66,6 +94,8 @@ static VALUE dd_cov_initialize(int argc, VALUE *argv, VALUE self)
rb_raise(rb_eArgError, "root is required");
}

VALUE rb_ignored_path = rb_hash_lookup(opt, ID2SYM(rb_intern("ignored_path")));

VALUE rb_mode = rb_hash_lookup(opt, ID2SYM(rb_intern("mode")));
if (!RTEST(rb_mode) || rb_mode == ID2SYM(rb_intern("files")))
{
Expand All @@ -84,6 +114,7 @@ static VALUE dd_cov_initialize(int argc, VALUE *argv, VALUE self)
TypedData_Get_Struct(self, struct dd_cov_data, &dd_cov_data_type, dd_cov_data);

dd_cov_data->root = rb_root;
dd_cov_data->ignored_path = rb_ignored_path;
dd_cov_data->mode = mode;

return Qnil;
Expand All @@ -100,20 +131,15 @@ static void dd_cov_update_line_coverage(rb_event_flag_t event, VALUE data, VALUE
return;
}

if (dd_cov_data->root == Qnil)
// if given filename is not located under the root, we skip it
if (is_prefix(dd_cov_data->root, filename) == 0)
{
return;
}

char *c_root = RSTRING_PTR(dd_cov_data->root);
if (c_root == NULL)
{
return;
}
long root_len = RSTRING_LEN(dd_cov_data->root);
// check that root is a prefix of the filename
// so this file is located under the given root
if (strncmp(c_root, filename, root_len) != 0)
// if ignored_path is provided and given filename is located under the ignored_path, we skip it too
// this is useful for ignoring bundled gems location
if (RTEST(dd_cov_data->ignored_path) && is_prefix(dd_cov_data->ignored_path, filename) == 1)
{
return;
}
Expand Down
3 changes: 2 additions & 1 deletion lib/datadog/ci/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ def activate_ci!(settings)
dd_env: settings.env,
config_tags: custom_configuration_tags,
coverage_writer: coverage_writer,
enabled: settings.ci.enabled && settings.ci.itr_enabled
enabled: settings.ci.enabled && settings.ci.itr_enabled,
bundle_location: settings.ci.itr_code_coverage_excluded_bundle_path
)

git_tree_uploader = Git::TreeUploader.new(api: test_visibility_api)
Expand Down
9 changes: 9 additions & 0 deletions lib/datadog/ci/configuration/settings.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require_relative "../ext/settings"
require_relative "../utils/bundle"

module Datadog
module CI
Expand Down Expand Up @@ -67,6 +68,14 @@ def self.add_settings!(base)
o.default true
end

option :itr_code_coverage_excluded_bundle_path do |o|
o.type :string, nilable: true
o.env CI::Ext::Settings::ENV_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH
o.default do
Datadog::CI::Utils::Bundle.location
end
end

define_method(:instrument) do |integration_name, options = {}, &block|
return unless enabled

Expand Down
5 changes: 5 additions & 0 deletions lib/datadog/ci/ext/environment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ module Environment
TAG_NODE_NAME = "ci.node.name"
TAG_CI_ENV_VARS = "_dd.ci.env_vars"

POSSIBLE_BUNDLE_LOCATIONS = [
"vendor/bundle",
".bundle"
].freeze

module_function

def tags(env)
Expand Down
1 change: 1 addition & 0 deletions lib/datadog/ci/ext/settings.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ module Settings
ENV_FORCE_TEST_LEVEL_VISIBILITY = "DD_CIVISIBILITY_FORCE_TEST_LEVEL_VISIBILITY"
ENV_ITR_ENABLED = "DD_CIVISIBILITY_ITR_ENABLED"
ENV_GIT_METADATA_UPLOAD_ENABLED = "DD_CIVISIBILITY_GIT_METADATA_UPLOAD_ENABLED"
ENV_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH = "DD_CIVISIBILITY_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH"

# Source: https://docs.datadoghq.com/getting_started/site/
DD_SITE_ALLOWLIST = [
Expand Down
16 changes: 13 additions & 3 deletions lib/datadog/ci/itr/runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,27 @@ def initialize(
config_tags: {},
api: nil,
coverage_writer: nil,
enabled: false
enabled: false,
bundle_location: nil
)
@enabled = enabled
@api = api
@dd_env = dd_env
@config_tags = config_tags || {}

@bundle_location = if bundle_location && !File.absolute_path?(bundle_location)
File.join(Git::LocalRepository.root, bundle_location)
else
bundle_location
end

@test_skipping_enabled = false
@code_coverage_enabled = false

@coverage_writer = coverage_writer

@correlation_id = nil
@skippable_tests = []
@skippable_tests = Set.new

@skipped_tests_count = 0
@mutex = Mutex.new
Expand Down Expand Up @@ -177,7 +184,10 @@ def write(event)
end

def coverage_collector
Thread.current[:dd_coverage_collector] ||= Coverage::DDCov.new(root: Git::LocalRepository.root)
Thread.current[:dd_coverage_collector] ||= Coverage::DDCov.new(
root: Git::LocalRepository.root,
ignored_path: @bundle_location
)
end

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

require_relative "../ext/environment"
require_relative "../git/local_repository"

module Datadog
module CI
module Utils
module Bundle
def self.location
require "bundler"
bundle_path = Bundler.bundle_path.to_s
bundle_path if bundle_path&.start_with?(Datadog::CI::Git::LocalRepository.root)
rescue => e
Datadog.logger.warn("Failed to find bundled gems location: #{e}")

Ext::Environment::POSSIBLE_BUNDLE_LOCATIONS.each do |location|
path = File.join(Datadog::CI::Git::LocalRepository.root, location)
return path if File.directory?(path)
end
nil
end
end
end
end
end
2 changes: 2 additions & 0 deletions sig/datadog/ci/ext/environment.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ module Datadog

TAG_CI_ENV_VARS: String

POSSIBLE_BUNDLE_LOCATIONS: Array[String]

PROVIDERS: ::Array[Array[String | Symbol]]

def self?.tags: (untyped env) -> Hash[String, String]
Expand Down
1 change: 1 addition & 0 deletions sig/datadog/ci/ext/settings.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module Datadog
ENV_FORCE_TEST_LEVEL_VISIBILITY: String
ENV_ITR_ENABLED: String
ENV_GIT_METADATA_UPLOAD_ENABLED: String
ENV_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH: String

DD_SITE_ALLOWLIST: Array[String]
end
Expand Down
2 changes: 1 addition & 1 deletion sig/datadog/ci/itr/coverage/ddcov.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ module Datadog
module Coverage
class DDCov

def initialize: (root: String, ?mode: Symbol) -> void
def initialize: (root: String, ?mode: Symbol, ignored_path: String?) -> void

def start: () -> void

Expand Down
3 changes: 2 additions & 1 deletion sig/datadog/ci/itr/runner.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ module Datadog
@api: Datadog::CI::Transport::Api::Base?
@dd_env: String?
@config_tags: Hash[String, String]
@bundle_location: String?

@skipped_tests_count: Integer
@mutex: Thread::Mutex
Expand All @@ -22,7 +23,7 @@ module Datadog
attr_reader skipped_tests_count: Integer
attr_reader correlation_id: String?

def initialize: (dd_env: String?, ?enabled: bool, coverage_writer: Datadog::CI::ITR::Coverage::Writer?, api: Datadog::CI::Transport::Api::Base?, ?config_tags: Hash[String, String]?) -> void
def initialize: (dd_env: String?, ?enabled: bool, ?coverage_writer: Datadog::CI::ITR::Coverage::Writer?, ?api: Datadog::CI::Transport::Api::Base?, ?config_tags: Hash[String, String]?, ?bundle_location: String?) -> void

def configure: (Hash[String, untyped] remote_configuration, test_session: Datadog::CI::TestSession, git_tree_upload_worker: Datadog::CI::Worker) -> void

Expand Down
9 changes: 9 additions & 0 deletions sig/datadog/ci/utils/bundle.rbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
module Datadog
module CI
module Utils
module Bundle
def self.location: () -> String?
end
end
end
end
48 changes: 48 additions & 0 deletions spec/datadog/ci/configuration/settings_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,54 @@ def patcher
end
end

describe "#itr_code_coverage_excluded_bundle_path" do
subject(:itr_code_coverage_excluded_bundle_path) do
settings.ci.itr_code_coverage_excluded_bundle_path
end

it { is_expected.to be nil }

context "when #{Datadog::CI::Ext::Settings::ENV_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH}" do
around do |example|
ClimateControl.modify(
Datadog::CI::Ext::Settings::ENV_ITR_CODE_COVERAGE_EXCLUDED_BUNDLE_PATH => path
) do
example.run
end
end

context "is not defined" do
let(:path) { nil }

it { is_expected.to be nil }

context "and when bundle location is found in project folder" do
let(:bundle_location) { "/path/to/repo/vendor/bundle" }
before do
allow(Datadog::CI::Utils::Bundle).to receive(:location).and_return(bundle_location)
end

it { is_expected.to eq bundle_location }
end
end

context "is set to some value" do
let(:path) { "/path/to/excluded" }

it { is_expected.to eq path }
end
end
end

describe "#itr_code_coverage_excluded_bundle_path=" do
it "updates the #enabled setting" do
expect { settings.ci.itr_code_coverage_excluded_bundle_path = "/path/to/excluded" }
.to change { settings.ci.itr_code_coverage_excluded_bundle_path }
.from(nil)
.to("/path/to/excluded")
end
end

describe "#instrument" do
let(:integration_name) { :fake }

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module Helper
def self.help?
true
end
end
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require_relative "helpers/helper"

Then "datadog" do
true
Helper.help?
end

Then "failure" do
Expand Down
3 changes: 2 additions & 1 deletion spec/datadog/ci/contrib/cucumber/instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
let(:itr_enabled) { true }
let(:code_coverage_enabled) { true }
let(:tests_skipping_enabled) { true }
let(:bundle_path) { "step_definitions/helpers" }
end

let(:cucumber_8_or_above) { Gem::Version.new("8.0.0") <= Datadog::CI::Contrib::Cucumber::Integration.version }
Expand Down Expand Up @@ -196,7 +197,7 @@
context "collecting coverage with features dir as root" do
before { skip if PlatformHelpers.jruby? }

it "creates coverage events for each non-skipped test" do
it "creates coverage events for each non-skipped test ignoring bundle_path" do
expect(coverage_events).to have(1).item

expect_coverage_events_belong_to_session(test_session_span)
Expand Down
Loading

0 comments on commit 0c97a8c

Please sign in to comment.