Enhance coverage to cover all appropriate binary-level tests (#1879)
* Cleanup and move test-only code into the test utils library
* Fix coverage for binary-level tests
* dartfmt
* Adjust timeout factor
* Unconditionally add coverage path environment variable
* Move main up to the top; coverage runs take a bit more time
diff --git a/.travis.yml b/.travis.yml
index 4e638d3..5493fca 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -4,9 +4,9 @@
- stable
- "dev/raw/latest"
env:
+ - DARTDOC_BOT=main
- DARTDOC_BOT=flutter
- DARTDOC_BOT=sdk-analyzer
- - DARTDOC_BOT=main
- DARTDOC_BOT=packages
- DARTDOC_BOT=sdk-docs
script: ./tool/travis.sh
diff --git a/lib/src/io_utils.dart b/lib/src/io_utils.dart
index 3c99b3a..9a79602 100644
--- a/lib/src/io_utils.dart
+++ b/lib/src/io_utils.dart
@@ -80,7 +80,6 @@
final partOfRegexp = new RegExp('part of ');
final newLinePartOfRegexp = new RegExp('\npart of ');
-
/// Best used with Future<void>.
class MultiFutureTracker<T> {
/// Approximate maximum number of simultaneous active Futures.
@@ -113,5 +112,3 @@
/// Wait until all futures added so far have completed.
Future<void> wait() async => await _waitUntil(0);
}
-
-
diff --git a/lib/src/model.dart b/lib/src/model.dart
index dbbce53..13bace5 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -464,8 +464,8 @@
@override
void warn(PackageWarning kind,
{String message,
- Iterable<Locatable> referredFrom,
- Iterable<String> extendedDebug}) {
+ Iterable<Locatable> referredFrom,
+ Iterable<String> extendedDebug}) {
enclosingCombo.warn(kind,
message: message,
referredFrom: referredFrom,
@@ -2031,7 +2031,8 @@
if (writeOnly) return r'←';
// ↔
if (readWrite) return r'↔';
- throw UnsupportedError('GetterSetterCombo must be one of readOnly, writeOnly, or readWrite');
+ throw UnsupportedError(
+ 'GetterSetterCombo must be one of readOnly, writeOnly, or readWrite');
}
bool get readOnly => hasPublicGetter && !hasPublicSetter;
diff --git a/test/compare_output_test.dart b/test/compare_output_test.dart
index 301a95b..4f80922 100644
--- a/test/compare_output_test.dart
+++ b/test/compare_output_test.dart
@@ -12,6 +12,8 @@
import 'package:path/path.dart' as pathLib;
import 'package:test/test.dart';
+import 'src/utils.dart';
+
const List<String> _filesToIgnore = const <String>['.DS_Store'];
const String gitBinName = 'git';
@@ -40,10 +42,20 @@
void main() {
group('compare outputs', () {
Directory tempDir;
+ CoverageSubprocessLauncher subprocessLauncher;
var dartdocBin =
pathLib.fromUri(_currentFileUri.resolve('../bin/dartdoc.dart'));
+ setUpAll(() {
+ subprocessLauncher =
+ new CoverageSubprocessLauncher('compare_output_test-subprocesses');
+ });
+
+ tearDownAll(() async {
+ await Future.wait(CoverageSubprocessLauncher.coverageResults);
+ });
+
setUp(() {
tempDir = Directory.systemTemp.createTempSync('dartdoc.test.');
});
@@ -55,27 +67,38 @@
});
test('Validate missing FLUTTER_ROOT exception is clean', () async {
+ StringBuffer output = new StringBuffer();
var args = <String>[dartdocBin];
- var result = Process.runSync(Platform.resolvedExecutable, args,
+ Future run = subprocessLauncher.runStreamed(
+ Platform.resolvedExecutable, args,
environment: new Map.from(Platform.environment)
..remove('FLUTTER_ROOT'),
includeParentEnvironment: false,
- workingDirectory: _testPackageFlutterPluginPath);
+ workingDirectory: _testPackageFlutterPluginPath, perLine: (s) {
+ output.writeln(s);
+ });
+ // Asynchronous exception, but we still need the output, too.
+ expect(run, throwsA(new TypeMatcher<ProcessException>()));
+ try {
+ await run;
+ } on ProcessException catch (_) {}
+
expect(
- result.stderr,
+ output.toString(),
contains(new RegExp(
'Top level package requires Flutter but FLUTTER_ROOT environment variable not set|test_package_flutter_plugin requires the Flutter SDK, version solving failed')));
- expect(result.stderr, isNot(contains('asynchronous gap')));
- expect(result.exitCode, isNot(0));
+ expect(output.toString(), isNot(contains('asynchronous gap')));
});
test("Validate --version works", () async {
+ StringBuffer output = new StringBuffer();
var args = <String>[dartdocBin, '--version'];
- var result = Process.runSync(Platform.resolvedExecutable, args,
- workingDirectory: _testPackagePath);
+ await subprocessLauncher.runStreamed(Platform.resolvedExecutable, args,
+ workingDirectory: _testPackagePath,
+ perLine: (s) => output.writeln(s));
PackageMeta dartdocMeta = new PackageMeta.fromFilename(dartdocBin);
- expect(
- result.stdout, equals('dartdoc version: ${dartdocMeta.version}\n'));
+ expect(output.toString(),
+ endsWith('dartdoc version: ${dartdocMeta.version}\n'));
});
test("Validate html output of test_package", () async {
@@ -100,6 +123,8 @@
'--pretty-index-json',
];
+ // Deliberately use runSync here to avoid counting this test as
+ // "test coverage".
var result = Process.runSync(Platform.resolvedExecutable, args,
workingDirectory: _testPackagePath);
@@ -157,7 +182,8 @@
fail(message.join('\n'));
});
- test('Check for sample code in examples', () {
+ test('Check for sample code in examples', () async {
+ StringBuffer output = new StringBuffer();
var args = <String>[
dartdocBin,
'--include',
@@ -167,28 +193,21 @@
tempDir.path
];
- var result = Process.runSync(Platform.resolvedExecutable, args,
- workingDirectory: _testPackagePath);
-
- if (result.exitCode != 0) {
- print(result.exitCode);
- print(result.stdout);
- print(result.stderr);
- fail('dartdoc failed');
- }
+ await subprocessLauncher.runStreamed(Platform.resolvedExecutable, args,
+ workingDirectory: _testPackagePath,
+ perLine: (s) => output.writeln(s));
// Examples are reported as unfound because we (purposefully)
// did not use --example-path-prefix above.
final sep = '.'; // We don't care what the path separator character is
final firstUnfoundExample = new RegExp('warning: lib${sep}example.dart: '
'@example file not found.*test_package${sep}dog${sep}food.md');
- if (!result.stderr.contains(firstUnfoundExample)) {
- fail('Should warn about unfound @example files: \n'
- 'stdout:\n${result.stdout}\nstderr:\n${result.stderr}');
+ if (!output.toString().contains(firstUnfoundExample)) {
+ fail('Should warn about unfound @example files');
}
});
- test('Validate JSON output', () {
+ test('Validate JSON output', () async {
var args = <String>[
dartdocBin,
'--include',
@@ -199,28 +218,15 @@
'--json'
];
- var result = Process.runSync(Platform.resolvedExecutable, args,
+ Iterable<Map> jsonValues = await subprocessLauncher.runStreamed(
+ Platform.resolvedExecutable, args,
workingDirectory: _testPackagePath);
- if (result.exitCode != 0) {
- print(result.exitCode);
- print(result.stdout);
- print(result.stderr);
- fail('dartdoc failed');
- }
-
- var jsonValues = LineSplitter.split(result.stdout)
- .map((j) => json.decode(j) as Map<String, dynamic>)
- .toList();
-
expect(jsonValues, isNotEmpty,
reason: 'All STDOUT lines should be JSON-encoded maps.');
+ }, timeout: new Timeout.factor(2));
- expect(result.stderr as String, isEmpty,
- reason: 'STDERR should be empty.');
- });
-
- test('--footer-text includes text', () {
+ test('--footer-text includes text', () async {
String footerTextPath =
pathLib.join(Directory.systemTemp.path, 'footer.txt');
new File(footerTextPath).writeAsStringSync(' footer text include ');
@@ -234,20 +240,15 @@
tempDir.path
];
- var result = Process.runSync(Platform.resolvedExecutable, args,
+ await subprocessLauncher.runStreamed(Platform.resolvedExecutable, args,
workingDirectory: _testPackagePath);
- if (result.exitCode != 0) {
- print(result.exitCode);
- print(result.stdout);
- print(result.stderr);
- fail('dartdoc failed');
- }
-
File outFile = new File(pathLib.join(tempDir.path, 'index.html'));
expect(outFile.readAsStringSync(), contains('footer text include'));
});
- }, onPlatform: {'windows': new Skip('Avoiding parsing git output')});
+ },
+ onPlatform: {'windows': new Skip('Avoiding parsing git output')},
+ timeout: new Timeout.factor(3));
}
Map<String, String> _parseOutput(
diff --git a/test/dartdoc_test.dart b/test/dartdoc_test.dart
index d0caa67..2ccee81 100644
--- a/test/dartdoc_test.dart
+++ b/test/dartdoc_test.dart
@@ -27,7 +27,11 @@
group('dartdoc with generators', () {
Directory tempDir;
List<String> outputParam;
+ CoverageSubprocessLauncher subprocessLauncher;
+
setUpAll(() async {
+ subprocessLauncher =
+ new CoverageSubprocessLauncher('dartdoc_test-subprocesses');
tempDir = Directory.systemTemp.createTempSync('dartdoc.test.');
outputParam = ['--output', tempDir.path];
DartdocOptionSet optionSet = await DartdocOptionSet.fromOptionGenerators(
@@ -38,6 +42,7 @@
});
tearDown(() async {
+ await Future.wait(CoverageSubprocessLauncher.coverageResults);
tempDir.listSync().forEach((FileSystemEntity f) {
f.deleteSync(recursive: true);
});
@@ -112,21 +117,24 @@
String dartdocPath = pathLib.join('bin', 'dartdoc.dart');
test('errors cause non-zero exit when warnings are off', () async {
- ProcessResult result = Process.runSync(Platform.resolvedExecutable, [
- dartdocPath,
- '--input=$testPackageToolError',
- '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}',
- ]);
- expect(result.exitCode, isNonZero);
+ expect(
+ () => subprocessLauncher.runStreamed(Platform.resolvedExecutable, [
+ dartdocPath,
+ '--input=${testPackageToolError.path}',
+ '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}'
+ ]),
+ throwsA(const TypeMatcher<ProcessException>()));
});
+
test('errors cause non-zero exit when warnings are on', () async {
- ProcessResult result = Process.runSync(Platform.resolvedExecutable, [
- dartdocPath,
- '--input=$testPackageToolError',
- '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}',
- '--show-warnings',
- ]);
- expect(result.exitCode, isNonZero);
+ expect(
+ () => subprocessLauncher.runStreamed(Platform.resolvedExecutable, [
+ dartdocPath,
+ '--input=${testPackageToolError.path}',
+ '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}',
+ '--show-warnings',
+ ]),
+ throwsA(const TypeMatcher<ProcessException>()));
});
});
diff --git a/test/src/utils.dart b/test/src/utils.dart
index 17ae32e..3cf9b4a 100644
--- a/test/src/utils.dart
+++ b/test/src/utils.dart
@@ -12,10 +12,11 @@
import 'package:dartdoc/src/html/html_generator.dart';
import 'package:dartdoc/src/model.dart';
import 'package:dartdoc/src/package_meta.dart';
-import 'package:dartdoc/src/tuple.dart';
import 'package:path/path.dart' as pathLib;
final RegExp quotables = new RegExp(r'[ "\r\n\$]');
+final RegExp observatoryPortRegexp =
+ new RegExp(r'^Observatory listening on http://.*:(\d+)');
Directory sdkDir;
PackageMeta sdkPackageMeta;
@@ -114,27 +115,34 @@
/// [CoverageSubprocessLauncher]. Requires that these be dart processes.
class CoverageSubprocessLauncher extends SubprocessLauncher {
CoverageSubprocessLauncher(String context, [Map<String, String> environment])
- : super(context, environment);
+ : super(context, environment) {
+ environment ??= {};
+ environment['DARTDOC_COVERAGE_DATA'] = tempDir.path;
+ }
- static int nextObservatoryPort = 9292;
+ static int nextRun = 0;
/// Set this to true to enable coverage runs.
- static bool coverageEnabled = false;
+ static bool get coverageEnabled =>
+ Platform.environment.containsKey('COVERAGE_TOKEN');
/// A list of all coverage results picked up by all launchers.
- static List<Tuple2<String, Future<Iterable<Map>>>> coverageResults = [];
+ static List<Future<Iterable<Map>>> coverageResults = [];
static Directory _tempDir;
- static Directory get tempDir =>
- _tempDir ??= Directory.systemTemp.createTempSync('dartdoc_coverage_data');
+ static Directory get tempDir {
+ if (_tempDir == null) {
+ if (Platform.environment['DARTDOC_COVERAGE_DATA'] != null) {
+ _tempDir = new Directory(Platform.environment['DARTDOC_COVERAGE_DATA']);
+ } else {
+ _tempDir = Directory.systemTemp.createTempSync('dartdoc_coverage_data');
+ }
+ }
+ return _tempDir;
+ }
- int _observatoryPort;
- // TODO(jcollins-g): use ephemeral ports
- int get observatoryPort => _observatoryPort ??= nextObservatoryPort++;
-
- String _outCoverageFilename;
- String get outCoverageFilename => _outCoverageFilename ??=
- pathLib.join(tempDir.path, 'dart-cov-0-${observatoryPort}.json');
+ static String buildNextCoverageFilename() =>
+ pathLib.join(tempDir.path, 'dart-cov-${pid}-${nextRun++}.json');
/// Call once all coverage runs have been generated by calling runStreamed
/// on all [CoverageSubprocessLaunchers].
@@ -145,7 +153,7 @@
var launcher = SubprocessLauncher('format_coverage');
/// Wait for all coverage runs to finish.
- await Future.wait(currentCoverageResults.map((t) => t.item2));
+ await Future.wait(currentCoverageResults);
return launcher.runStreamed(Platform.executable, [
'tool/format_coverage.dart', // TODO(jcollins-g): use pub after dart-lang/coverage#240 is landed
@@ -162,31 +170,54 @@
@override
Future<Iterable<Map>> runStreamed(String executable, List<String> arguments,
- {String workingDirectory}) {
- assert(executable == Platform.executable,
- 'Must use dart executable for tracking coverage');
+ {String workingDirectory,
+ Map<String, String> environment,
+ bool includeParentEnvironment = true,
+ void Function(String) perLine}) async {
+ environment ??= {};
+ assert(
+ executable == Platform.executable ||
+ executable == Platform.resolvedExecutable,
+ 'Must use dart executable for tracking coverage');
- if (coverageEnabled) {
- arguments = [
- '--enable-vm-service=${observatoryPort}',
- '--pause-isolates-on-exit'
- ]..addAll(arguments);
+ Completer<String> portAsString = new Completer();
+ void parsePortAsString(String line) {
+ if (perLine != null) perLine(line);
+ if (!portAsString.isCompleted) {
+ Match m = observatoryPortRegexp.matchAsPrefix(line);
+ if (m?.group(1) != null) portAsString.complete(m.group(1));
+ }
}
- Future<Iterable<Map>> results = super
- .runStreamed(executable, arguments, workingDirectory: workingDirectory);
+ Completer<Iterable<Map>> coverageResult;
if (coverageEnabled) {
- coverageResults.add(new Tuple2(
- outCoverageFilename,
- super.runStreamed('pub', [
- 'run',
- 'coverage:collect_coverage',
- '--wait-paused',
- '--resume-isolates',
- '--port=${observatoryPort}',
- '--out=${outCoverageFilename}',
- ])));
+ coverageResult = new Completer();
+ // This must be added before awaiting in this method.
+ coverageResults.add(coverageResult.future);
+ arguments = ['--enable-vm-service:0', '--pause-isolates-on-exit']
+ ..addAll(arguments);
+ if (!environment.containsKey('DARTDOC_COVERAGE_DATA')) {
+ environment['DARTDOC_COVERAGE_DATA'] = tempDir.path;
+ }
+ }
+
+ Future<Iterable<Map>> results = super.runStreamed(executable, arguments,
+ environment: environment,
+ includeParentEnvironment: includeParentEnvironment,
+ workingDirectory: workingDirectory,
+ perLine: parsePortAsString);
+
+ if (coverageEnabled) {
+ // ignore: unawaited_futures
+ super.runStreamed('pub', [
+ 'run',
+ 'coverage:collect_coverage',
+ '--wait-paused',
+ '--resume-isolates',
+ '--port=${await portAsString.future}',
+ '--out=${buildNextCoverageFilename()}',
+ ]).then((r) => coverageResult.complete(r));
}
return results;
}
@@ -194,7 +225,7 @@
class SubprocessLauncher {
final String context;
- final Map<String, String> environment;
+ final Map<String, String> environmentDefaults;
String get prefix => context.isNotEmpty ? '$context: ' : '';
@@ -216,7 +247,7 @@
}
SubprocessLauncher(this.context, [Map<String, String> environment])
- : this.environment = environment ?? <String, String>{};
+ : this.environmentDefaults = environment ?? <String, String>{};
/// A wrapper around start/await process.exitCode that will display the
/// output of the executable continuously and fail on non-zero exit codes.
@@ -226,15 +257,22 @@
///
/// Makes running programs in grinder similar to set -ex for bash, even on
/// Windows (though some of the bashisms will no longer make sense).
- /// TODO(jcollins-g): move this to grinder?
+ /// TODO(jcollins-g): refactor to return a stream of stderr/stdout lines
+ /// and their associated JSON objects.
Future<Iterable<Map>> runStreamed(String executable, List<String> arguments,
- {String workingDirectory}) async {
+ {String workingDirectory,
+ Map<String, String> environment,
+ bool includeParentEnvironment: true,
+ void Function(String) perLine}) async {
+ environment ??= {};
+ environment.addAll(environmentDefaults);
List<Map> jsonObjects;
/// Allow us to pretend we didn't pass the JSON flag in to dartdoc by
/// printing what dartdoc would have printed without it, yet storing
/// json objects into [jsonObjects].
Iterable<String> jsonCallback(String line) {
+ if (perLine != null) perLine(line);
Map result;
try {
result = json.decoder.convert(line);
@@ -296,7 +334,9 @@
realArguments.addAll(arguments);
Process process = await Process.start(realExecutable, realArguments,
- workingDirectory: workingDirectory, environment: environment);
+ workingDirectory: workingDirectory,
+ environment: environment,
+ includeParentEnvironment: includeParentEnvironment);
Future<void> stdoutFuture = _printStream(process.stdout, stdout,
prefix: prefix, filter: jsonCallback);
Future<void> stderrFuture = _printStream(process.stderr, stderr,
@@ -310,4 +350,4 @@
}
return jsonObjects;
}
-}
\ No newline at end of file
+}
diff --git a/tool/grind.dart b/tool/grind.dart
index d07ebd6..46fc1c9 100644
--- a/tool/grind.dart
+++ b/tool/grind.dart
@@ -856,8 +856,6 @@
Future<void> testDart2() async {
List<String> parameters = ['--enable-asserts'];
- CoverageSubprocessLauncher.coverageEnabled =
- Platform.environment.containsKey('COVERAGE_TOKEN');
for (File dartFile in testFiles) {
await testFutures.addFutureFromClosure(() => new CoverageSubprocessLauncher(
'dart2-${pathLib.basename(dartFile.path)}')
