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

Author: jcollins-g

.travis.yml

@@ -4,9 +4,9 @@ dart:
   - 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

lib/src/io_utils.dart

@@ -80,7 +80,6 @@ final libraryNameRegexp = new RegExp('[.:]');
 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 @@ class MultiFutureTracker<T> {
   /// Wait until all futures added so far have completed.
   Future<void> wait() async => await _waitUntil(0);
 }
-
-

lib/src/model.dart

@@ -464,8 +464,8 @@ class Accessor extends ModelElement implements EnclosedElement {
   @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 @@ abstract class GetterSetterCombo implements ModelElement {
     if (writeOnly) return r'&#8592;';
     // ↔
     if (readWrite) return r'&#8596;';
-    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;

test/compare_output_test.dart

@@ -12,6 +12,8 @@ import 'package:dartdoc/src/package_meta.dart';
 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 @@ String get _testPackageFlutterPluginPath => pathLib
 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 @@ void main() {
     });
 
     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 @@ void main() {
         '--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 @@ void main() {
       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 @@ void main() {
         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 @@ void main() {
         '--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 @@ void main() {
         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(

test/dartdoc_test.dart

@@ -27,7 +27,11 @@ void main() {
   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 @@ void main() {
     });
 
     tearDown(() async {
+      await Future.wait(CoverageSubprocessLauncher.coverageResults);
       tempDir.listSync().forEach((FileSystemEntity f) {
         f.deleteSync(recursive: true);
       });
@@ -112,21 +117,24 @@ void main() {
       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>()));
       });
     });