Skip to content

Commit afb2160

Browse files
mrooneytrimblejavax-swing
authored andcommitted
Fix the issue with coverage that was caused by information being lost with a TypeApply
Add the ability to verify measured coverage invocations in CoverageTests
1 parent 3212890 commit afb2160

File tree

7 files changed

+184
-4
lines changed

7 files changed

+184
-4
lines changed

Diff for: compiler/src/dotty/tools/dotc/transform/InstrumentCoverage.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,8 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
237237
val InstrumentedParts(pre, coverageCall, expr) = tryInstrument(fun)
238238

239239
if coverageCall.isEmpty then
240-
// `fun` cannot be instrumented, and `args` is a type so we keep this tree as it is
241-
tree
240+
// `fun` cannot be instrumented and `args` is a type, but `expr` may have been transformed
241+
cpy.TypeApply(tree)(expr, args)
242242
else
243243
// expr[T] shouldn't be transformed to:
244244
// {invoked(...), expr}[T]

Diff for: compiler/test/dotty/tools/dotc/coverage/CoverageTests.scala

+28
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,26 @@ class CoverageTests:
6161
val instructions = FileDiff.diffMessage(expectFile.toString, targetFile.toString)
6262
fail(s"Coverage report differs from expected data.\n$instructions")
6363

64+
// measurement files only exist in the "run" category
65+
// as these are generated at runtime by the scala.runtime.coverage.Invoker
66+
val expectMeasurementFile = path.resolveSibling(s"$fileName.measurement.check")
67+
if run && Files.exists(expectMeasurementFile) then
68+
69+
// Note that this assumes that the test invoked was single threaded,
70+
// if that is not the case then this will have to be adjusted
71+
val targetMeasurementFile = findMeasurementFile(targetDir)
72+
73+
if updateCheckFiles then
74+
Files.copy(targetMeasurementFile, expectMeasurementFile, StandardCopyOption.REPLACE_EXISTING)
75+
76+
else
77+
val targetMeasurementFile = findMeasurementFile(targetDir)
78+
val expectedMeasurements = fixWindowsPaths(Files.readAllLines(expectMeasurementFile).asScala)
79+
val obtainedMeasurements = fixWindowsPaths(Files.readAllLines(targetMeasurementFile).asScala)
80+
if expectedMeasurements != obtainedMeasurements then
81+
val instructions = FileDiff.diffMessage(expectMeasurementFile.toString, targetMeasurementFile.toString)
82+
fail(s"Measurement report differs from expected data.\n$instructions")
83+
()
6484
})
6585

6686
/** Generates the coverage report for the given input file, in a temporary directory. */
@@ -75,6 +95,14 @@ class CoverageTests:
7595
test.checkCompile()
7696
target
7797

98+
private def findMeasurementFile(targetDir: Path): Path = {
99+
val allFilesInTarget = Files.list(targetDir).toList.asScala
100+
allFilesInTarget.filter(_.getFileName.toString.startsWith("scoverage.measurements.")).headOption.getOrElse(
101+
throw new AssertionError(s"Expected to find measurement file in targetDir [${targetDir}] but none were found.")
102+
)
103+
}
104+
105+
78106
object CoverageTests extends ParallelTesting:
79107
import scala.concurrent.duration.*
80108

Diff for: tests/coverage/run/type-apply/test.check

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
List(List(1), List(2), List(3))

Diff for: tests/coverage/run/type-apply/test.measurement.check

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
6
2+
1
3+
3
4+
2
5+
5
6+
4
7+
0

Diff for: tests/coverage/run/type-apply/test.scala

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
@main
2+
def Test: Unit =
3+
// verifies a problematic case where the TypeApply instrumentation was added to the coverage file,
4+
// but was never marked as invoked
5+
println(List(1,2,3).map(a => List(a)))

Diff for: tests/coverage/run/type-apply/test.scoverage.check

+139
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
# Coverage data, format version: 3.0
2+
# Statement data:
3+
# - id
4+
# - source path
5+
# - package name
6+
# - class name
7+
# - class type (Class, Object or Trait)
8+
# - full class name
9+
# - method name
10+
# - start offset
11+
# - end offset
12+
# - line number
13+
# - symbol name
14+
# - tree name
15+
# - is branch
16+
# - invocations count
17+
# - is ignored
18+
# - description (can be multi-line)
19+
# ' ' sign
20+
# ------------------------------------------
21+
0
22+
type-apply/test.scala
23+
<empty>
24+
test$package$
25+
Object
26+
<empty>.test$package$
27+
Test
28+
163
29+
201
30+
4
31+
println
32+
Apply
33+
false
34+
0
35+
false
36+
println(List(1,2,3).map(a => List(a)))
37+
38+
1
39+
type-apply/test.scala
40+
<empty>
41+
test$package$
42+
Object
43+
<empty>.test$package$
44+
Test
45+
171
46+
200
47+
4
48+
map
49+
Apply
50+
false
51+
0
52+
false
53+
List(1,2,3).map(a => List(a))
54+
55+
2
56+
type-apply/test.scala
57+
<empty>
58+
test$package$
59+
Object
60+
<empty>.test$package$
61+
Test
62+
171
63+
182
64+
4
65+
apply
66+
Apply
67+
false
68+
0
69+
false
70+
List(1,2,3)
71+
72+
3
73+
type-apply/test.scala
74+
<empty>
75+
test$package$
76+
Object
77+
<empty>.test$package$
78+
Test
79+
171
80+
175
81+
4
82+
List
83+
Ident
84+
false
85+
0
86+
false
87+
List
88+
89+
4
90+
type-apply/test.scala
91+
<empty>
92+
test$package$
93+
Object
94+
<empty>.test$package$
95+
$anonfun
96+
192
97+
199
98+
4
99+
apply
100+
Apply
101+
false
102+
0
103+
false
104+
List(a)
105+
106+
5
107+
type-apply/test.scala
108+
<empty>
109+
test$package$
110+
Object
111+
<empty>.test$package$
112+
$anonfun
113+
192
114+
196
115+
4
116+
List
117+
Ident
118+
false
119+
0
120+
false
121+
List
122+
123+
6
124+
type-apply/test.scala
125+
<empty>
126+
test$package$
127+
Object
128+
<empty>.test$package$
129+
Test
130+
0
131+
14
132+
1
133+
Test
134+
DefDef
135+
false
136+
0
137+
false
138+
@main\ndef Test
139+

Diff for: tests/pos-with-compiler-cc/dotc/transform/InstrumentCoverage.scala

+2-2
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,8 @@ class InstrumentCoverage extends MacroTransform with IdentityDenotTransformer:
236236
val InstrumentedParts(pre, coverageCall, expr) = tryInstrument(fun)
237237

238238
if coverageCall.isEmpty then
239-
// `fun` cannot be instrumented, and `args` is a type so we keep this tree as it is
240-
tree
239+
// `fun` cannot be instrumented and `args` is a type, but `expr` may have been transformed
240+
cpy.TypeApply(tree)(expr, args)
241241
else
242242
// expr[T] shouldn't be transformed to:
243243
// {invoked(...), expr}[T]

0 commit comments

Comments
 (0)