diff --git a/analysis_options.yaml b/analysis_options.yaml index faa3ef955..e9b4026ac 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -8,45 +8,66 @@ analyzer: override_on_non_overriding_method: error linter: rules: - # Errors - - avoid_empty_else - - await_only_futures - - comment_references - - control_flow_in_finally - - empty_statements - - hash_and_equals - - iterable_contains_unrelated_type - - no_duplicate_case_values - - test_types_in_equals - - throw_in_finally - - unawaited_futures - - unnecessary_statements - - unrelated_type_equality_checks - - valid_regexps - - # Style - annotate_overrides + - avoid_empty_else - avoid_function_literals_in_foreach_calls - avoid_init_to_null + - avoid_null_checks_in_equality_operators - avoid_return_types_on_setters - avoid_returning_null - avoid_unused_constructor_parameters + - await_only_futures - camel_case_types + - cancel_subscriptions + #- cascade_invocations + - comment_references + - constant_identifier_names + - control_flow_in_finally - directives_ordering - empty_catches - empty_constructor_bodies + - empty_statements + - hash_and_equals + - implementation_imports + - invariant_booleans + - iterable_contains_unrelated_type - library_names - library_prefixes + - list_remove_unrelated_type + - no_adjacent_strings_in_list + - no_duplicate_case_values - non_constant_identifier_names + - omit_local_variable_types + - only_throw_errors + - overridden_fields + - package_api_docs + - package_names + - package_prefixed_library_names + - prefer_adjacent_string_concatenation + - prefer_collection_literals - prefer_conditional_assignment + - prefer_const_constructors + - prefer_contains - prefer_final_fields + #- prefer_final_locals + - prefer_initializing_formals + - prefer_interpolation_to_compose_strings - prefer_is_empty - prefer_is_not_empty + - prefer_single_quotes - prefer_typing_uninitialized_variables - recursive_getters - slash_for_doc_comments + - super_goes_last + - test_types_in_equals + - throw_in_finally - type_init_formals + - unawaited_futures - unnecessary_brace_in_string_interps - unnecessary_getters_setters - unnecessary_lambdas - unnecessary_null_aware_assignments + - unnecessary_statements + - unnecessary_this + - unrelated_type_equality_checks + - valid_regexps diff --git a/webdev/CHANGELOG.md b/webdev/CHANGELOG.md index 098ae2b62..8ff025c90 100644 --- a/webdev/CHANGELOG.md +++ b/webdev/CHANGELOG.md @@ -1,3 +1,8 @@ +## 0.1.1 + +- Checks for a dependency on `build_web_compilers`. +- Gracefully handle exceptions. + ## 0.1.0 - Initial release. Supports basic invocation of `build` and `serve` with diff --git a/webdev/bin/webdev.dart b/webdev/bin/webdev.dart index b032ab62d..9f0ca846b 100644 --- a/webdev/bin/webdev.dart +++ b/webdev/bin/webdev.dart @@ -4,6 +4,7 @@ import 'dart:async'; import 'dart:io'; +import 'dart:isolate'; import 'package:args/command_runner.dart'; import 'package:io/ansi.dart'; @@ -28,5 +29,16 @@ Future main(List args) async { } exitCode = ExitCode.config.code; + } on FileSystemException catch (e) { + print(yellow.wrap('Could not run in the current directory.')); + print(e.message); + if (e.path != null) { + print(' ${e.path}'); + } + exitCode = ExitCode.config.code; + } on IsolateSpawnException catch (e) { + print(red.wrap('An unexpected exception has occurred.')); + print(e.message); + exitCode = ExitCode.software.code; } } diff --git a/webdev/lib/src/command/build_runner_command_base.dart b/webdev/lib/src/command/build_runner_command_base.dart index 8c47bf136..7d2e309ce 100644 --- a/webdev/lib/src/command/build_runner_command_base.dart +++ b/webdev/lib/src/command/build_runner_command_base.dart @@ -11,6 +11,8 @@ import 'package:stack_trace/stack_trace.dart'; import '../pubspec.dart'; +const _packagesFileName = '.packages'; + /// Extend to get a command with the arguments common to all build_runner /// commands. abstract class BuildRunnerCommandBase extends Command { @@ -30,6 +32,8 @@ abstract class BuildRunnerCommandBase extends Command { Future runCore(String command) async { await checkPubspecLock(); + var buildRunnerScript = await _buildRunnerScript(); + final arguments = [command]..addAll(argResults.arguments); var exitCode = 0; @@ -49,68 +53,87 @@ abstract class BuildRunnerCommandBase extends Command { stderr.writeln(new Trace.parse(trace).terse); if (exitCode == 0) exitCode = 1; }); - await Isolate.spawnUri( - await _buildRunnerScript(), arguments, messagePort.sendPort, - onExit: exitPort.sendPort, - onError: errorPort.sendPort, - automaticPackageResolution: true); - StreamSubscription exitCodeListener; - exitCodeListener = messagePort.listen((isolateExitCode) { - if (isolateExitCode is! int) { - throw new StateError( - 'Bad response from isolate, expected an exit code but got ' - '$isolateExitCode'); - } - exitCode = isolateExitCode as int; - exitCodeListener.cancel(); - exitCodeListener = null; - }); - await exitPort.first; - await errorListener.cancel(); - await exitCodeListener?.cancel(); - return exitCode; + try { + await Isolate.spawnUri(buildRunnerScript, arguments, messagePort.sendPort, + onExit: exitPort.sendPort, + onError: errorPort.sendPort, + automaticPackageResolution: true); + StreamSubscription exitCodeListener; + exitCodeListener = messagePort.listen((isolateExitCode) { + if (isolateExitCode is! int) { + throw new StateError( + 'Bad response from isolate, expected an exit code but got ' + '$isolateExitCode'); + } + exitCode = isolateExitCode as int; + exitCodeListener.cancel(); + exitCodeListener = null; + }); + await exitPort.first; + await errorListener.cancel(); + await exitCodeListener?.cancel(); + + return exitCode; + } finally { + exitPort.close(); + errorPort.close(); + messagePort.close(); + } } } Future _buildRunnerScript() async { + var packagesFile = new File(_packagesFileName); + if (!packagesFile.existsSync()) { + throw new FileSystemException( + 'A `$_packagesFileName` file does not exist in the target directory.', + packagesFile.absolute.path); + } + var dataUri = new Uri.dataFromString(_bootstrapScript); var messagePort = new ReceivePort(); var exitPort = new ReceivePort(); var errorPort = new ReceivePort(); - await Isolate.spawnUri(dataUri, [], messagePort.sendPort, - onExit: exitPort.sendPort, - onError: errorPort.sendPort, - errorsAreFatal: true, - packageConfig: new Uri.file('.packages')); - - var allErrorsFuture = errorPort.forEach((error) { - var errorList = error as List; - var message = errorList[0] as String; - var stack = new StackTrace.fromString(errorList[1] as String); - - stderr.writeln(message); - stderr.writeln(stack); - }); - - var items = await Future.wait([ - messagePort.toList(), - allErrorsFuture, - exitPort.first.whenComplete(() { - messagePort.close(); - errorPort.close(); - }) - ]); + try { + await Isolate.spawnUri(dataUri, [], messagePort.sendPort, + onExit: exitPort.sendPort, + onError: errorPort.sendPort, + errorsAreFatal: true, + packageConfig: new Uri.file(_packagesFileName)); - var messages = items[0] as List; - if (messages.isEmpty) { - throw new StateError('An error occurred while running booting.'); - } + var allErrorsFuture = errorPort.forEach((error) { + var errorList = error as List; + var message = errorList[0] as String; + var stack = new StackTrace.fromString(errorList[1] as String); + + stderr.writeln(message); + stderr.writeln(stack); + }); - assert(messages.length == 1); - return new Uri.file(messages.single as String); + var items = await Future.wait([ + messagePort.toList(), + allErrorsFuture, + exitPort.first.whenComplete(() { + messagePort.close(); + errorPort.close(); + }) + ]); + + var messages = items[0] as List; + if (messages.isEmpty) { + throw new StateError('An error occurred while bootstrapping.'); + } + + assert(messages.length == 1); + return new Uri.file(messages.single as String); + } finally { + messagePort.close(); + exitPort.close(); + errorPort.close(); + } } const _bootstrapScript = r''' diff --git a/webdev/lib/src/pubspec.dart b/webdev/lib/src/pubspec.dart index fe0ca56e7..48836bfee 100644 --- a/webdev/lib/src/pubspec.dart +++ b/webdev/lib/src/pubspec.dart @@ -33,6 +33,12 @@ class PackageExceptionDetails { 'You must have a dependency on `build_runner` in `pubspec.yaml`. ' 'It can be in either `dependencies` or `dev_dependencies`.'); + static const noBuildWebCompilersDep = const PackageExceptionDetails._( + 'A dependency on `build_web_compilers` was not found.', + description: + 'You must have a dependency on `build_web_compilers` in `pubspec.yaml` ' + 'or transitively via another dependency.'); + @override String toString() => [error, description].join('\n'); } @@ -81,8 +87,10 @@ Future checkPubspecLock() async { } } - // TODO: validate build_web_compilers - //var buldWebCompilers = packages['build_web_compilers']; + var buldWebCompilers = packages['build_web_compilers']; + if (buldWebCompilers == null) { + issues.add(PackageExceptionDetails.noBuildWebCompilersDep); + } if (issues.isNotEmpty) { throw new PackageException._(issues); diff --git a/webdev/pubspec.yaml b/webdev/pubspec.yaml index a0395eb88..8742c3848 100644 --- a/webdev/pubspec.yaml +++ b/webdev/pubspec.yaml @@ -1,5 +1,5 @@ name: webdev -version: 0.1.0 +version: 0.1.1-dev author: Dart Team homepage: https://github.com/dart-lang/webdev description: >- diff --git a/webdev/test/integration_test.dart b/webdev/test/integration_test.dart index f42ed62bf..e2df7fdb0 100644 --- a/webdev/test/integration_test.dart +++ b/webdev/test/integration_test.dart @@ -14,29 +14,27 @@ final _webdevBin = p.absolute('bin/webdev.dart'); void main() { test('README contains help output', () async { var process = await TestProcess.start('dart', [_webdevBin]); - var output = (await process.stdoutStream().join('\n')).trim(); + await process.shouldExit(0); var readme = new File('README.md'); - expect(readme.readAsStringSync(), contains('```console\n\$ webdev\n$output\n```')); - - await process.shouldExit(0); }); test('non-existant commands create errors', () async { var process = await TestProcess.start('dart', [_webdevBin, 'monkey']); - var output = (await process.stdoutStream().join('\n')).trim(); - - expect(output, contains('Could not find a command named "monkey".')); + await expectLater( + process.stdout, emits('Could not find a command named "monkey".')); await process.shouldExit(64); }); test('should fail in a package without a build_runner dependency', () async { - var process = await TestProcess.start('dart', [_webdevBin, 'serve']); + // Running on the `webdev` package directory – which has no dependency on + // build runner. + var process = await TestProcess.start('dart', [_webdevBin, 'build']); var output = (await process.stdoutStream().join('\n')).trim(); expect(output, contains(r'''Could not run in the current directory. @@ -61,15 +59,138 @@ packages: var process = await TestProcess.start('dart', [_webdevBin, 'build'], workingDirectory: d.sandbox); - var output = (await process.stdoutStream().join('\n')).trim(); - expect(output, contains('Could not run in the current directory.')); - expect( - output, - contains('The `build_runner` version – $version – ' + await expectLater( + process.stdout, emits('Could not run in the current directory.')); + await expectLater( + process.stdout, + emits('The `build_runner` version – $version – ' 'is not within the supported range – >=0.8.0 <0.9.0.')); await process.shouldExit(78); }); } }); + + test('should fail with no `build_web_compilers` dependency', () async { + await d.file('pubspec.lock', ''' +# Copy-pasted from a valid run +packages: + build_runner: + dependency: "direct main" + description: + name: build_runner + url: "https://pub.dartlang.org" + source: hosted + version: "0.8.0" +''').create(); + + var process = await TestProcess.start('dart', [_webdevBin, 'build'], + workingDirectory: d.sandbox); + + await expectLater( + process.stdout, emits('Could not run in the current directory.')); + await expectLater(process.stdout, + emits('A dependency on `build_web_compilers` was not found.')); + await process.shouldExit(78); + }); + + test('should fail gracefully if there is no .packages file', () async { + await d.file('pubspec.lock', ''' +# Copy-pasted from a valid run +packages: + build_runner: + dependency: "direct main" + description: + name: build_runner + url: "https://pub.dartlang.org" + source: hosted + version: "0.8.0" + build_web_compilers: + dependency: "direct main" + description: + name: build_web_compilers + url: "https://pub.dartlang.org" + source: hosted + version: "0.3.4+2" +''').create(); + + var process = await TestProcess.start('dart', [_webdevBin, 'build'], + workingDirectory: d.sandbox); + + await expectLater( + process.stdout, emits('Could not run in the current directory.')); + await expectLater(process.stdout, + emits('A `.packages` file does not exist in the target directory.')); + await process.shouldExit(78); + }); + + test('should fail gracefully if there is an isolate error', () async { + await d.file('pubspec.lock', ''' +# Copy-pasted from a valid run +packages: + build_runner: + dependency: "direct main" + description: + name: build_runner + url: "https://pub.dartlang.org" + source: hosted + version: "0.8.0" + build_web_compilers: + dependency: "direct main" + description: + name: build_web_compilers + url: "https://pub.dartlang.org" + source: hosted + version: "0.3.4+2" +''').create(); + + await d.file('.packages', '').create(); + + var process = await TestProcess.start('dart', [_webdevBin, 'build'], + workingDirectory: d.sandbox); + + await expectLater( + process.stdout, emits('An unexpected exception has occurred.')); + await process.shouldExit(70); + }); + + test('should succeed with valid configuration', () async { + var exampleDirectory = p.absolute(p.join(p.current, '..', 'example')); + var process = await TestProcess.start('pub', ['get'], + workingDirectory: exampleDirectory, environment: _getPubEnvironment()); + + await process.shouldExit(0); + + await d.file('.packages', isNotEmpty).validate(exampleDirectory); + await d.file('pubspec.lock', isNotEmpty).validate(exampleDirectory); + + process = await TestProcess.start( + 'dart', [_webdevBin, 'build', '-o', d.sandbox], + workingDirectory: exampleDirectory); + + var output = await process.stdoutStream().join('\n'); + + expect(output, contains(d.sandbox)); + expect(output, contains('[INFO] Succeeded')); + await process.shouldExit(0); + + await d.file('web/main.dart.js', isNotEmpty).validate(); + }, timeout: const Timeout(const Duration(minutes: 5))); +} + +/// Returns an environment map that includes `PUB_ENVIRONMENT`. +/// +/// Maintains any existing values for this environment var. +/// Adds a new value that flags this is a bot/test and not human usage. +Map _getPubEnvironment() { + var pubEnvironmentKey = 'PUB_ENVIRONMENT'; + var pubEnvironment = Platform.environment[pubEnvironmentKey] ?? ''; + if (pubEnvironment.isNotEmpty) { + pubEnvironment = '$pubEnvironment;'; + } + pubEnvironment = '${pubEnvironment}bot.pkg.webdev.test'; + + var environment = {'PUB_ENVIRONMENT': pubEnvironment}; + + return environment; }