https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112011-05-29T13:51:04ZRuby Issue Tracking SystemRuby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=173002011-05-29T13:51:04Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Category</strong> set to <i>lib</i></li><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>mame (Yusuke Endoh)</i></li><li><strong>Target version</strong> set to <i>1.9.3</i></li></ul><p>Hello.</p>
<p>As a maintainer of ext/coverage, I like to approve this feature unless<br>
there is objection. But I'd like to confirm some points.</p>
<p>2011/5/29 Xavier Shay <a href="mailto:xavier-list@rhnh.net" class="email">xavier-list@rhnh.net</a>:</p>
<blockquote>
<p>The problem is that <code>Coverage.start</code> doesn't track any files loaded before it is called. This is probably desired behaviour so that stdlib files are not tracked, but it limits the usefulness of Coverage.</p>
</blockquote>
<p>First of all, it is difficult to track files loaded before <code>start' is first called. This is because the VM compiler inserts instructions to measure coverage after </code>start' is first called. Since the mechanism<br>
have considerable overhead, it cannot be enabled by default.</p>
<p>You are talking about second (or later) calls to Coverage.start, right?</p>
<blockquote>
<p>Specifically, I am trying to collate coverage reports from workers in multiple processes.</p>
</blockquote>
<p>I cannot understand why the feature is needed for this use case.<br>
Could you elaborate this?</p>
<blockquote>
<p>Also I want to associate coverages with specific tests (this test executed this code, etc...). This is very difficult without being able to restart.</p>
</blockquote>
<p>I can understand this.</p>
<p>Here is a patch.</p>
<p>diff --git a/ext/coverage/coverage.c b/ext/coverage/coverage.c<br>
index 29ac709..3a26aaa 100644<br>
--- a/ext/coverage/coverage.c<br>
+++ b/ext/coverage/coverage.c<br>
@@ -11,6 +11,8 @@<br>
#include "ruby.h"<br>
#include "vm_core.h"</p>
<p>+static VALUE rb_coverages = Qundef;<br>
+<br>
/*</p>
<ul>
<li>call-seq:</li>
<li>Coverage.start => nil<br>
@@ -21,19 +23,25 @@ static VALUE<br>
rb_coverage_start(VALUE klass)<br>
{<br>
if (!RTEST(rb_get_coverages())) {</li>
</ul>
<ul>
<li>VALUE coverages = rb_hash_new();</li>
<li>RBASIC(coverages)->klass = 0;</li>
<li>rb_set_coverages(coverages);</li>
</ul>
<ul>
<li>if (rb_coverages == Qundef) {</li>
<li>
<pre><code> rb_coverages = rb_hash_new();
</code></pre>
</li>
<li>
<pre><code> RBASIC(rb_coverages)->klass = 0;
</code></pre>
</li>
<li>}</li>
<li>rb_set_coverages(rb_coverages);<br>
}<br>
return Qnil;<br>
}</li>
</ul>
<p>static int<br>
-coverage_result_i(st_data_t key, st_data_t val, st_data_t dummy)<br>
+coverage_result_i(st_data_t key, st_data_t val, st_data_t h)<br>
{</p>
<ul>
<li>VALUE path = (VALUE)key;<br>
VALUE coverage = (VALUE)val;</li>
</ul>
<ul>
<li>RBASIC(coverage)->klass = rb_cArray;</li>
</ul>
<ul>
<li>VALUE coverages = (VALUE)h;</li>
<li>coverage = rb_ary_dup(coverage);</li>
<li>rb_ary_clear((VALUE)val);<br>
rb_ary_freeze(coverage);</li>
<li>rb_hash_aset(coverages, path, coverage);<br>
return ST_CONTINUE;<br>
}</li>
</ul>
<p>@@ -48,14 +56,14 @@ static VALUE<br>
rb_coverage_result(VALUE klass)<br>
{<br>
VALUE coverages = rb_get_coverages();</p>
<ul>
<li>VALUE ncoverages = rb_hash_new();<br>
if (!RTEST(coverages)) {<br>
rb_raise(rb_eRuntimeError, "coverage measurement is not enabled");<br>
}</li>
</ul>
<ul>
<li>RBASIC(coverages)->klass = rb_cHash;</li>
<li>st_foreach(RHASH_TBL(coverages), coverage_result_i, 0);</li>
<li>rb_hash_freeze(coverages);</li>
</ul>
<ul>
<li>st_foreach(RHASH_TBL(coverages), coverage_result_i, ncoverages);</li>
<li>rb_hash_freeze(ncoverages);<br>
rb_reset_coverages();</li>
</ul>
<ul>
<li>return coverages;</li>
</ul>
<ul>
<li>return ncoverages;<br>
}</li>
</ul>
<p>/* Coverage provides coverage measurement feature for Ruby.</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=173012011-05-29T14:37:41Zxaviershay (Xavier Shay)xavier-list@rhnh.net
<ul></ul><blockquote>
<p>You are talking about second (or later) calls to Coverage.start, right?<br>
Yes, that would be fine.</p>
</blockquote>
<blockquote>
<p>I cannot understand why the feature is needed for this use case.<br>
Could you elaborate this?</p>
</blockquote>
<p>It's not required, it just would of made my life easier. I was trying to integrate with the <code>parallel</code> and <code>SimpleCov</code> gems, and ended up with a work around which involves conditionally hooking at_exit and a file lock, which is a bit of a mess.</p>
<blockquote>
<p>Here is a patch.<br>
I will try it out and report back. Thanks!</p>
</blockquote> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=175062011-06-05T15:15:33Zxaviershay (Xavier Shay)xavier-list@rhnh.net
<ul></ul><p>I tried this patch and it works as expected. Here is a patch for the test case, slightly cleaned up:</p>
<p>diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb<br>
index ace49d3..56966b1 100644<br>
--- a/test/coverage/test_coverage.rb<br>
+++ b/test/coverage/test_coverage.rb<br>
@@ -1,5 +1,6 @@<br>
require "test/unit"<br>
require "coverage"<br>
+require "tmpdir"</p>
<p>class TestCoverage < Test::Unit::TestCase<br>
def test_result_without_start<br>
@@ -14,4 +15,29 @@ class TestCoverage < Test::Unit::TestCase<br>
assert_kind_of(Array, val)<br>
end<br>
end<br>
+</p>
<ul>
<li>def test_restarting_coverage</li>
<li>loaded_features = $".dup</li>
<li>
<li>Dir.mktmpdir {|tmp|</li>
<li>
<pre><code> Dir.chdir(tmp) {
</code></pre>
</li>
<li>
<pre><code> File.open("test.rb", "w") do |f|
</code></pre>
</li>
<li>
<pre><code> f.puts <<-EOS
</code></pre>
</li>
<li>
<pre><code> def coverage_test_method
</code></pre>
</li>
<li>
<pre><code> :ok
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<pre><code> EOS
</code></pre>
</li>
<li>
<pre><code> end
</code></pre>
</li>
<li>
<li>
<pre><code> Coverage.start
</code></pre>
</li>
<li>
<pre><code> require tmp + '/test.rb'
</code></pre>
</li>
<li>
<pre><code> Coverage.result
</code></pre>
</li>
<li>
<pre><code> Coverage.start
</code></pre>
</li>
<li>
<pre><code> coverage_test_method
</code></pre>
</li>
<li>
<pre><code> assert_equal 1, Coverage.result.size
</code></pre>
</li>
<li>
<pre><code> }
</code></pre>
</li>
<li>}</li>
<li>ensure</li>
<li>$".replace loaded_features</li>
<li>end<br>
end</li>
</ul> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=179162011-06-14T01:14:36Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Sorry for late action. I've committed my implementation and<br>
your test patch. Thank you!</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=182902011-06-26T14:58:04Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Closed</i> to <i>Open</i></li><li><strong>Assignee</strong> deleted (<del><i>mame (Yusuke Endoh)</i></del>)</li><li><strong>Priority</strong> changed from <i>Normal</i> to <i>3</i></li><li><strong>Target version</strong> changed from <i>1.9.3</i> to <i>2.0.0</i></li></ul><p>Unfortunately this patch seems to cause SEGV on os x. [<a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: crash on test/coverage/test_coverage.rb (Closed)" href="https://bugs.ruby-lang.org/issues/4927">#4927</a>]<br>
I cannot fix it because I have no os x. So I can't help but<br>
revert it.</p>
<p>Anyone who wants this feature, please fix it by yourself or<br>
please give me a Mac :-)</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=188142011-07-05T00:47:36Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>nagachika (Tomoyuki Chikanaga)</i></li><li><strong>Target version</strong> changed from <i>2.0.0</i> to <i>1.9.3</i></li></ul><p>Sorry, I don't have extra Mac. But I can show a patch.<br>
I'll commit it.</p> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=188152011-07-05T00:55:08Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>This issue was solved with changeset r32404.<br>
Xavier, thank you for reporting this issue.<br>
Your contribution to Ruby is greatly appreciated.<br>
May Ruby be with you.</p>
<hr>
<ul>
<li>
<p>ext/coverage/coverage.c: resurrect r32071 + add GC guard for<br>
rb_coverages. <a href="/issues/4927">[ruby-core:37352]</a> [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: crash on test/coverage/test_coverage.rb (Closed)" href="https://bugs.ruby-lang.org/issues/4927">#4927</a>]<br>
<a href="/issues/4796">[ruby-core:36539]</a> [Feature <a class="issue tracker-2 status-5 priority-4 priority-default closed" title="Feature: Coverage should be restartable (Closed)" href="https://bugs.ruby-lang.org/issues/4796">#4796</a>]</p>
</li>
<li>
<p>test/coverage/test_coverage.rb resurrect r32071.</p>
</li>
</ul> Ruby master - Feature #4796: Coverage should be restartablehttps://bugs.ruby-lang.org/issues/4796?journal_id=188162011-07-05T00:55:51Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul></ul><p>That's the best! Thank you nagachika!</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p>