Changes to adapt to vscode and fix a deadlock (#2351)

* testing

* test_package_flutter_plugin move out of the way

* Revert accidental change

* Add analysis options changes

* Fix deadlock blocking tests

* print debugging

* more prints, don't duplicate model_test

* Fix presubmit error

* more debugging

* switch to raw, maybe dart run bug fixed?

* return to dev version

* Unconditionally disable coverage

* Fix deadlock on exceptions and strip extraneous stuff.

* Fix tests to properly reproduce race

* travis setting tweaks

* Disallow flutter failures and fix plugin directory

Author: jcollins-g

.gitignore

@@ -6,6 +6,7 @@
 .packages
 .pub
 .settings/
+.vscode/
 build/
 doc/
 lcov.info

.travis.yml

@@ -3,7 +3,6 @@ sudo: false
 dart:
   - stable
   - dev
-
 os:
   - linux
 
@@ -13,9 +12,13 @@ matrix:
     - env: DARTDOC_BOT=main
       os: osx
       dart: dev
-  allow_failures:
-    # See https://github.com/dart-lang/dartdoc/issues/2302.
+  exclude:
     - env: DARTDOC_BOT=flutter
+      # Do not try to run flutter against the "stable" sdk,
+      # it is unlikely to work and produces uninteresting
+      # results.
+      dart: stable
+  allow_failures:
     # Some packages at HEAD require 2.10.0-dev.
     - env: DARTDOC_BOT=sdk-analyzer
       dart: stable

analysis_options.yaml

@@ -12,7 +12,7 @@ analyzer:
     - 'lib/templates/*.html'
     - 'pub.dartlang.org/**'
     - 'testing/**'
-    - 'testing/test_package_flutter_plugin/**'
+    - 'testing/flutter_packages/test_package_flutter_plugin/**'
     - 'testing/test_package_export_error/**'
 linter:
   rules:

analysis_options_presubmit.yaml

@@ -15,7 +15,7 @@ analyzer:
     - 'lib/templates/*.html'
     - 'pub.dartlang.org/**'
     - 'testing/**'
-    - 'testing/test_package_flutter_plugin/**'
+    - 'testing/flutter_packages/test_package_flutter_plugin/**'
     - 'testing/test_package_export_error/**'
 linter:
   rules:

lib/src/io_utils.dart

@@ -123,21 +123,25 @@ class MultiFutureTracker<T> {
   /// Wait until fewer or equal to this many Futures are outstanding.
   Future<void> _waitUntil(int max) async {
     while (_trackedFutures.length > max) {
-      await Future.any(_trackedFutures);
+      try {
+        await Future.any(_trackedFutures);
+        // The empty catch is OK because we're just counting future completions
+        // and we don't care about errors (the original caller of
+        // addFutureToClosure is responsible for those).
+      } catch (e) {} // ignore: empty_catches
     }
   }
 
   /// Generates a [Future] from the given closure and adds it to the queue,
   /// once the queue is sufficiently empty.  The returned future completes
   /// when the generated [Future] has been added to the queue.
   Future<void> addFutureFromClosure(Future<T> Function() closure) async {
-    while (_trackedFutures.length > parallel - 1) {
-      await Future.any(_trackedFutures);
-    }
+    await _waitUntil(parallel - 1);
     Future<void> future = closure();
     _trackedFutures.add(future);
     // ignore: unawaited_futures
-    future.then((f) => _trackedFutures.remove(future));
+    future.then((f) => _trackedFutures.remove(future),
+        onError: (s, e) => _trackedFutures.remove(future));
   }
 
   /// Wait until all futures added so far have completed.

test/end2end/dartdoc_integration_test.dart

@@ -19,8 +19,8 @@ Uri get _currentFileUri =>
     (reflect(main) as ClosureMirror).function.location.sourceUri;
 String get _testPackagePath =>
     path.fromUri(_currentFileUri.resolve('../../testing/test_package'));
-String get _testPackageFlutterPluginPath => path.fromUri(
-    _currentFileUri.resolve('../../testing/test_package_flutter_plugin'));
+String get _testPackageFlutterPluginPath => path.fromUri(_currentFileUri
+    .resolve('../../testing/flutter_packages/test_package_flutter_plugin'));
 
 void main() {
   group('Invoking command-line dartdoc', () {
@@ -122,6 +122,7 @@ void main() {
 
     test('help prints command line args', () async {
       var outputLines = <String>[];
+      print('dartdocPath: $dartdocPath');
       await subprocessLauncher.runStreamed(
           Platform.resolvedExecutable, [dartdocPath, '--help'],
           perLine: outputLines.add);

test/io_utils_test.dart

@@ -17,4 +17,77 @@ void main() {
       expect(getFileNameFor('dartdoc.generator'), 'dartdoc-generator.html');
     });
   });
+
+  group('MultiFutureTracker', () {
+    /// A special test designed to check shared [MultiFutureTracker]
+    /// behavior when exceptions occur after a delay in the passed closures to
+    /// [MultiFutureTracker.addFutureFromClosure].
+    test('no deadlock when delayed exceptions fire in closures', () async {
+      var sharedTracker = MultiFutureTracker(2);
+      var t =
+          Future.delayed(Duration(milliseconds: 10), () => throw Exception());
+      await sharedTracker.addFutureFromClosure(() => t);
+      expect(t, throwsA(const TypeMatcher<Exception>()));
+      var u =
+          Future.delayed(Duration(milliseconds: 10), () => throw Exception());
+      await sharedTracker.addFutureFromClosure(() => u);
+      expect(u, throwsA(const TypeMatcher<Exception>()));
+      var v =
+          Future.delayed(Duration(milliseconds: 10), () => throw Exception());
+      await sharedTracker.addFutureFromClosure(() => v);
+      expect(v, throwsA(const TypeMatcher<Exception>()));
+
+      /// We deadlock here if the exception is not handled properly.
+      await sharedTracker.wait();
+    });
+
+    test('basic sequential processing works with no deadlock', () async {
+      var completed = <int>{};
+      var tracker = MultiFutureTracker(1);
+      await tracker.addFutureFromClosure(() async => completed.add(1));
+      await tracker.addFutureFromClosure(() async => completed.add(2));
+      await tracker.addFutureFromClosure(() async => completed.add(3));
+      await tracker.wait();
+      expect(completed.length, equals(3));
+    });
+
+    test('basic sequential processing works on exceptions', () async {
+      var completed = <int>{};
+      var tracker = MultiFutureTracker(1);
+      await tracker.addFutureFromClosure(() async => completed.add(0));
+      await tracker.addFutureFromClosure(() async => throw Exception());
+      await tracker.addFutureFromClosure(() async => throw Exception());
+      await tracker.addFutureFromClosure(() async => completed.add(3));
+      await tracker.wait();
+      expect(completed.length, equals(2));
+    });
+
+    /// Verify that if there are more exceptions than the maximum number
+    /// of in-flight [Future]s that there is no deadlock.
+    test('basic parallel processing works with no deadlock', () async {
+      var completed = <int>{};
+      var tracker = MultiFutureTracker(10);
+      for (var i = 0; i < 100; i++) {
+        await tracker.addFutureFromClosure(() async => completed.add(i));
+      }
+      await tracker.wait();
+      expect(completed.length, equals(100));
+    });
+
+    test('basic parallel processing works on exceptions', () async {
+      var completed = <int>{};
+      var tracker = MultiFutureTracker(10);
+      for (var i = 0; i < 50; i++) {
+        await tracker.addFutureFromClosure(() async => completed.add(i));
+      }
+      for (var i = 50; i < 65; i++) {
+        await tracker.addFutureFromClosure(() async => throw Exception());
+      }
+      for (var i = 65; i < 100; i++) {
+        await tracker.addFutureFromClosure(() async => completed.add(i));
+      }
+      await tracker.wait();
+      expect(completed.length, equals(85));
+    });
+  });
 }