diff --git a/ext/datadog_cov/datadog_cov.c b/ext/datadog_cov/datadog_cov.c index 7d057307..cc260678 100644 --- a/ext/datadog_cov/datadog_cov.c +++ b/ext/datadog_cov/datadog_cov.c @@ -6,14 +6,12 @@ struct dd_cov_data { VALUE root; - VALUE coverage; st_table *covered_files; }; 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); } @@ -23,18 +21,11 @@ static int free_hashtable_keys(st_data_t key, st_data_t value, st_data_t data) return ST_CONTINUE; } -// temporary function to inspect hashtable -static int inspect_hashtable(st_data_t key, st_data_t value, st_data_t data) -{ - printf("key: %s\n", (char *)key); - return ST_CONTINUE; -} - static void dd_cov_free(void *ptr) { struct dd_cov_data *dd_cov_data = ptr; - st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0); + // st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0); st_free_table(dd_cov_data->covered_files); xfree(dd_cov_data); @@ -43,7 +34,6 @@ static void dd_cov_free(void *ptr) 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); } @@ -61,9 +51,7 @@ static VALUE dd_cov_allocate(VALUE klass) struct dd_cov_data *dd_cov_data; 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->covered_files = st_init_strtable(); return obj; @@ -118,12 +106,7 @@ static void dd_cov_update_line_coverage(rb_event_flag_t event, VALUE data, VALUE return; } - // new way to store covered files st_insert(dd_cov_data->covered_files, (st_data_t)filename, 1); - - // old way to store covered files - VALUE rb_str_source_file = rb_str_new2(filename); - rb_hash_aset(dd_cov_data->coverage, rb_str_source_file, Qtrue); } static VALUE dd_cov_start(VALUE self) @@ -137,6 +120,12 @@ static VALUE dd_cov_start(VALUE self) return self; } +static int insert_into_result(st_data_t filename, st_data_t _val, st_data_t ary) +{ + rb_ary_push((VALUE)ary, rb_str_new2((char *)filename)); + return ST_CONTINUE; +} + static VALUE dd_cov_stop(VALUE self) { // get current thread @@ -147,13 +136,9 @@ static VALUE dd_cov_stop(VALUE self) struct dd_cov_data *dd_cov_data; TypedData_Get_Struct(self, struct dd_cov_data, &dd_cov_data_type, dd_cov_data); - st_foreach(dd_cov_data->covered_files, inspect_hashtable, 0); - - VALUE cov = dd_cov_data->coverage; - - dd_cov_data->coverage = rb_hash_new(); + VALUE cov = rb_ary_new2(dd_cov_data->covered_files->num_entries); + st_foreach(dd_cov_data->covered_files, insert_into_result, cov); - // freeing these strings makes them unavailable for Ruby too // st_foreach(dd_cov_data->covered_files, free_hashtable_keys, 0); st_clear(dd_cov_data->covered_files); diff --git a/sig/datadog/ci/itr/coverage/ddcov.rbs b/sig/datadog/ci/itr/coverage/ddcov.rbs index 8fe33f94..b6cafaa5 100644 --- a/sig/datadog/ci/itr/coverage/ddcov.rbs +++ b/sig/datadog/ci/itr/coverage/ddcov.rbs @@ -8,7 +8,7 @@ module Datadog def start: () -> void - def stop: () -> Hash[String, untyped] + def stop: () -> Array[String] end end end diff --git a/spec/ddcov/ddcov_spec.rb b/spec/ddcov/ddcov_spec.rb index f5e662d4..edc1ff49 100644 --- a/spec/ddcov/ddcov_spec.rb +++ b/spec/ddcov/ddcov_spec.rb @@ -22,7 +22,7 @@ def absolute_path(path) expect(calculator.add(1, 2)).to eq(3) coverage = cov.stop - expect(coverage).to eq({}) + expect(coverage).to eq([]) end end @@ -38,7 +38,7 @@ def absolute_path(path) coverage = subject.stop expect(coverage.size).to eq(3) - expect(coverage.keys).to include( + expect(coverage).to include( absolute_path("calculator/calculator.rb"), absolute_path("calculator/operations/add.rb"), absolute_path("calculator/operations/subtract.rb") @@ -51,7 +51,7 @@ def absolute_path(path) coverage = subject.stop expect(coverage.size).to eq(1) # this string will have a bunch of UTF-8 codepoints in it - expect(coverage.keys.first).to include("calculator/code_with_") + expect(coverage.first).to include("calculator/code_with_") end end @@ -82,7 +82,7 @@ def absolute_path(path) coverage = subject.stop expect(coverage.size).to eq(2) - expect(coverage.keys).to include( + expect(coverage).to include( absolute_path("calculator/operations/add.rb"), absolute_path("calculator/operations/subtract.rb") ) @@ -93,13 +93,13 @@ def absolute_path(path) expect(calculator.add(1, 2)).to eq(3) coverage = subject.stop expect(coverage.size).to eq(1) - expect(coverage.keys).to include(absolute_path("calculator/operations/add.rb")) + expect(coverage).to include(absolute_path("calculator/operations/add.rb")) subject.start expect(calculator.subtract(1, 2)).to eq(-1) coverage = subject.stop expect(coverage.size).to eq(1) - expect(coverage.keys).to include(absolute_path("calculator/operations/subtract.rb")) + expect(coverage).to include(absolute_path("calculator/operations/subtract.rb")) end it "does not track coverage when stopped" do @@ -113,7 +113,7 @@ def absolute_path(path) expect(calculator.multiply(1, 2)).to eq(2) coverage = subject.stop expect(coverage.size).to eq(1) - expect(coverage.keys).to include(absolute_path("calculator/operations/multiply.rb")) + expect(coverage).to include(absolute_path("calculator/operations/multiply.rb")) end it "does not fail if start called several times" do @@ -131,7 +131,7 @@ def absolute_path(path) coverage = subject.stop expect(coverage.size).to eq(1) - expect(subject.stop).to eq({}) + expect(subject.stop).to eq([]) end it "tracks coverage in mixins" do @@ -139,8 +139,8 @@ def absolute_path(path) expect(calculator.divide(6, 3)).to eq(2) coverage = subject.stop expect(coverage.size).to eq(2) - expect(coverage.keys).to include(absolute_path("calculator/operations/divide.rb")) - expect(coverage.keys).to include(absolute_path("calculator/operations/helpers/calculator_logger.rb")) + expect(coverage).to include(absolute_path("calculator/operations/divide.rb")) + expect(coverage).to include(absolute_path("calculator/operations/helpers/calculator_logger.rb")) end context "multi threaded execution" do @@ -167,8 +167,8 @@ def thread_local_cov coverage = cov.stop expect(coverage.size).to eq(2) - expect(coverage.keys).to include(absolute_path("calculator/operations/add.rb")) - expect(coverage.keys).to include(absolute_path("calculator/operations/multiply.rb")) + expect(coverage).to include(absolute_path("calculator/operations/add.rb")) + expect(coverage).to include(absolute_path("calculator/operations/multiply.rb")) end t2 = Thread.new do @@ -185,7 +185,7 @@ def thread_local_cov coverage = cov.stop expect(coverage.size).to eq(1) - expect(coverage.keys).to include(absolute_path("calculator/operations/subtract.rb")) + expect(coverage).to include(absolute_path("calculator/operations/subtract.rb")) end [t1, t2].each(&:join)