pkg/vm_service_client: add getSourceReport to VMServiceReference

test/source_report_test.dart

Index: test/source_report_test.dart
diff --git a/test/source_report_test.dart b/test/source_report_test.dart
new file mode 100644
index 0000000000000000000000000000000000000000..7fdf8e52c385332ac7772096d7f898e08a482045
--- /dev/null
+++ b/test/source_report_test.dart
@@ -0,0 +1,218 @@
+// Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:source_span/source_span.dart';
+import 'package:test/test.dart';
+import 'package:vm_service_client/vm_service_client.dart';
+
+import 'utils.dart';
+
+const _mainContent = r"""
+print("one");

[nweiz, 2016/05/04 23:05:52]

What is going on with this indentation? Why are the two prints indented
differently?

Either the quotes should be treated like brackets, in which case everything
should be indented two spaces and the closing quotes should be at the base
level; or this should be treated as a normal multiline string rather than a code
block, in which case its base indentation should be 0. I prefer the former.

[kevmoo, 2016/05/05 20:46:27]

Right?

+  print("two");
+
+  if (false) {
+    print("three");
+    print("four");
+  }""";
+
+void main() {
+  VMServiceClient client;
+  VMIsolateRef isolate;
+
+  tearDown(() {
+    if (client != null) client.close();
+  });
+
+  group('a script with one range', () {
+    setUp(() async {
+      client = await runAndConnect(main: _mainContent);
+
+      isolate = (await client.getVM()).isolates.single;
+
+      await isolate.pause();
+    });
+
+    test("and no options", () async {

[nweiz, 2016/05/04 23:05:52]

The group name plus the test name should be a sentence describing the behavior
you're testing.

[kevmoo, 2016/05/05 20:46:28]

Done.

[nweiz, 2016/05/11 23:03:31]

"behaves correctly" doesn't actually describe any behavior. 

+      var report = await isolate.getSourceReport();
+
+      expect(report.ranges, hasLength(greaterThan(1)));
+
+      var range = report.ranges.singleWhere((range) =>
+          range.script.uri.toString().startsWith('data:application/dart'));
+
+      expect(range.compiled, isTrue);
+
+      var script = await range.script.load();
+
+      var startLocation = script.sourceLocation(range.location.token);
+      expect(startLocation.line, 4);
+      expect(startLocation.column, 0);

[nweiz, 2016/05/04 23:05:52]

Testing the explicit line and column is brittle, and it's not obvious to a
reader that these numbers are correct. You can get the actual source location of
the main method through isolate.rootLibrary (see the script tests I added). Then
you can just check that the corresponding source_span SourceLocations are ==.

[kevmoo, 2016/05/05 20:46:27]

Done.

+
+      var endLocation = script.sourceLocation(range.location.end);
+      expect(endLocation.line, 14);
+      expect(endLocation.column, 0);
+
+      expect(range.hits, isNull);
+      expect(range.misses, isNull);
+      expect(range.possibleBreakpoints, isNull);
+    });
+
+    test("including coverage", () async {
+      var report = await isolate.getSourceReport(includeCoverageReport: true);
+
+      var range = report.ranges.singleWhere((range) =>
+          range.script.uri.toString().startsWith('data:application/dart'));
+      expect(range.possibleBreakpoints, isNull);
+
+      var script = await range.script.load();
+
+      var hitLocations =
+          range.hits.map((token) => script.sourceLocation(token).line);
+
+      expect(hitLocations, [6, 7, 8, 14]);

[nweiz, 2016/05/04 23:05:52]

Add a comment explaining what actual lines these represent. Also below.

[kevmoo, 2016/05/05 20:46:27]

Done.

+
+      var missLocations =
+          range.misses.map((token) => script.sourceLocation(token).line);
+
+      expect(missLocations, [11, 12]);
+    });
+
+    test("including possible breakpoints", () async {
+      var report =
+          await isolate.getSourceReport(includePossibleBreakpoints: true);
+
+      var range = report.ranges.singleWhere((range) =>
+          range.script.uri.toString().startsWith('data:application/dart'));
+
+      expect(range.hits, isNull);
+      expect(range.misses, isNull);
+
+      var script = await range.script.load();
+
+      expect(range.possibleBreakpoints, isNotEmpty);
+
+      var breakPointLines = range.possibleBreakpoints
+          .map((token) => script.sourceLocation(token).line);
+
+      expect(breakPointLines, [4, 6, 7, 8, 11, 12, 14]);
+    });
+
+    test("including coverage and possible breakpoints", () async {
+      var report = await isolate.getSourceReport(
+          includeCoverageReport: true, includePossibleBreakpoints: true);
+
+      var range = report.ranges.singleWhere((range) =>
+          range.script.uri.toString().startsWith('data:application/dart'));
+
+      expect(range.hits, isNotEmpty);
+      expect(range.misses, isNotEmpty);
+      expect(range.possibleBreakpoints, isNotEmpty);
+    });
+  });
+
+  group('a script with multiple ranges', () {
+    VMScript script;
+    VMLibrary rootLib;
+    VMSourceLocation mainFunctionLocation;

[nweiz, 2016/05/04 23:05:52]

I think just "mainLocation" would be fine here.

[kevmoo, 2016/05/05 20:46:28]

Done.

[nweiz, 2016/05/11 23:03:31]

This comment was intended to also apply to other similarly-named variables.

+    FileSpan mainFunctionSpan;
+    VMSourceLocation unusedFunc2Location;

[nweiz, 2016/05/04 23:05:52]

"Func" -> "Function"

[kevmoo, 2016/05/05 20:46:27]

Done.

+
+    setUp(() async {
+      client = await runAndConnect(
+          topLevel: r'''final unusedField = 5;
+
+int unusedFunction(a, b) {
+  return a + b;
+}
+
+void unusedFunction2(value) {
+  print(value);
+}''',
+          main: _mainContent);
+
+      isolate = (await client.getVM()).isolates.single;
+
+      await isolate.pause();
+
+      var runnableIsolate = await isolate.loadRunnable();
+      rootLib = await runnableIsolate.rootLibrary.load();
+      script = await rootLib.scripts.single.load();
+
+      var mainFunc = await rootLib.functions['main'].load();

[nweiz, 2016/05/04 23:05:53]

"Func" -> "Function"

[kevmoo, 2016/05/05 20:46:27]

Done.

+      mainFunctionLocation = mainFunc.location;
+      mainFunctionSpan = script.sourceSpan(mainFunctionLocation);
+
+      var unusedFunction2 = await rootLib.functions['unusedFunction2'].load();
+      unusedFunc2Location = unusedFunc2Location = unusedFunction2.location;

[nweiz, 2016/05/04 23:05:52]

Extra assignment.

[kevmoo, 2016/05/05 20:46:28]

Done.

+    });
+
+    test("not force compiled", () async {
+      var report = await script.getSourceReport(
+          includeCoverageReport: true, includePossibleBreakpoints: true);
+
+      expect(report.ranges, hasLength(3));
+
+      var firstRange = report.ranges.first;
+      expect(firstRange.compiled, isFalse);

[nweiz, 2016/05/04 23:05:53]

Make assertions about the other properties as well. Also below.

[kevmoo, 2016/05/05 20:46:28]

Done.

+      var firstRangeLocation =

[nweiz, 2016/05/04 23:05:53]

Why is this called "Location" if it's an offset?

[kevmoo, 2016/05/05 20:46:27]

Acknowledged.

+          script.sourceLocation(firstRange.location.end).offset;
+      expect(firstRangeLocation, lessThan(mainFunctionSpan.start.offset));

[nweiz, 2016/05/04 23:05:53]

You can just do

  expect(script.sourceSpan(firstRange.location), lessThan(mainFunctionSpan)))

same below.

[kevmoo, 2016/05/05 20:46:27]

Sadly, `lessThan` only uses the `>` operator.

We should fix that...

+
+      var lastRange = report.ranges.last;
+      expect(lastRange.compiled, isTrue);
+      var lastRangeOffset =
+          script.sourceLocation(lastRange.location.token).offset;
+      expect(lastRangeOffset, mainFunctionSpan.start.offset);
+    });
+
+    test("force compiled", () async {
+      var report = await script.getSourceReport(
+          includeCoverageReport: true,
+          includePossibleBreakpoints: true,
+          forceCompile: true);
+
+      expect(report.ranges, hasLength(3));
+
+      var firstRange = report.ranges.first;
+      expect(firstRange.compiled, isTrue);
+
+      var secondRange = report.ranges.last;
+      expect(secondRange.compiled, isTrue);
+    });
+
+    test("with tokenPos set", () async {

[nweiz, 2016/05/04 23:05:52]

It's not called "tokenPos" (or "endTokenPos") anymore.

[kevmoo, 2016/05/05 20:46:27]

Acknowledged.

+      var report = await script.getSourceReport(
+          includeCoverageReport: true,
+          includePossibleBreakpoints: true,
+          start: mainFunctionLocation.token);
+
+      var rangeOffset =
+          script.sourceLocation(report.ranges.single.location.token).offset;
+
+      expect(rangeOffset, mainFunctionSpan.start.offset);
+    });
+
+    test("with endTokenPos set", () async {
+      var report = await script.getSourceReport(
+          includeCoverageReport: true,
+          includePossibleBreakpoints: true,
+          end: unusedFunc2Location.token);
+
+      expect(report.ranges, hasLength(2),
+          reason: 'Includes the range containing only unusedFunction2, main.');
+    });
+
+    test("with an empty token range", () async {
+      var report = await script.getSourceReport(
+          includeCoverageReport: true,
+          includePossibleBreakpoints: true,
+          start: unusedFunc2Location.token,
+          end: unusedFunc2Location.end);
+
+      expect(report.ranges, hasLength(1),
+          reason: 'Includes the range containing only unusedFunction2.');

[nweiz, 2016/05/04 23:05:52]

Assert that this is in fact the range it contains.

[kevmoo, 2016/05/05 20:46:28]

Done.

+    });
+  });
+}