From 18454a8a9c5220362916a805a6699e2780e0354e Mon Sep 17 00:00:00 2001 From: eregon Date: Wed, 6 Apr 2016 14:29:17 +0000 Subject: * ext/coverage/coverage.c: Fully reset coverage to not persist global state. It was returning old file coverages as empty arrays to the user. [ruby-core:74596] [Bug #12220] * ext/coverage/coverage.c (rb_coverages): remove unused static state. * thread.c: Moved and renamed coverage_clear_result_i to reset_coverage_i. * test/coverage/test_coverage.rb: improve precision of tests. git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@54504 b2dd03c8-39d4-4d8f-98ff-823fe69b080e --- ChangeLog | 12 ++++++++++++ ext/coverage/coverage.c | 25 +++++-------------------- test/coverage/test_coverage.rb | 13 ++++++------- thread.c | 11 +++++++++++ 4 files changed, 34 insertions(+), 27 deletions(-) diff --git a/ChangeLog b/ChangeLog index f8d2b81b85..80103d9928 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,15 @@ +Fri Apr 1 01:26:00 2016 Benoit Daloze + + * ext/coverage/coverage.c: Fully reset coverage to not persist global state. + It was returning old file coverages as empty arrays to the user. + [ruby-core:74596] [Bug #12220] + + * ext/coverage/coverage.c (rb_coverages): remove unused static state. + + * thread.c: Moved and renamed coverage_clear_result_i to reset_coverage_i. + + * test/coverage/test_coverage.rb: improve precision of tests. + Wed Apr 6 22:41:31 2016 Nobuyoshi Nakada * configure.in (rb_cv_lgamma_r_m0): fix the condition for diff --git a/ext/coverage/coverage.c b/ext/coverage/coverage.c index f621280ca6..9a9011a7da 100644 --- a/ext/coverage/coverage.c +++ b/ext/coverage/coverage.c @@ -11,8 +11,6 @@ #include "ruby.h" #include "vm_core.h" -static VALUE rb_coverages = Qundef; - /* * call-seq: * Coverage.start => nil @@ -22,25 +20,15 @@ static VALUE rb_coverages = Qundef; static VALUE rb_coverage_start(VALUE klass) { - if (!RTEST(rb_get_coverages())) { - if (rb_coverages == Qundef) { - rb_coverages = rb_hash_new(); - rb_obj_hide(rb_coverages); - } - rb_set_coverages(rb_coverages); + VALUE coverages = rb_get_coverages(); + if (!RTEST(coverages)) { + coverages = rb_hash_new(); + rb_obj_hide(coverages); + rb_set_coverages(coverages); } return Qnil; } -/* Make coverage arrays empty so old covered files are no longer tracked. */ -static int -coverage_clear_result_i(st_data_t key, st_data_t val, st_data_t h) -{ - VALUE coverage = (VALUE)val; - rb_ary_clear(coverage); - return ST_CONTINUE; -} - static int coverage_peek_result_i(st_data_t key, st_data_t val, st_data_t h) { @@ -83,8 +71,6 @@ static VALUE rb_coverage_result(VALUE klass) { VALUE ncoverages = rb_coverage_peek_result(klass); - VALUE coverages = rb_get_coverages(); - st_foreach(RHASH_TBL(coverages), coverage_clear_result_i, ncoverages); rb_reset_coverages(); return ncoverages; } @@ -129,5 +115,4 @@ Init_coverage(void) rb_define_module_function(rb_mCoverage, "start", rb_coverage_start, 0); rb_define_module_function(rb_mCoverage, "result", rb_coverage_result, 0); rb_define_module_function(rb_mCoverage, "peek_result", rb_coverage_peek_result, 0); - rb_gc_register_address(&rb_coverages); } diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb index 1611e1778d..49b64eb2e4 100644 --- a/test/coverage/test_coverage.rb +++ b/test/coverage/test_coverage.rb @@ -63,28 +63,27 @@ class TestCoverage < Test::Unit::TestCase File.open("test2.rb", "w") do |f| f.puts <<-EOS - def coverage_test_method2 - :ok - :ok - end + 1 + 2 EOS end Coverage.start require tmp + '/test.rb' - assert_equal 3, Coverage.result[tmp + '/test.rb'].size + cov = { "#{tmp}/test.rb" => [1, 0, nil] } + assert_equal cov, Coverage.result # Restart coverage but '/test.rb' is required before restart, # so coverage is not recorded. Coverage.start coverage_test_method - assert_equal 0, Coverage.result[tmp + '/test.rb'].size + assert_equal({}, Coverage.result) # Restart coverage and '/test2.rb' is required after restart, # so coverage is recorded. Coverage.start require tmp + '/test2.rb' - assert_equal 4, Coverage.result[tmp + '/test2.rb'].size + cov = { "#{tmp}/test2.rb" => [1] } + assert_equal cov, Coverage.result } } ensure diff --git a/thread.c b/thread.c index a68651b2e1..e3efcd3946 100644 --- a/thread.c +++ b/thread.c @@ -4800,9 +4800,20 @@ rb_set_coverages(VALUE coverages) rb_add_event_hook(update_coverage, RUBY_EVENT_COVERAGE, Qnil); } +/* Make coverage arrays empty so old covered files are no longer tracked. */ +static int +reset_coverage_i(st_data_t key, st_data_t val, st_data_t dummy) +{ + VALUE coverage = (VALUE)val; + rb_ary_clear(coverage); + return ST_CONTINUE; +} + void rb_reset_coverages(void) { + VALUE coverages = rb_get_coverages(); + st_foreach(rb_hash_tbl_raw(coverages), reset_coverage_i, 0); GET_VM()->coverages = Qfalse; rb_remove_event_hook(update_coverage); } -- cgit v1.2.3