Feature #4796

Coverage should be restartable

Added by Xavier Shay almost 3 years ago. Updated almost 3 years ago.

[ruby-core:36539]
Status:Closed
Priority:Low
Assignee:Tomoyuki Chikanaga
Category:lib
Target version:1.9.3

Description

I would like a way to be able to make the following test past:

require "test/unit"
require "coverage"
require 'tmpdir'

class TestCoverage < Test::Unit::TestCase
  def test_restarting_coverage
    Dir.mktmpdir {|tmp|
      Dir.chdir(tmp) {
        File.open("test.rb", "w") do |f|
          f.puts <<-EOS
            def coverage_test_method
              puts :ok
            end
          EOS
        end

        Coverage.start
        require tmp + '/test.rb'
        Coverage.result
        Coverage.start
        coverage_test_method
        assert_equal 1, Coverage.result.size
      }
    }
  end
end

The problem is that Coverage.start 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. Specifically, I am trying to collate coverage reports from workers in multiple processes. Also I want to associate coverages with specific tests (this test executed this code, etc...). This is very difficult without being able to restart.

What would be involved in doing this? If you point me in the right direction I can perhaps have a go.

Associated revisions

Revision 32404
Added by Tomoyuki Chikanaga almost 3 years ago

  • ext/coverage/coverage.c: resurrect r32071 + add GC guard for
    rb_coverages. [Bug #4927]
    [Feature #4796]

  • test/coverage/test_coverage.rb resurrect r32071.

History

#1 Updated by Yusuke Endoh almost 3 years ago

  • Category set to lib
  • Status changed from Open to Assigned
  • Assignee set to Yusuke Endoh
  • Target version set to 1.9.3

Hello.

As a maintainer of ext/coverage, I like to approve this feature unless
there is objection. But I'd like to confirm some points.

2011/5/29 Xavier Shay xavier-list@rhnh.net:

The problem is that Coverage.start 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.

First of all, it is difficult to track files loaded before start' is
first called. This is because the VM compiler inserts instructions to
measure coverage after
start' is first called. Since the mechanism
have considerable overhead, it cannot be enabled by default.

You are talking about second (or later) calls to Coverage.start, right?

Specifically, I am trying to collate coverage reports from workers in multiple processes.

I cannot understand why the feature is needed for this use case.
Could you elaborate this?

Also I want to associate coverages with specific tests (this test executed this code, etc...). This is very difficult without being able to restart.

I can understand this.

Here is a patch.

diff --git a/ext/coverage/coverage.c b/ext/coverage/coverage.c
index 29ac709..3a26aaa 100644
--- a/ext/coverage/coverage.c
+++ b/ext/coverage/coverage.c
@@ -11,6 +11,8 @@
#include "ruby.h"
#include "vm_core.h"

+static VALUE rbcoverages = Qundef;
+
/*
* call-seq:
* Coverage.start => nil
@@ -21,19 +23,25 @@ static VALUE
rb
coveragestart(VALUE klass)
{
if (!RTEST(rb
getcoverages())) {
- VALUE coverages = rb
hashnew();
- RBASIC(coverages)->klass = 0;
- rb
setcoverages(coverages);
+ if (rb
coverages == Qundef) {
+ rbcoverages = rbhashnew();
+ RBASIC(rb
coverages)->klass = 0;
+ }
+ rbsetcoverages(rb_coverages);
}
return Qnil;
}

static int
-coverageresulti(stdatat key, stdatat val, stdatat dummy)
+coverageresulti(stdatat key, stdatat val, stdatat h)
{
+ VALUE path = (VALUE)key;
VALUE coverage = (VALUE)val;
- RBASIC(coverage)->klass = rbcArray;
+ VALUE coverages = (VALUE)h;
+ coverage = rb
arydup(coverage);
+ rb
aryclear((VALUE)val);
rb
aryfreeze(coverage);
+ rb
hashaset(coverages, path, coverage);
return ST
CONTINUE;
}

@@ -48,14 +56,14 @@ static VALUE
rbcoverageresult(VALUE klass)
{
VALUE coverages = rbgetcoverages();
+ VALUE ncoverages = rbhashnew();
if (!RTEST(coverages)) {
rbraise(rbeRuntimeError, "coverage measurement is not enabled");
}
- RBASIC(coverages)->klass = rbcHash;
- st
foreach(RHASHTBL(coverages), coverageresulti, 0);
- rb
hashfreeze(coverages);
+ st
foreach(RHASHTBL(coverages), coverageresulti, ncoverages);
+ rb
hashfreeze(ncoverages);
rb
reset_coverages();
- return coverages;
+ return ncoverages;
}

/* Coverage provides coverage measurement feature for Ruby.

Yusuke Endoh mame@tsg.ne.jp

#2 Updated by Xavier Shay almost 3 years ago

You are talking about second (or later) calls to Coverage.start, right?
Yes, that would be fine.

I cannot understand why the feature is needed for this use case.
Could you elaborate this?

It's not required, it just would of made my life easier. I was trying to integrate with the parallel and SimpleCov 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.

Here is a patch.
I will try it out and report back. Thanks!

#3 Updated by Xavier Shay almost 3 years ago

I tried this patch and it works as expected. Here is a patch for the test case, slightly cleaned up:

diff --git a/test/coverage/testcoverage.rb b/test/coverage/testcoverage.rb
index ace49d3..56966b1 100644
--- a/test/coverage/testcoverage.rb
+++ b/test/coverage/test
coverage.rb
@@ -1,5 +1,6 @@
require "test/unit"
require "coverage"
+require "tmpdir"

class TestCoverage < Test::Unit::TestCase
def testresultwithoutstart
@@ -14,4 +15,29 @@ class TestCoverage < Test::Unit::TestCase
assert
kindof(Array, val)
end
end
+
+ def test
restartingcoverage
+ loaded
features = $".dup
+
+ Dir.mktmpdir {|tmp|
+ Dir.chdir(tmp) {
+ File.open("test.rb", "w") do |f|
+ f.puts <<-EOS
+ def coveragetestmethod
+ :ok
+ end
+ EOS
+ end
+
+ Coverage.start
+ require tmp + '/test.rb'
+ Coverage.result
+ Coverage.start
+ coveragetestmethod
+ assertequal 1, Coverage.result.size
+ }
+ }
+ ensure
+ $".replace loaded
features
+ end
end

#4 Updated by Yusuke Endoh almost 3 years ago

  • Status changed from Assigned to Closed

Sorry for late action. I've committed my implementation and
your test patch. Thank you!

Yusuke Endoh mame@tsg.ne.jp

#5 Updated by Yusuke Endoh almost 3 years ago

  • Status changed from Closed to Open
  • Assignee deleted (Yusuke Endoh)
  • Priority changed from Normal to Low
  • Target version changed from 1.9.3 to 2.0.0

Unfortunately this patch seems to cause SEGV on os x. [#4927]
I cannot fix it because I have no os x. So I can't help but
revert it.

Anyone who wants this feature, please fix it by yourself or
please give me a Mac :-)

Yusuke Endoh mame@tsg.ne.jp

#6 Updated by Tomoyuki Chikanaga almost 3 years ago

  • Status changed from Open to Assigned
  • Assignee set to Tomoyuki Chikanaga
  • Target version changed from 2.0.0 to 1.9.3

Sorry, I don't have extra Mac. But I can show a patch.
I'll commit it.

#7 Updated by Tomoyuki Chikanaga almost 3 years ago

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r32404.
Xavier, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • ext/coverage/coverage.c: resurrect r32071 + add GC guard for
    rb_coverages. [Bug #4927]
    [Feature #4796]

  • test/coverage/test_coverage.rb resurrect r32071.

#8 Updated by Yusuke Endoh almost 3 years ago

That's the best! Thank you nagachika!

Yusuke Endoh mame@tsg.ne.jp

Also available in: Atom PDF