From a62d2b337d26e05c013c787decc0384acae320f9 Mon Sep 17 00:00:00 2001 From: Yegor Date: Wed, 9 Jun 2021 22:12:27 -0700 Subject: [PATCH] migrate the rest of dev/ to null safety (flutter/engine#26668) * migrate the rest of dev/ to null safety --- engine/src/flutter/lib/web_ui/dev/felt.dart | 1 - .../flutter/lib/web_ui/dev/test_platform.dart | 145 +++++++++--------- .../flutter/lib/web_ui/dev/test_runner.dart | 67 ++++---- engine/src/flutter/lib/web_ui/dev/utils.dart | 2 +- 4 files changed, 102 insertions(+), 113 deletions(-) diff --git a/engine/src/flutter/lib/web_ui/dev/felt.dart b/engine/src/flutter/lib/web_ui/dev/felt.dart index 7d8688a5d06..d3269056230 100644 --- a/engine/src/flutter/lib/web_ui/dev/felt.dart +++ b/engine/src/flutter/lib/web_ui/dev/felt.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.6 import 'dart:io' as io; import 'package:args/command_runner.dart'; diff --git a/engine/src/flutter/lib/web_ui/dev/test_platform.dart b/engine/src/flutter/lib/web_ui/dev/test_platform.dart index cc2d3aeab8a..f21013158e7 100644 --- a/engine/src/flutter/lib/web_ui/dev/test_platform.dart +++ b/engine/src/flutter/lib/web_ui/dev/test_platform.dart @@ -2,7 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.6 import 'dart:async'; import 'dart:convert'; import 'dart:io'; @@ -50,7 +49,7 @@ class BrowserPlatform extends PlatformPlugin { /// [root] is the root directory that the server should serve. It defaults to /// the working directory. static Future start(String name, - {String root, bool doUpdateScreenshotGoldens: false}) async { + {required String root, bool doUpdateScreenshotGoldens = false}) async { assert(SupportedBrowsers.instance.supportedBrowserNames.contains(name)); var server = shelf_io.IOServer(await HttpMultiServer.loopback(0)); return BrowserPlatform._( @@ -96,12 +95,12 @@ class BrowserPlatform extends PlatformPlugin { final String _root; /// The HTTP client to use when caching JS files in `pub serve`. - final HttpClient _http; + final HttpClient? _http; /// Handles taking screenshots during tests. /// /// Implementation will differ depending on the browser. - ScreenshotManager _screenshotManager; + ScreenshotManager? _screenshotManager; /// Whether [close] has been called. bool get _closed => _closeMemo.hasRun; @@ -111,10 +110,10 @@ class BrowserPlatform extends PlatformPlugin { BrowserPlatform._( String name, this._server, Configuration config, String faviconPath, - {String root, this.doUpdateScreenshotGoldens}) + {required String root, required this.doUpdateScreenshotGoldens}) : this.browserName = name, _config = config, - _root = root == null ? p.current : root, + _root = root, _http = config.pubServeUrl == null ? null : HttpClient() { var cascade = shelf.Cascade().add(_webSocketHandler.handler); @@ -187,7 +186,7 @@ class BrowserPlatform extends PlatformPlugin { } filename = - filename.replaceAll('.png', '${_screenshotManager.filenameSuffix}.png'); + filename.replaceAll('.png', '${_screenshotManager!.filenameSuffix}.png'); String goldensDirectory; if (filename.startsWith('__local__')) { @@ -213,7 +212,7 @@ class BrowserPlatform extends PlatformPlugin { ); // Take screenshot. - final Image screenshot = await _screenshotManager.capture(regionAsRectange); + final Image screenshot = await _screenshotManager!.capture(regionAsRectange); return compareImage( screenshot, @@ -227,14 +226,14 @@ class BrowserPlatform extends PlatformPlugin { /// A handler that serves wrapper files used to bootstrap tests. shelf.Response _wrapperHandler(shelf.Request request) { - var path = p.fromUri(request.url); + final String path = p.fromUri(request.url); if (path.endsWith('.html')) { - var test = p.withoutExtension(path) + '.dart'; + final String test = p.withoutExtension(path) + '.dart'; // Link to the Dart wrapper. - var scriptBase = htmlEscape.convert(p.basename(test)); - var link = ''; + final String scriptBase = htmlEscape.convert(p.basename(test)); + final String link = ''; return shelf.Response.ok(''' @@ -251,68 +250,70 @@ class BrowserPlatform extends PlatformPlugin { return shelf.Response.notFound('Not found.'); } + void _checkNotClosed() { + if (_closed) { + throw StateError('Cannot load test suite. Test platform is closed.'); + } + } + /// Loads the test suite at [path] on the platform [platform]. /// /// This will start a browser to load the suite if one isn't already running. /// Throws an [ArgumentError] if `platform.platform` isn't a browser. + @override Future load(String path, SuitePlatform platform, SuiteConfiguration suiteConfig, Object message) async { + _checkNotClosed(); if (suiteConfig.precompiledPath == null) { throw Exception('This test platform only supports precompiled JS.'); } - var browser = platform.runtime; + final Runtime browser = platform.runtime; assert(suiteConfig.runtimes.contains(browser.identifier)); if (!browser.isBrowser) { throw ArgumentError('$browser is not a browser.'); } + _checkNotClosed(); - if (_closed) { - return null; - } - Uri suiteUrl = url.resolveUri( + final Uri suiteUrl = url.resolveUri( p.toUri(p.withoutExtension(p.relative(path, from: _root)) + '.html')); + _checkNotClosed(); - if (_closed) { - return null; + final BrowserManager? browserManager = await _browserManagerFor(browser); + if (browserManager == null) { + throw StateError('Failed to initialize browser manager for ${browser.name}'); } + _checkNotClosed(); - var browserManager = await _browserManagerFor(browser); - if (_closed || browserManager == null) { - return null; - } - - var suite = await browserManager.load(path, suiteUrl, suiteConfig, message); - if (_closed) { - return null; - } + final RunnerSuite suite = await browserManager.load(path, suiteUrl, suiteConfig, message); + _checkNotClosed(); return suite; } StreamChannel loadChannel(String path, SuitePlatform platform) => throw UnimplementedError(); - Future _browserManager; + Future? _browserManager; /// Returns the [BrowserManager] for [runtime], which should be a browser. /// /// If no browser manager is running yet, starts one. - Future _browserManagerFor(Runtime browser) { + Future _browserManagerFor(Runtime browser) { if (_browserManager != null) { - return _browserManager; + return _browserManager!; } - var completer = Completer.sync(); - var path = _webSocketHandler.create(webSocketHandler(completer.complete)); - var webSocketUrl = url.replace(scheme: 'ws').resolve(path); - var hostUrl = (_config.pubServeUrl == null ? url : _config.pubServeUrl) + final Completer completer = Completer.sync(); + final String path = _webSocketHandler.create(webSocketHandler(completer.complete)); + final Uri webSocketUrl = url.replace(scheme: 'ws').resolve(path); + final Uri hostUrl = (_config.pubServeUrl ?? url) .resolve('packages/web_engine_tester/static/index.html') .replace(queryParameters: { 'managerUrl': webSocketUrl.toString(), 'debug': _config.pauseAfterLoad.toString() }); - var future = BrowserManager.start(browser, hostUrl, completer.future, + final Future future = BrowserManager.start(browser, hostUrl, completer.future, debug: _config.pauseAfterLoad); // Store null values for browsers that error out so we know not to load them @@ -327,9 +328,9 @@ class BrowserPlatform extends PlatformPlugin { /// Note that this doesn't close the server itself. Browser tests can still be /// loaded, they'll just spawn new browsers. Future closeEphemeral() async { - final BrowserManager result = await _browserManager; - if (result != null) { - await result.close(); + if (_browserManager != null) { + final BrowserManager? result = await _browserManager!; + await result?.close(); } } @@ -341,9 +342,9 @@ class BrowserPlatform extends PlatformPlugin { return _closeMemo.runOnce(() async { final List> futures = >[]; futures.add(Future.microtask(() async { - final BrowserManager result = await _browserManager; - if (result != null) { - await result.close(); + if (_browserManager != null) { + final BrowserManager? result = await _browserManager!; + await result?.close(); } })); futures.add(_server.close()); @@ -351,7 +352,7 @@ class BrowserPlatform extends PlatformPlugin { await Future.wait(futures); if (_config.pubServeUrl != null) { - _http.close(); + _http!.close(); } }); } @@ -432,12 +433,12 @@ class PathHandler { } FutureOr _onRequest(shelf.Request request) { - shelf.Handler handler; - int handlerIndex; - var node = _paths; + shelf.Handler? handler; + int handlerIndex = -1; + _Node? node = _paths; var components = p.url.split(request.url.path); for (var i = 0; i < components.length; i++) { - node = node.children[components[i]]; + node = node?.children[components[i]]; if (node == null) { break; } @@ -459,7 +460,7 @@ class PathHandler { /// A trie node. class _Node { - shelf.Handler handler; + shelf.Handler? handler; final children = Map(); } @@ -477,7 +478,7 @@ class BrowserManager { /// The channel used to communicate with the browser. /// /// This is connected to a page running `static/host.dart`. - MultiChannel _channel; + late final MultiChannel _channel; /// A pool that ensures that limits the number of initial connections the /// manager will wait for at once. @@ -501,13 +502,13 @@ class BrowserManager { /// /// This will be `null` as long as the browser isn't displaying a pause /// screen. - CancelableCompleter _pauseCompleter; + CancelableCompleter? _pauseCompleter; /// The controller for [_BrowserEnvironment.onRestart]. final _onRestartController = StreamController.broadcast(); /// The environment to attach to each suite. - Future<_BrowserEnvironment> _environment; + late final Future<_BrowserEnvironment> _environment; /// Controllers for every suite in this browser. /// @@ -519,7 +520,7 @@ class BrowserManager { // // Because the browser stops running code when the user is actively debugging, // this lets us detect whether they're debugging reasonably accurately. - RestartableTimer _timer; + late final RestartableTimer _timer; /// Starts the browser identified by [runtime] and has it connect to [url]. /// @@ -532,7 +533,7 @@ class BrowserManager { /// /// Returns the browser manager, or throws an [Exception] if a /// connection fails to be established. - static Future start( + static Future start( Runtime runtime, Uri url, Future future, {bool debug = false}) { var browser = _newBrowser(url, runtime, debug: debug); @@ -544,7 +545,7 @@ class BrowserManager { // websocket is available. Therefore do not throw an error if process // exits with exitCode 0. Note that `browser` will throw and error if the // exit code was not 0, which will be processed by the next callback. - browser.onExit.catchError((dynamic error, StackTrace stackTrace) { + browser.onExit.catchError((Object error, StackTrace stackTrace) { if (completer.isCompleted) { return; } @@ -556,7 +557,7 @@ class BrowserManager { return; } completer.complete(BrowserManager._(browser, runtime, webSocket)); - }).catchError((dynamic error, StackTrace stackTrace) { + }).catchError((Object error, StackTrace stackTrace) { browser.close(); if (completer.isCompleted) { return null; @@ -631,7 +632,7 @@ class BrowserManager { }))); var suiteID = _suiteID++; - RunnerSuiteController controller; + RunnerSuiteController? controller; void closeIframe() { if (_closed) { return; @@ -670,7 +671,7 @@ class BrowserManager { 'build', pathToTest, sourceMapFileName); PackageConfig packageConfig = - await loadPackageConfigUri(await Isolate.packageConfig); + await loadPackageConfigUri((await Isolate.packageConfig)!); Map packageMap = { for (var p in packageConfig.packages) p.name: p.packageUriRoot }; @@ -681,10 +682,10 @@ class BrowserManager { sdkRoot: p.toUri(sdkDir), ); - controller.channel('test.browser.mapper').sink.add(mapper.serialize()); + controller!.channel('test.browser.mapper').sink.add(mapper.serialize()); - _controllers.add(controller); - return await controller.suite; + _controllers.add(controller!); + return await controller!.suite; } catch (_) { closeIframe(); rethrow; @@ -694,22 +695,24 @@ class BrowserManager { /// An implementation of [Environment.displayPause]. CancelableOperation _displayPause() { - if (_pauseCompleter != null) { - return _pauseCompleter.operation; + CancelableCompleter? pauseCompleter = _pauseCompleter; + if (pauseCompleter != null) { + return pauseCompleter.operation; } - _pauseCompleter = CancelableCompleter(onCancel: () { + pauseCompleter = CancelableCompleter(onCancel: () { _channel.sink.add({'command': 'resume'}); _pauseCompleter = null; }); + _pauseCompleter = pauseCompleter; - _pauseCompleter.operation.value.whenComplete(() { + pauseCompleter.operation.value.whenComplete(() { _pauseCompleter = null; }); _channel.sink.add({'command': 'displayPause'}); - return _pauseCompleter.operation; + return pauseCompleter.operation; } /// The callback for handling messages received from the host page. @@ -723,9 +726,7 @@ class BrowserManager { break; case 'resume': - if (_pauseCompleter != null) { - _pauseCompleter.complete(); - } + _pauseCompleter?.complete(); break; default: @@ -740,9 +741,7 @@ class BrowserManager { Future close() => _closeMemoizer.runOnce(() { _closed = true; _timer.cancel(); - if (_pauseCompleter != null) { - _pauseCompleter.complete(); - } + _pauseCompleter?.complete(); _pauseCompleter = null; _controllers.clear(); return _browser.close(); @@ -758,9 +757,9 @@ class _BrowserEnvironment implements Environment { final supportsDebugging = true; - final Uri observatoryUrl; + final Uri? observatoryUrl; - final Uri remoteDebuggerUrl; + final Uri? remoteDebuggerUrl; final Stream onRestart; diff --git a/engine/src/flutter/lib/web_ui/dev/test_runner.dart b/engine/src/flutter/lib/web_ui/dev/test_runner.dart index 5712d2a653a..95aa7e34b6f 100644 --- a/engine/src/flutter/lib/web_ui/dev/test_runner.dart +++ b/engine/src/flutter/lib/web_ui/dev/test_runner.dart @@ -2,12 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -// @dart = 2.6 import 'dart:async'; import 'dart:io' as io; import 'package:args/command_runner.dart'; -import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; import 'package:pool/pool.dart'; @@ -69,12 +67,14 @@ class TestCommand extends Command with ArgUtils { argParser ..addFlag( 'debug', + defaultsTo: false, help: 'Pauses the browser before running a test, giving you an ' 'opportunity to add breakpoints or inspect loaded code before ' 'running the code.', ) ..addFlag( 'watch', + defaultsTo: false, abbr: 'w', help: 'Run in watch mode so the tests re-run whenever a change is ' 'made.', @@ -148,11 +148,9 @@ class TestCommand extends Command with ArgUtils { @override final String description = 'Run tests.'; - bool get isWatchMode => boolArg('watch'); + bool get isWatchMode => boolArg('watch')!; - bool get failEarly => boolArg('fail-early'); - - TestTypesRequested testTypesRequested = null; + bool get failEarly => boolArg('fail-early')!; /// How many dart2js build tasks are running at the same time. final Pool _pool = Pool(8); @@ -165,14 +163,14 @@ class TestCommand extends Command with ArgUtils { bool _testPreparationReady = false; /// Check the flags to see what type of tests are requested. - TestTypesRequested findTestType() { - if (boolArg('unit-tests-only') && boolArg('integration-tests-only')) { + TestTypesRequested get testType { + if (boolArg('unit-tests-only')! && boolArg('integration-tests-only')!) { throw ArgumentError('Conflicting arguments: unit-tests-only and ' 'integration-tests-only are both set'); - } else if (boolArg('unit-tests-only')) { + } else if (boolArg('unit-tests-only')!) { print('Running the unit tests only'); return TestTypesRequested.unit; - } else if (boolArg('integration-tests-only')) { + } else if (boolArg('integration-tests-only')!) { if (!isChrome && !isSafariOnMacOS && !isFirefox) { throw UnimplementedError( 'Integration tests are only available on Chrome Desktop for now'); @@ -186,11 +184,8 @@ class TestCommand extends Command with ArgUtils { @override Future run() async { SupportedBrowsers.instance - ..argParsers.forEach((t) => t.parseOptions(argResults)); - GeneralTestsArgumentParser.instance.parseOptions(argResults); - - // Check the flags to see what type of integration tests are requested. - testTypesRequested = findTestType(); + ..argParsers.forEach((t) => t.parseOptions(argResults!)); + GeneralTestsArgumentParser.instance.parseOptions(argResults!); if (isSafariOnMacOS) { /// Collect information on the bot. @@ -200,7 +195,7 @@ class TestCommand extends Command with ArgUtils { final Pipeline testPipeline = Pipeline(steps: [ () async => clearTerminalScreen(), - () => runTestsOfType(testTypesRequested), + () => runTests(), ]); await testPipeline.start(); @@ -263,9 +258,9 @@ class TestCommand extends Command with ArgUtils { return message.toString(); } - Future runTestsOfType(TestTypesRequested testTypesRequested) async { + Future runTests() async { try { - switch (testTypesRequested) { + switch (testType) { case TestTypesRequested.unit: return runUnitTests(); case TestTypesRequested.integration: @@ -284,8 +279,6 @@ class TestCommand extends Command with ArgUtils { return await runUnitTests(); } } - throw UnimplementedError( - 'Unknown test type requested: $testTypesRequested'); } on TestFailureException { return true; } @@ -293,7 +286,7 @@ class TestCommand extends Command with ArgUtils { Future runIntegrationTests() async { // Parse additional arguments specific for integration testing. - IntegrationTestsArgumentParser.instance.parseOptions(argResults); + IntegrationTestsArgumentParser.instance.parseOptions(argResults!); await _prepare(); final bool result = await IntegrationTestsManager( browser, useSystemFlutter, doUpdateScreenshotGoldens) @@ -460,23 +453,19 @@ class TestCommand extends Command with ArgUtils { /// /// In this mode the browser pauses before running the test to allow /// you set breakpoints or inspect the code. - bool get isDebug => boolArg('debug'); + bool get isDebug => boolArg('debug')!; /// Paths to targets to run, e.g. a single test. - List get targets => argResults.rest; + List get targets => argResults!.rest; /// The target test files to run. - /// - /// The value can be null if the developer prefers to run all the tests. - List get targetFiles => (targets.isEmpty) - ? null - : targets.map((t) => FilePath.fromCwd(t)).toList(); + List get targetFiles => targets.map((t) => FilePath.fromCwd(t)).toList(); /// Whether all tests should run. bool get runAllTests => targets.isEmpty; /// The name of the browser to run tests in. - String get browser => (argResults != null) ? stringArg('browser') : 'chrome'; + String get browser => stringArg('browser')!; /// Whether [browser] is set to "chrome". bool get isChrome => browser == 'chrome'; @@ -535,14 +524,14 @@ class TestCommand extends Command with ArgUtils { /// Use system flutter instead of cloning the repository. /// /// Read the flag help for more details. Uses PATH to locate flutter. - bool get useSystemFlutter => boolArg('use-system-flutter'); + bool get useSystemFlutter => boolArg('use-system-flutter')!; /// When running screenshot tests writes them to the file system into /// ".dart_tool/goldens". - bool get doUpdateScreenshotGoldens => boolArg('update-screenshot-goldens'); + bool get doUpdateScreenshotGoldens => boolArg('update-screenshot-goldens')!; /// Whether to fetch the goldens repo prior to running tests. - bool get doFetchGoldensRepo => boolArg('fetch-goldens-repo'); + bool get doFetchGoldensRepo => boolArg('fetch-goldens-repo')!; /// Runs all tests specified in [targets]. /// @@ -687,8 +676,10 @@ class TestCommand extends Command with ArgUtils { timestampFile.writeAsStringSync(timestamp); } - Future _buildTestsInParallel( - {List targets, bool forCanvasKit = false}) async { + Future _buildTestsInParallel({ + required List targets, + bool forCanvasKit = false, + }) async { final List buildInputs = targets .map((FilePath f) => TestBuildInput(f, forCanvasKit: forCanvasKit)) .toList(); @@ -776,8 +767,8 @@ class TestCommand extends Command with ArgUtils { /// value if any tests fail. Future _runTestBatch( List testFiles, { - @required int concurrency, - @required bool expectFailure, + required int concurrency, + required bool expectFailure, }) async { final List testArgs = [ ...['-r', 'compact'], @@ -788,7 +779,7 @@ class TestCommand extends Command with ArgUtils { '--reporter=name-only', '--platform=${SupportedBrowsers.instance.supportedBrowserToPlatform[browser]}', '--precompiled=${environment.webUiRootDir.path}/build', - SupportedBrowsers.instance.browserToConfiguration[browser], + SupportedBrowsers.instance.browserToConfiguration[browser]!, '--', ...testFiles.map((f) => f.relativeToWebUi).toList(), ]; @@ -803,7 +794,7 @@ class TestCommand extends Command with ArgUtils { } hack.registerPlatformPlugin([ - SupportedBrowsers.instance.supportedBrowsersToRuntimes[browser] + SupportedBrowsers.instance.supportedBrowsersToRuntimes[browser]! ], () { return BrowserPlatform.start( browser, diff --git a/engine/src/flutter/lib/web_ui/dev/utils.dart b/engine/src/flutter/lib/web_ui/dev/utils.dart index ae81c2e877e..ec86d8ddcc8 100644 --- a/engine/src/flutter/lib/web_ui/dev/utils.dart +++ b/engine/src/flutter/lib/web_ui/dev/utils.dart @@ -246,7 +246,7 @@ typedef AsyncCallback = Future Function(); final List cleanupCallbacks = []; /// Cleanup the remaning processes, close open browsers, delete temp files. -void cleanup() async { +Future cleanup() async { // Cleanup remaining processes if any. if (processesToCleanUp.length > 0) { for (io.Process process in processesToCleanUp) {