diff --git a/.gitignore b/.gitignore index a144cd77ce..851a9844ed 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ .packages .pub .settings/ +.vscode/ build/ doc/ lcov.info diff --git a/.travis.yml b/.travis.yml index a350bc2e77..69098546b5 100644 --- a/.travis.yml +++ b/.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 diff --git a/analysis_options.yaml b/analysis_options.yaml index 0d9a667f52..90e3d507af 100644 --- a/analysis_options.yaml +++ b/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: diff --git a/analysis_options_presubmit.yaml b/analysis_options_presubmit.yaml index 2e20ff4bcd..446a625655 100644 --- a/analysis_options_presubmit.yaml +++ b/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: diff --git a/lib/src/io_utils.dart b/lib/src/io_utils.dart index 27fcbeaa26..0aa8ce5119 100644 --- a/lib/src/io_utils.dart +++ b/lib/src/io_utils.dart @@ -123,7 +123,12 @@ class MultiFutureTracker { /// Wait until fewer or equal to this many Futures are outstanding. Future _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 } } @@ -131,13 +136,12 @@ class MultiFutureTracker { /// once the queue is sufficiently empty. The returned future completes /// when the generated [Future] has been added to the queue. Future addFutureFromClosure(Future Function() closure) async { - while (_trackedFutures.length > parallel - 1) { - await Future.any(_trackedFutures); - } + await _waitUntil(parallel - 1); Future 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. diff --git a/test/end2end/dartdoc_integration_test.dart b/test/end2end/dartdoc_integration_test.dart index 3114e120e8..2d552229cb 100644 --- a/test/end2end/dartdoc_integration_test.dart +++ b/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 = []; + print('dartdocPath: $dartdocPath'); await subprocessLauncher.runStreamed( Platform.resolvedExecutable, [dartdocPath, '--help'], perLine: outputLines.add); diff --git a/test/io_utils_test.dart b/test/io_utils_test.dart index 5b0548c4ca..fab394966b 100644 --- a/test/io_utils_test.dart +++ b/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())); + var u = + Future.delayed(Duration(milliseconds: 10), () => throw Exception()); + await sharedTracker.addFutureFromClosure(() => u); + expect(u, throwsA(const TypeMatcher())); + var v = + Future.delayed(Duration(milliseconds: 10), () => throw Exception()); + await sharedTracker.addFutureFromClosure(() => v); + expect(v, throwsA(const TypeMatcher())); + + /// We deadlock here if the exception is not handled properly. + await sharedTracker.wait(); + }); + + test('basic sequential processing works with no deadlock', () async { + var completed = {}; + 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 = {}; + 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 = {}; + 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 = {}; + 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)); + }); + }); } diff --git a/testing/test_package_flutter_plugin/.gitignore b/testing/flutter_packages/test_package_flutter_plugin/.gitignore similarity index 100% rename from testing/test_package_flutter_plugin/.gitignore rename to testing/flutter_packages/test_package_flutter_plugin/.gitignore diff --git a/testing/test_package_flutter_plugin/analysis_options.yaml b/testing/flutter_packages/test_package_flutter_plugin/analysis_options.yaml similarity index 100% rename from testing/test_package_flutter_plugin/analysis_options.yaml rename to testing/flutter_packages/test_package_flutter_plugin/analysis_options.yaml diff --git a/testing/test_package_flutter_plugin/lib/testlib.dart b/testing/flutter_packages/test_package_flutter_plugin/lib/testlib.dart similarity index 100% rename from testing/test_package_flutter_plugin/lib/testlib.dart rename to testing/flutter_packages/test_package_flutter_plugin/lib/testlib.dart diff --git a/testing/test_package_flutter_plugin/pubspec.yaml b/testing/flutter_packages/test_package_flutter_plugin/pubspec.yaml similarity index 100% rename from testing/test_package_flutter_plugin/pubspec.yaml rename to testing/flutter_packages/test_package_flutter_plugin/pubspec.yaml diff --git a/tool/grind.dart b/tool/grind.dart index 0a5d5a9cb8..6f2a185dff 100644 --- a/tool/grind.dart +++ b/tool/grind.dart @@ -120,8 +120,8 @@ Directory get testPackage => Directory get testPackageExperiments => Directory(path.joinAll(['testing', 'test_package_experiments'])); -Directory get pluginPackage => - Directory(path.joinAll(['testing', 'test_package_flutter_plugin'])); +Directory get pluginPackage => Directory(path + .joinAll(['testing', 'flutter_packages', 'test_package_flutter_plugin'])); Directory _testPackageDocsDir; @@ -278,7 +278,7 @@ void dartfmt() async { void presubmit() => null; @Task('Run long tests, self-test dartdoc, and run the publish test') -@Depends(presubmit, test, testDartdoc) +@Depends(presubmit, longTest, testDartdoc) void buildbot() => null; @Task('Generate docs for the Dart SDK') diff --git a/tool/travis.sh b/tool/travis.sh index a35697e282..12d982c9d9 100755 --- a/tool/travis.sh +++ b/tool/travis.sh @@ -11,7 +11,9 @@ set -ex export PATH="$PATH":"~/.pub-cache/bin" DART_VERSION=`dart --version 2>&1 | awk '{print $4}'` # Do not run coverage on non-dev builds or non-Linux platforms. -if ! echo "${DART_VERSION}" | grep -q dev || ! uname | grep -q Linux ; then +# Disable coverage on dev builds and switch to stable until +# dart-lang/sdk#43487 is fixed. +if ! echo "${DART_VERSION}" | grep -q stable || ! uname | grep -q Linux ; then unset COVERAGE_TOKEN fi