Skip to content

Commit 5370c56

Browse files
jensjohacommit-bot@chromium.org
authored andcommitted
[VM] Read and report constant constructor coverage from dill
This CL makes use of the now included constant constructor coverage in the dill file. It works like this: * When the CFE evaluates constants, every constant constructor invocation evaluated saves the reference to the constructor in the `Source` (from the Components uri to source table) for the callers Library. * This data is loaded into the VM in a "raw" format. * When a request for coverage comes in, the VM - on top of the normal coverage processing - goes through all scripts to find constant constructor coverage for the requested script and offset. Note that all scripts must be checked because library A can have evaluated a constructor from library B - so even if only coverage for library B was requested, library A has to be checked. For all constructors found the start and end position is reported as covered. Note that this does not mark any initializes and there are (at least currently) no good way of marking which initializes were evaluated (because it has to be stable across edits even when the `advanced invalidation feature` is enabled). * Note that the reason for the coverage to work on references - as hinted above - is because we want it to be stable across hot reloads even if/when advanced invalidation is enabled. This means, that library A cannot record "positional coverage" for library B because library B might get (for instance) new comments that will make any old offsets invalid. By using references we always lookup in the current world and use the correct offsets. #38934 TEST=Existing test suite, new tests for the new coverage added. Change-Id: I925963d1a9b9907efe621c72deb7348fa3be5ae8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171949 Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Ben Konyi <[email protected]> Reviewed-by: Vyacheslav Egorov <[email protected]>
1 parent 27ed791 commit 5370c56

20 files changed

+768
-12
lines changed

pkg/vm/test/incremental_compiler_test.dart

Lines changed: 254 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,9 @@ main() {
134134
/// If [getAllSources] is false it will ask specifically for report
135135
/// (and thus hits) for "lib1.dart" only.
136136
Future<Set<int>> collectAndCheckCoverageData(int port, bool getAllSources,
137-
{bool resume: true}) async {
137+
{bool resume: true,
138+
bool onGetAllVerifyCount: true,
139+
Set<int> coverageForLines}) async {
138140
RemoteVm remoteVm = new RemoteVm(port);
139141

140142
// Wait for the script to have finished.
@@ -189,7 +191,7 @@ main() {
189191
}
190192
i++;
191193
}
192-
if (getAllSources) {
194+
if (getAllSources && onGetAllVerifyCount) {
193195
expect(scriptIdToIndex.length >= 2, isTrue);
194196
}
195197

@@ -226,12 +228,16 @@ main() {
226228
}
227229
}
228230
for (int pos in coverage["misses"]) positions.add(pos);
229-
for (int pos in range["possibleBreakpoints"]) positions.add(pos);
231+
if (range["possibleBreakpoints"] != null) {
232+
for (int pos in range["possibleBreakpoints"]) positions.add(pos);
233+
}
230234
Map script = scriptIndexToScript[range["scriptIndex"]];
231235
Set<int> knownPositions = new Set<int>();
236+
Map<int, int> tokenPosToLine = {};
232237
if (script["tokenPosTable"] != null) {
233238
for (List tokenPosTableLine in script["tokenPosTable"]) {
234239
for (int i = 1; i < tokenPosTableLine.length; i += 2) {
240+
tokenPosToLine[tokenPosTableLine[i]] = tokenPosTableLine[0];
235241
knownPositions.add(tokenPosTableLine[i]);
236242
}
237243
}
@@ -244,6 +250,14 @@ main() {
244250
"line and column.");
245251
}
246252
}
253+
254+
if (coverageForLines != null) {
255+
for (int pos in coverage["hits"]) {
256+
if (lib1scriptIndices.contains(range["scriptIndex"])) {
257+
coverageForLines.add(tokenPosToLine[pos]);
258+
}
259+
}
260+
}
247261
}
248262
}
249263
}
@@ -486,6 +500,243 @@ main() {
486500
});
487501
});
488502

503+
group('multiple kernels constant coverage', () {
504+
Directory mytest;
505+
File main;
506+
File lib1;
507+
int lineForUnnamedConstructor;
508+
int lineForNamedConstructor;
509+
Process vm;
510+
setUpAll(() {
511+
mytest = Directory.systemTemp.createTempSync('incremental');
512+
main = new File('${mytest.path}/main.dart')..createSync();
513+
main.writeAsStringSync("""
514+
// This file - combined with the lib - should have coverage for both
515+
// constructors of Foo.
516+
import 'lib1.dart' as lib1;
517+
518+
void testFunction() {
519+
const foo = lib1.Foo.named();
520+
const foo2 = lib1.Foo.named();
521+
if (!identical(foo, foo2)) throw "what?";
522+
}
523+
524+
main() {
525+
lib1.testFunction();
526+
testFunction();
527+
print("main");
528+
}
529+
""");
530+
lib1 = new File('${mytest.path}/lib1.dart')..createSync();
531+
lib1.writeAsStringSync("""
532+
// Compiling this file should mark the default constructor - but not the
533+
// named constructor - as having coverage.
534+
class Foo {
535+
final int x;
536+
const Foo([int? x]) : this.x = x ?? 42;
537+
const Foo.named([int? x]) : this.x = x ?? 42;
538+
}
539+
540+
void testFunction() {
541+
const foo = Foo();
542+
const foo2 = Foo();
543+
if (!identical(foo, foo2)) throw "what?";
544+
}
545+
546+
main() {
547+
testFunction();
548+
print("lib1");
549+
}
550+
""");
551+
lineForUnnamedConstructor = 5;
552+
lineForNamedConstructor = 6;
553+
});
554+
555+
tearDownAll(() {
556+
try {
557+
mytest.deleteSync(recursive: true);
558+
} catch (_) {
559+
// Ignore errors;
560+
}
561+
try {
562+
vm.kill();
563+
} catch (_) {
564+
// Ignore errors;
565+
}
566+
});
567+
568+
Future<Set<int>> runAndGetLineCoverage(
569+
File list, String expectStdoutContains) async {
570+
vm = await Process.start(Platform.resolvedExecutable, <String>[
571+
"--pause-isolates-on-exit",
572+
"--enable-vm-service:0",
573+
"--disable-service-auth-codes",
574+
"--disable-dart-dev",
575+
list.path
576+
]);
577+
578+
const kObservatoryListening = 'Observatory listening on ';
579+
final RegExp observatoryPortRegExp =
580+
new RegExp("Observatory listening on http://127.0.0.1:\([0-9]*\)");
581+
int port;
582+
final splitter = new LineSplitter();
583+
Completer<String> portLineCompleter = new Completer<String>();
584+
Set<int> coverageLines = {};
585+
bool foundExpectedString = false;
586+
vm.stdout
587+
.transform(utf8.decoder)
588+
.transform(splitter)
589+
.listen((String s) async {
590+
if (s == expectStdoutContains) {
591+
foundExpectedString = true;
592+
}
593+
if (s.startsWith(kObservatoryListening)) {
594+
expect(observatoryPortRegExp.hasMatch(s), isTrue);
595+
final match = observatoryPortRegExp.firstMatch(s);
596+
port = int.parse(match.group(1));
597+
await collectAndCheckCoverageData(port, true,
598+
onGetAllVerifyCount: false, coverageForLines: coverageLines);
599+
if (!portLineCompleter.isCompleted) {
600+
portLineCompleter.complete("done");
601+
}
602+
}
603+
print("vm stdout: $s");
604+
});
605+
vm.stderr.transform(utf8.decoder).transform(splitter).listen((String s) {
606+
print("vm stderr: $s");
607+
});
608+
await portLineCompleter.future;
609+
print("Compiler terminated with ${await vm.exitCode} exit code");
610+
expect(foundExpectedString, isTrue);
611+
return coverageLines;
612+
}
613+
614+
test('compile seperatly, check coverage', () async {
615+
Directory dir = mytest.createTempSync();
616+
617+
// First compile lib, run and verify coverage (un-named constructor
618+
// covered, but not the named constructor).
619+
// Note that it's called 'lib1' to match with expectations from coverage
620+
// collector helper in this file.
621+
File libDill = File(p.join(dir.path, p.basename(lib1.path + ".dill")));
622+
IncrementalCompiler compiler = new IncrementalCompiler(options, lib1.uri);
623+
Component component = await compiler.compile();
624+
expect(component.libraries.length, equals(1));
625+
expect(component.libraries.single.fileUri, equals(lib1.uri));
626+
IOSink sink = libDill.openWrite();
627+
BinaryPrinter printer = new BinaryPrinter(sink);
628+
printer.writeComponentFile(component);
629+
await sink.flush();
630+
await sink.close();
631+
File list = new File(p.join(dir.path, 'dill.list'))..createSync();
632+
list.writeAsStringSync("#@dill\n${libDill.path}\n");
633+
Set<int> lineCoverage = await runAndGetLineCoverage(list, "lib1");
634+
// Expect coverage for unnamed constructor but not for the named one.
635+
expect(
636+
lineCoverage.intersection(
637+
{lineForUnnamedConstructor, lineForNamedConstructor}),
638+
equals({lineForUnnamedConstructor}));
639+
640+
try {
641+
vm.kill();
642+
} catch (_) {
643+
// Ignore errors;
644+
}
645+
// Accept the compile to not include the lib again.
646+
compiler.accept();
647+
648+
// Then compile lib, run and verify coverage (un-named constructor
649+
// covered, and the named constructor coveraged too).
650+
File mainDill = File(p.join(dir.path, p.basename(main.path + ".dill")));
651+
component = await compiler.compile(entryPoint: main.uri);
652+
expect(component.libraries.length, equals(1));
653+
expect(component.libraries.single.fileUri, equals(main.uri));
654+
sink = mainDill.openWrite();
655+
printer = new BinaryPrinter(sink);
656+
printer.writeComponentFile(component);
657+
await sink.flush();
658+
await sink.close();
659+
list.writeAsStringSync("#@dill\n${mainDill.path}\n${libDill.path}\n");
660+
lineCoverage = await runAndGetLineCoverage(list, "main");
661+
662+
// Expect coverage for both unnamed constructor and for the named one.
663+
expect(
664+
lineCoverage.intersection(
665+
{lineForUnnamedConstructor, lineForNamedConstructor}),
666+
equals({lineForUnnamedConstructor, lineForNamedConstructor}));
667+
668+
try {
669+
vm.kill();
670+
} catch (_) {
671+
// Ignore errors;
672+
}
673+
// Accept the compile to not include the lib again.
674+
compiler.accept();
675+
676+
// Finally, change lib to shift the constructors so the old line numbers
677+
// doesn't match. Compile lib by itself, compile lib, run with the old
678+
// main and verify coverage is still correct (both un-named constructor
679+
// and named constructor (at new line numbers) are covered, and the old
680+
// line numbers are not coverage.
681+
682+
lib1.writeAsStringSync("""
683+
//
684+
// Shift lines down by five
685+
// lines so the original
686+
// lines can't be coverred
687+
//
688+
class Foo {
689+
final int x;
690+
const Foo([int? x]) : this.x = x ?? 42;
691+
const Foo.named([int? x]) : this.x = x ?? 42;
692+
}
693+
694+
void testFunction() {
695+
const foo = Foo();
696+
const foo2 = Foo();
697+
if (!identical(foo, foo2)) throw "what?";
698+
}
699+
700+
main() {
701+
testFunction();
702+
print("lib1");
703+
}
704+
""");
705+
int newLineForUnnamedConstructor = 8;
706+
int newLineForNamedConstructor = 9;
707+
compiler.invalidate(lib1.uri);
708+
component = await compiler.compile(entryPoint: lib1.uri);
709+
expect(component.libraries.length, equals(1));
710+
expect(component.libraries.single.fileUri, equals(lib1.uri));
711+
sink = libDill.openWrite();
712+
printer = new BinaryPrinter(sink);
713+
printer.writeComponentFile(component);
714+
await sink.flush();
715+
await sink.close();
716+
list.writeAsStringSync("#@dill\n${mainDill.path}\n${libDill.path}\n");
717+
lineCoverage = await runAndGetLineCoverage(list, "main");
718+
719+
// Expect coverage for both unnamed constructor and for the named one on
720+
// the new positions, but no coverage on the old positions.
721+
expect(
722+
lineCoverage.intersection({
723+
lineForUnnamedConstructor,
724+
lineForNamedConstructor,
725+
newLineForUnnamedConstructor,
726+
newLineForNamedConstructor
727+
}),
728+
equals({newLineForUnnamedConstructor, newLineForNamedConstructor}));
729+
730+
try {
731+
vm.kill();
732+
} catch (_) {
733+
// Ignore errors;
734+
}
735+
// Accept the compile to not include the lib again.
736+
compiler.accept();
737+
});
738+
});
739+
489740
group('multiple kernels 2', () {
490741
Directory mytest;
491742
File main;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Copyright (c) 2020, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import "get_source_report_const_coverage_test.dart";
6+
7+
void testFunction() {
8+
const namedFoo = Foo.named3();
9+
const namedFoo2 = Foo.named3();
10+
const namedIdentical = identical(namedFoo, namedFoo2);
11+
print("namedIdentical: $namedIdentical");
12+
}

0 commit comments

Comments
 (0)