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.
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index a72ff2d..e0ea94c 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -159,6 +159,17 @@
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.
diff --git a/dev/tools/coverage.dart b/dev/tools/coverage.dart
deleted file mode 100644
index 87f54f3..0000000
--- a/dev/tools/coverage.dart
+++ /dev/null
@@ -1,64 +0,0 @@
-// 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;
-}
diff --git a/packages/flutter/test/animation/animation_controller_test.dart b/packages/flutter/test/animation/animation_controller_test.dart
index 9011253..7cb9788 100644
--- a/packages/flutter/test/animation/animation_controller_test.dart
+++ b/packages/flutter/test/animation/animation_controller_test.dart
@@ -176,17 +176,17 @@
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();
});
}
diff --git a/packages/flutter/test/animation/animations_test.dart b/packages/flutter/test/animation/animations_test.dart
index 6217fd8..7f3a3e9 100644
--- a/packages/flutter/test/animation/animations_test.dart
+++ b/packages/flutter/test/animation/animations_test.dart
@@ -13,16 +13,16 @@
});
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 @@
curve: Curves.ease,
reverseCurve: Curves.elasticOut
);
- expect(curvedAnimation.toString(), isOneLineDescription);
+ expect(curvedAnimation.toString(), hasOneLineDescription);
controller.stop();
});
@@ -42,9 +42,9 @@
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 @@
expect(didReceiveCallback, isFalse);
controller.value = 0.7;
expect(didReceiveCallback, isFalse);
- expect(animation.toString(), isOneLineDescription);
+ expect(animation.toString(), hasOneLineDescription);
});
test('TrainHoppingAnimation', () {
@@ -96,11 +96,11 @@
});
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'));
});
}
diff --git a/packages/flutter/test/animation/curves_test.dart b/packages/flutter/test/animation/curves_test.dart
new file mode 100644
index 0000000..e99c51b
--- /dev/null
+++ b/packages/flutter/test/animation/curves_test.dart
@@ -0,0 +1,34 @@
+// 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);
+ });
+}
diff --git a/packages/flutter_test/lib/src/matchers.dart b/packages/flutter_test/lib/src/matchers.dart
index d076bff..ace7741 100644
--- a/packages/flutter_test/lib/src/matchers.dart
+++ b/packages/flutter_test/lib/src/matchers.dart
@@ -51,11 +51,11 @@
/// [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 @@
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;
diff --git a/packages/flutter_tools/lib/src/commands/test.dart b/packages/flutter_tools/lib/src/commands/test.dart
index d94b7a4..534d10c 100644
--- a/packages/flutter_tools/lib/src/commands/test.dart
+++ b/packages/flutter_tools/lib/src/commands/test.dart
@@ -9,6 +9,7 @@
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 @@
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 @@
}
}
+ 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 @@
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;
