Project

General

Profile

Actions

Feature #4796

closed

Coverage should be restartable

Added by xaviershay (Xavier Shay) over 13 years ago. Updated over 13 years ago.

Status:
Closed
Target version:
[ruby-core:36539]

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.

Updated by mame (Yusuke Endoh) over 13 years ago

  • Category set to lib
  • Status changed from Open to Assigned
  • Assignee set to mame (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 :

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 rb_coverages = Qundef;
+
/*

  • call-seq:
  • Coverage.start => nil
    @@ -21,19 +23,25 @@ static VALUE
    rb_coverage_start(VALUE klass)
    {
    if (!RTEST(rb_get_coverages())) {
  • VALUE coverages = rb_hash_new();
  • RBASIC(coverages)->klass = 0;
  • rb_set_coverages(coverages);
  • if (rb_coverages == Qundef) {
  •   rb_coverages = rb_hash_new();
    
  •   RBASIC(rb_coverages)->klass = 0;
    
  • }
  • rb_set_coverages(rb_coverages);
    }
    return Qnil;
    }

static int
-coverage_result_i(st_data_t key, st_data_t val, st_data_t dummy)
+coverage_result_i(st_data_t key, st_data_t val, st_data_t h)
{

  • VALUE path = (VALUE)key;
    VALUE coverage = (VALUE)val;
  • RBASIC(coverage)->klass = rb_cArray;
  • VALUE coverages = (VALUE)h;
  • coverage = rb_ary_dup(coverage);
  • rb_ary_clear((VALUE)val);
    rb_ary_freeze(coverage);
  • rb_hash_aset(coverages, path, coverage);
    return ST_CONTINUE;
    }

@@ -48,14 +56,14 @@ static VALUE
rb_coverage_result(VALUE klass)
{
VALUE coverages = rb_get_coverages();

  • VALUE ncoverages = rb_hash_new();
    if (!RTEST(coverages)) {
    rb_raise(rb_eRuntimeError, "coverage measurement is not enabled");
    }
  • RBASIC(coverages)->klass = rb_cHash;
  • st_foreach(RHASH_TBL(coverages), coverage_result_i, 0);
  • rb_hash_freeze(coverages);
  • st_foreach(RHASH_TBL(coverages), coverage_result_i, ncoverages);
  • rb_hash_freeze(ncoverages);
    rb_reset_coverages();
  • return coverages;
  • return ncoverages;
    }

/* Coverage provides coverage measurement feature for Ruby.

--
Yusuke Endoh

Updated by xaviershay (Xavier Shay) over 13 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!

Updated by xaviershay (Xavier Shay) over 13 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/test_coverage.rb b/test/coverage/test_coverage.rb
index ace49d3..56966b1 100644
--- a/test/coverage/test_coverage.rb
+++ b/test/coverage/test_coverage.rb
@@ -1,5 +1,6 @@
require "test/unit"
require "coverage"
+require "tmpdir"

class TestCoverage < Test::Unit::TestCase
def test_result_without_start
@@ -14,4 +15,29 @@ class TestCoverage < Test::Unit::TestCase
assert_kind_of(Array, val)
end
end
+

  • def test_restarting_coverage
  • loaded_features = $".dup
  • Dir.mktmpdir {|tmp|
  •  Dir.chdir(tmp) {
    
  •    File.open("test.rb", "w") do |f|
    
  •      f.puts <<-EOS
    
  •        def coverage_test_method
    
  •          :ok
    
  •        end
    
  •      EOS
    
  •    end
    
  •    Coverage.start
    
  •    require tmp + '/test.rb'
    
  •    Coverage.result
    
  •    Coverage.start
    
  •    coverage_test_method
    
  •    assert_equal 1, Coverage.result.size
    
  •  }
    
  • }
  • ensure
  • $".replace loaded_features
  • end
    end

Updated by mame (Yusuke Endoh) over 13 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

Updated by mame (Yusuke Endoh) over 13 years ago

  • Status changed from Closed to Open
  • Assignee deleted (mame (Yusuke Endoh))
  • Priority changed from Normal to 3
  • 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

Updated by nagachika (Tomoyuki Chikanaga) over 13 years ago

  • Status changed from Open to Assigned
  • Assignee set to nagachika (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.

Actions #7

Updated by nagachika (Tomoyuki Chikanaga) over 13 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.


Updated by mame (Yusuke Endoh) over 13 years ago

That's the best! Thank you nagachika!

--
Yusuke Endoh

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0