Skip to content

Commit cf6aa76

Browse files
committed
Fix coverage results when source contains class_eval
1 parent 574d6bd commit cf6aa76

File tree

8 files changed

+42
-6
lines changed

8 files changed

+42
-6
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Bug fixes:
1212
* Fixed issue when extending FFI from File (#2094).
1313
* Fixed issue with `Kernel#freeze` not freezing singleton class (#2093).
1414
* Fixed `String#encode` with options issue (#2091, #2095, @LillianZ)
15+
* Fixed `coverage` issue when `*eval` is used (#2078).
1516

1617
Compatibility:
1718

spec/ruby/.mspec.constants

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ ComparisonTest
4141
ConstantSpecsIncludedModule
4242
ConstantVisibility
4343
Coverage
44+
CoverageSpecs
4445
CustomArgumentError
4546
DRb
4647
DRbIdConv
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
5 + 5
2+
3+
module CoverageSpecs
4+
5+
class_eval <<-RUBY, __FILE__, __LINE__ + 1
6+
attr_reader :ok
7+
RUBY
8+
9+
end
10+
11+
4 + 4

spec/ruby/library/coverage/result_spec.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@
55
before :all do
66
@class_file = fixture __FILE__, 'some_class.rb'
77
@config_file = fixture __FILE__, 'start_coverage.rb'
8+
@eval_code_file = fixture __FILE__, 'eval_code.rb'
89
end
910

1011
after :each do
1112
$LOADED_FEATURES.delete(@class_file)
1213
$LOADED_FEATURES.delete(@config_file)
14+
$LOADED_FEATURES.delete(@eval_code_file)
1315
end
1416

1517
it 'gives the covered files as a hash with arrays of count or nil' do
@@ -75,4 +77,16 @@
7577
require @config_file.chomp('.rb')
7678
Coverage.result.should_not include(@config_file)
7779
end
80+
81+
it 'returns the correct results when eval is used' do
82+
Coverage.start
83+
require @eval_code_file.chomp('.rb')
84+
result = Coverage.result
85+
86+
result.should == {
87+
@eval_code_file => [
88+
1, nil, 1, nil, 1, nil, nil, nil, nil, nil, 1
89+
]
90+
}
91+
end
7892
end

src/main/java/org/truffleruby/language/eval/CreateEvalSourceNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public RubySource createEvalSource(Rope code, String method, String file, int li
4848

4949
final Source source = Source.newBuilder(TruffleRuby.LANGUAGE_ID, sourceString, file).build();
5050

51-
return new RubySource(source, file, sourceRope);
51+
return new RubySource(source, file, sourceRope, true);
5252
}
5353

5454
private static Rope createEvalRope(Rope source, String method, String file, int line) {

src/main/java/org/truffleruby/parser/RubySource.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,23 @@ public class RubySource {
2424
* for non-file Sources in the future (but then we'll need to still use this path in Ruby backtraces). */
2525
private final String sourcePath;
2626
private final Rope sourceRope;
27+
private final boolean isEval;
2728

2829
public RubySource(Source source, String sourcePath) {
29-
this(source, sourcePath, null);
30+
this(source, sourcePath, null, false);
3031
}
3132

3233
public RubySource(Source source, String sourcePath, Rope sourceRope) {
33-
assert RubyContext.getPath(source).equals(sourcePath) : RubyContext.getPath(source) + " vs " + sourcePath;
34+
this(source, sourcePath, sourceRope, false);
35+
}
3436

37+
public RubySource(Source source, String sourcePath, Rope sourceRope, boolean isEval) {
38+
assert RubyContext.getPath(source).equals(sourcePath) : RubyContext.getPath(source) + " vs " + sourcePath;
3539
this.source = Objects.requireNonNull(source);
3640
//intern() to improve footprint
3741
this.sourcePath = Objects.requireNonNull(sourcePath).intern();
3842
this.sourceRope = sourceRope;
43+
this.isEval = isEval;
3944
}
4045

4146
public Source getSource() {
@@ -50,4 +55,7 @@ public Rope getRope() {
5055
return sourceRope;
5156
}
5257

58+
public boolean isEval() {
59+
return isEval;
60+
}
5361
}

src/main/java/org/truffleruby/parser/TranslatorDriver.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,10 @@ public RubyRootNode parse(RubySource rubySource, ParserContext parserContext, St
233233

234234
// Translate to Ruby Truffle nodes
235235

236-
context.getCoverageManager().loadingSource(source);
236+
// Like MRI, do not track coverage of eval's. This also avoids having to merge multiple Sources with the same RubyContext.getPath().
237+
if (!rubySource.isEval()) {
238+
context.getCoverageManager().loadingSource(source);
239+
}
237240

238241
final BodyTranslator translator = new BodyTranslator(
239242
context,

src/main/java/org/truffleruby/stdlib/CoverageManager.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import org.truffleruby.RubyContext;
2222
import org.truffleruby.collections.ConcurrentOperations;
23-
import org.truffleruby.shared.TruffleRuby;
2423

2524
import com.oracle.truffle.api.CompilerDirectives;
2625
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -87,7 +86,6 @@ public synchronized void enable() {
8786
binding = instrumenter.attachExecutionEventFactory(
8887
SourceSectionFilter
8988
.newBuilder()
90-
.mimeTypeIs(TruffleRuby.MIME_TYPE)
9189
.sourceIs(coveredSources::contains)
9290
.tagIs(LineTag.class)
9391
.build(),

0 commit comments

Comments
 (0)