* 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

@@ -1,3 +1,15 @@
+Fri Apr  1 01:26:00 2016  Benoit Daloze  <eregontp@gmail.com>
+
+	* 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  <nobu@ruby-lang.org>
 
 	* configure.in (rb_cv_lgamma_r_m0): fix the condition for

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())) {
+    VALUE coverages = rb_get_coverages();
-	if (rb_coverages == Qundef) {
+    if (!RTEST(coverages)) {
-	    rb_coverages = rb_hash_new();
+	coverages = rb_hash_new();
-	    rb_obj_hide(rb_coverages);
+	rb_obj_hide(coverages);
-	}
+	rb_set_coverages(coverages);
-	rb_set_coverages(rb_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);
 }

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
+            1 + 2
-              :ok
-              :ok
-            end
           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

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);
 }