Add a simple way of merging coverage data (#4726)

`flutter test` now has a `--merge-coverage` flag that can be used to merge
coverage data from previous runs, enabling faster iteration cycles.

CONTRIBUTING.md

@@ -159,6 +159,17 @@ If you don't see any coverage data, check that you have an `lcov.info` file in
 the `packages/flutter/coverage` directory. It should have been downloaded by the
 `flutter update-packages` command you ran previously.
 
+If you want to iterate quickly on improving test coverage, consider using this
+workflow:
+
+ * Open a file and observe that some line is untested.
+ * Write a test that exercises that line.
+ * Run `flutter test --merge-coverage path/to/your/test_test.dart`.
+ * After the test passes, observe that the line is now tested.
+
+This workflow merges the coverage data from this test run with the base coverage
+data downloaded by `flutter update-packages`.
+
 See [issue 4719](https://github.com/flutter/flutter/issues/4719) for ideas about
 how to improve this workflow.
 

dev/tools/coverage.dart

-// Copyright 2016 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-// Downloads and merges line coverage data files for package:flutter.
-
-import 'dart:async';
-import 'dart:io';
-
-import 'package:args/args.dart';
-import 'package:path/path.dart' as path;
-
-const String kBaseLcov = 'packages/flutter/coverage/lcov.base.info';
-const String kTargetLcov = 'packages/flutter/coverage/lcov.info';
-const String kSourceLcov = 'packages/flutter/coverage/lcov.source.info';
-
-Future<int> main(List<String> args) async {
-  if (path.basename(Directory.current.path) == 'tools')
-    Directory.current = Directory.current.parent.parent;
-
-  ProcessResult result = Process.runSync('which', <String>['lcov']);
-  if (result.exitCode != 0) {
-    print('Cannot find lcov. Consider running "apt-get install lcov".\n');
-    return 1;
-  }
-
-  if (!FileSystemEntity.isFileSync(kBaseLcov)) {
-    print(
-      'Cannot find "$kBaseLcov". Consider downloading it from from cloud storage.\n'
-      'https://storage.googleapis.com/flutter_infra/flutter/coverage/lcov.info\n'
-    );
-    return 1;
-  }
-
-  ArgParser argParser = new ArgParser();
-  argParser.addFlag('merge', negatable: false);
-  ArgResults results = argParser.parse(args);
-
-  if (FileSystemEntity.isFileSync(kTargetLcov)) {
-    if (results['merge']) {
-      new File(kTargetLcov).renameSync(kSourceLcov);
-    } else {
-      print('"$kTargetLcov" already exists. Did you want to --merge?\n');
-      return 1;
-    }
-  }
-
-  if (results['merge']) {
-    if (!FileSystemEntity.isFileSync(kSourceLcov)) {
-      print('Cannot merge because "$kSourceLcov" does not exist.\n');
-      return 1;
-    }
-
-    ProcessResult result = Process.runSync('lcov', <String>[
-      '--add-tracefile', kBaseLcov,
-      '--add-tracefile', kSourceLcov,
-      '--output-file', kTargetLcov,
-    ]);
-    return result.exitCode;
-  }
-
-  print('No operation requested. Did you want to --merge?\n');
-  return 0;
-}

packages/flutter/test/animation/animation_controller_test.dart

@@ -176,17 +176,17 @@ void main() {
     AnimationController controller = new AnimationController(
       duration: const Duration(milliseconds: 100)
     );
-    expect(controller.toString(), isOneLineDescription);
+    expect(controller.toString(), hasOneLineDescription);
     controller.forward();
     WidgetsBinding.instance.handleBeginFrame(const Duration(milliseconds: 20));
     WidgetsBinding.instance.handleBeginFrame(const Duration(milliseconds: 30));
-    expect(controller.toString(), isOneLineDescription);
+    expect(controller.toString(), hasOneLineDescription);
     WidgetsBinding.instance.handleBeginFrame(const Duration(milliseconds: 120));
-    expect(controller.toString(), isOneLineDescription);
+    expect(controller.toString(), hasOneLineDescription);
     controller.reverse();
     WidgetsBinding.instance.handleBeginFrame(const Duration(milliseconds: 20));
     WidgetsBinding.instance.handleBeginFrame(const Duration(milliseconds: 30));
-    expect(controller.toString(), isOneLineDescription);
+    expect(controller.toString(), hasOneLineDescription);
     controller.stop();
   });
 }

packages/flutter/test/animation/animations_test.dart

@@ -13,16 +13,16 @@ void main() {
   });
 
   test('toString control test', () {
-    expect(kAlwaysCompleteAnimation.toString(), isOneLineDescription);
-    expect(kAlwaysDismissedAnimation.toString(), isOneLineDescription);
-    expect(new AlwaysStoppedAnimation<double>(0.5).toString(), isOneLineDescription);
+    expect(kAlwaysCompleteAnimation.toString(), hasOneLineDescription);
+    expect(kAlwaysDismissedAnimation.toString(), hasOneLineDescription);
+    expect(new AlwaysStoppedAnimation<double>(0.5).toString(), hasOneLineDescription);
     CurvedAnimation curvedAnimation = new CurvedAnimation(
       parent: kAlwaysDismissedAnimation,
       curve: Curves.ease
     );
-    expect(curvedAnimation.toString(), isOneLineDescription);
+    expect(curvedAnimation.toString(), hasOneLineDescription);
     curvedAnimation.reverseCurve = Curves.elasticOut;
-    expect(curvedAnimation.toString(), isOneLineDescription);
+    expect(curvedAnimation.toString(), hasOneLineDescription);
     AnimationController controller = new AnimationController(
       duration: const Duration(milliseconds: 500)
     );
@@ -34,7 +34,7 @@ void main() {
       curve: Curves.ease,
       reverseCurve: Curves.elasticOut
     );
-    expect(curvedAnimation.toString(), isOneLineDescription);
+    expect(curvedAnimation.toString(), hasOneLineDescription);
     controller.stop();
   });
 
@@ -42,9 +42,9 @@ void main() {
     ProxyAnimation animation = new ProxyAnimation();
     expect(animation.value, 0.0);
     expect(animation.status, AnimationStatus.dismissed);
-    expect(animation.toString(), isOneLineDescription);
+    expect(animation.toString(), hasOneLineDescription);
     animation.parent = kAlwaysDismissedAnimation;
-    expect(animation.toString(), isOneLineDescription);
+    expect(animation.toString(), hasOneLineDescription);
   });
 
   test('ProxyAnimation set parent generates value changed', () {
@@ -81,7 +81,7 @@ void main() {
     expect(didReceiveCallback, isFalse);
     controller.value = 0.7;
     expect(didReceiveCallback, isFalse);
-    expect(animation.toString(), isOneLineDescription);
+    expect(animation.toString(), hasOneLineDescription);
   });
 
   test('TrainHoppingAnimation', () {
@@ -96,11 +96,11 @@ void main() {
       });
     expect(didSwitchTrains, isFalse);
     expect(animation.value, 0.5);
-    expect(animation.toString(), isOneLineDescription);
+    expect(animation.toString(), hasOneLineDescription);
     nextTrain.value = 0.25;
     expect(didSwitchTrains, isTrue);
     expect(animation.value, 0.25);
-    expect(animation.toString(), isOneLineDescription);
+    expect(animation.toString(), hasOneLineDescription);
     expect(animation.toString(), contains('no next'));
   });
 }

packages/flutter/test/animation/curves_test.dart

+// Copyright 2016 The Chromium Authors. 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:flutter_test/flutter_test.dart';
+import 'package:flutter/animation.dart';
+import 'package:flutter/widgets.dart';
+
+void main() {
+  test('toString control test', () {
+    expect(Curves.linear, hasOneLineDescription);
+    expect(new SawTooth(3), hasOneLineDescription);
+    expect(new Interval(0.25, 0.75), hasOneLineDescription);
+    expect(new Interval(0.25, 0.75, curve: Curves.ease), hasOneLineDescription);
+  });
+
+  test('Curve flipped control test', () {
+    Curve ease = Curves.ease;
+    Curve flippedEase = ease.flipped;
+    expect(flippedEase.transform(0.0), lessThan(0.001));
+    expect(flippedEase.transform(0.5), lessThan(ease.transform(0.5)));
+    expect(flippedEase.transform(1.0), greaterThan(0.999));
+    expect(flippedEase, hasOneLineDescription);
+  });
+
+  test('Step has a step', () {
+    Curve step = new Step(0.25);
+    expect(step.transform(0.0), 0.0);
+    expect(step.transform(0.24), 0.0);
+    expect(step.transform(0.25), 1.0);
+    expect(step.transform(0.26), 1.0);
+    expect(step.transform(1.0), 1.0);
+  });
+}

packages/flutter_test/lib/src/matchers.dart

@@ -51,11 +51,11 @@ const Matcher isInCard = const _IsInCard();
 /// [Card] widget ancestors.
 const Matcher isNotInCard = const _IsNotInCard();
 
-/// Asserts that a string is a plausible one-line description of an object.
+/// Asserts that an object's toString() is a plausible one-line description.
 ///
 /// Specifically, this matcher checks that the string does not contains newline
 /// characters and does not have leading or trailing whitespace.
-const Matcher isOneLineDescription = const _IsOneLineDescription();
+const Matcher hasOneLineDescription = const _HasOneLineDescription();
 
 class _FindsWidgetMatcher extends Matcher {
   const _FindsWidgetMatcher(this.min, this.max);
@@ -189,11 +189,12 @@ class _IsNotInCard extends Matcher {
   Description describe(Description description) => description.add('not in card');
 }
 
-class _IsOneLineDescription extends Matcher {
-  const _IsOneLineDescription();
+class _HasOneLineDescription extends Matcher {
+  const _HasOneLineDescription();
 
   @override
-  bool matches(String description, Map<dynamic, dynamic> matchState) {
+  bool matches(Object object, Map<dynamic, dynamic> matchState) {
+    String description = object.toString();
     return description.isNotEmpty &&
         !description.contains('\n') &&
         description.trim() == description;

packages/flutter_tools/lib/src/commands/test.dart

@@ -9,6 +9,7 @@ import 'package:path/path.dart' as path;
 import 'package:test/src/executable.dart' as executable; // ignore: implementation_imports
 
 import '../base/logger.dart';
+import '../base/os.dart';
 import '../cache.dart';
 import '../dart/package_map.dart';
 import '../globals.dart';
@@ -22,8 +23,15 @@ class TestCommand extends FlutterCommand {
     usesPubOption();
     argParser.addFlag('coverage',
       defaultsTo: false,
+      negatable: false,
       help: 'Whether to collect coverage information.'
     );
+    argParser.addFlag('merge-coverage',
+      defaultsTo: false,
+      negatable: false,
+      help: 'Whether to merge converage data with "coverage/lcov.base.info". '
+            'Implies collecting coverage data. (Linux only)'
+    );
     argParser.addOption('coverage-path',
       defaultsTo: 'coverage/lcov.info',
       help: 'Where to store coverage information (if coverage is enabled).'
@@ -83,6 +91,57 @@ class TestCommand extends FlutterCommand {
     }
   }
 
+  Future<bool> _collectCoverageData(CoverageCollector collector, { bool mergeCoverageData: false }) async {
+    Status status = logger.startProgress('Collecting coverage information...');
+    String coverageData = await collector.finalizeCoverage();
+    status.stop(showElapsedTime: true);
+
+    String coveragePath = argResults['coverage-path'];
+    File coverageFile = new File(coveragePath)
+      ..createSync(recursive: true)
+      ..writeAsStringSync(coverageData, flush: true);
+    printTrace('wrote coverage data to $coveragePath (size=${coverageData.length})');
+
+    String baseCoverageData = 'coverage/lcov.base.info';
+    if (mergeCoverageData) {
+      if (!os.isLinux) {
+        printError(
+          'Merging coverage data is supported only on Linux because it '
+          'requires the "lcov" tool.'
+        );
+        return false;
+      }
+
+      if (!FileSystemEntity.isFileSync(baseCoverageData)) {
+        printError('Missing "$baseCoverageData". Unable to merge coverage data.');
+        return false;
+      }
+
+      if (os.which('lcov') == null) {
+        printError(
+          'Missing "lcov" tool. Unable to merge coverage data.\n'
+          'Consider running "sudo apt-get install lcov".'
+        );
+        return false;
+      }
+
+      Directory tempDir = Directory.systemTemp.createTempSync('flutter_tools');
+      try {
+        File sourceFile = coverageFile.copySync(path.join(tempDir.path, 'lcov.source.info'));
+        ProcessResult result = Process.runSync('lcov', <String>[
+          '--add-tracefile', baseCoverageData,
+          '--add-tracefile', sourceFile.path,
+          '--output-file', coverageFile.path,
+        ]);
+        if (result.exitCode != 0)
+          return false;
+      } finally {
+        tempDir.deleteSync(recursive: true);
+      }
+    }
+    return true;
+  }
+
   @override
   Future<int> runInProject() async {
     List<String> testArgs = argResults.rest.map((String testPath) => path.absolute(testPath)).toList();
@@ -119,20 +178,13 @@ class TestCommand extends FlutterCommand {
     Cache.releaseLockEarly();
 
     CoverageCollector collector = CoverageCollector.instance;
-    collector.enabled = argResults['coverage'];
+    collector.enabled = argResults['coverage'] || argResults['merge-coverage'];
 
     int result = await _runTests(testArgs, testDir);
 
     if (collector.enabled) {
-      Status status = logger.startProgress("Collecting coverage information...");
-      String coverageData = await collector.finalizeCoverage();
-      status.stop(showElapsedTime: true);
-
-      String coveragePath = argResults['coverage-path'];
-      new File(coveragePath)
-        ..createSync(recursive: true)
-        ..writeAsStringSync(coverageData, flush: true);
-      printTrace('wrote coverage data to $coveragePath (size=${coverageData.length})');
+      if (!await _collectCoverageData(collector, mergeCoverageData: argResults['merge-coverage']))
+        return 1;
     }
 
     return result;