Unverified Commit 1edec6fc authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Revert "[flutter_tools] refactor drive launch into separate service, split by...

Revert "[flutter_tools] refactor drive launch into separate service, split by mobile+desktop and web (#68451)" (#68845)

This reverts commit 2e75f52a.
parent b25ce5b1
...@@ -95,10 +95,7 @@ Future<void> main(List<String> args) async { ...@@ -95,10 +95,7 @@ Future<void> main(List<String> args) async {
DevicesCommand(), DevicesCommand(),
DoctorCommand(verbose: verbose), DoctorCommand(verbose: verbose),
DowngradeCommand(), DowngradeCommand(),
DriveCommand(verboseHelp: verboseHelp, DriveCommand(verboseHelp: verboseHelp),
fileSystem: globals.fs,
logger: globals.logger,
),
EmulatorsCommand(), EmulatorsCommand(),
FormatCommand(), FormatCommand(),
GenerateCommand(), GenerateCommand(),
......
...@@ -590,17 +590,13 @@ class AndroidDevice extends Device { ...@@ -590,17 +590,13 @@ class AndroidDevice extends Device {
} }
final bool traceStartup = platformArgs['trace-startup'] as bool ?? false; final bool traceStartup = platformArgs['trace-startup'] as bool ?? false;
_logger.printTrace('$this startApp');
ProtocolDiscovery observatoryDiscovery; ProtocolDiscovery observatoryDiscovery;
if (debuggingOptions.debuggingEnabled) { if (debuggingOptions.debuggingEnabled) {
observatoryDiscovery = ProtocolDiscovery.observatory( observatoryDiscovery = ProtocolDiscovery.observatory(
// Avoid using getLogReader, which returns a singleton instance, because the await getLogReader(),
// observatory discovery will dipose at the end. creating a new logger here allows
// logs to be surfaced normally during `flutter drive`.
await AdbLogReader.createLogReader(
this,
_processManager,
),
portForwarder: portForwarder, portForwarder: portForwarder,
hostPort: debuggingOptions.hostVmServicePort, hostPort: debuggingOptions.hostVmServicePort,
devicePort: debuggingOptions.deviceVmServicePort, devicePort: debuggingOptions.deviceVmServicePort,
...@@ -673,6 +669,8 @@ class AndroidDevice extends Device { ...@@ -673,6 +669,8 @@ class AndroidDevice extends Device {
// Wait for the service protocol port here. This will complete once the // Wait for the service protocol port here. This will complete once the
// device has printed "Observatory is listening on...". // device has printed "Observatory is listening on...".
_logger.printTrace('Waiting for observatory port to be available...'); _logger.printTrace('Waiting for observatory port to be available...');
// TODO(danrubel): Waiting for observatory services can be made common across all devices.
try { try {
Uri observatoryUri; Uri observatoryUri;
if (debuggingOptions.buildInfo.isDebug || debuggingOptions.buildInfo.isProfile) { if (debuggingOptions.buildInfo.isDebug || debuggingOptions.buildInfo.isProfile) {
......
...@@ -22,6 +22,9 @@ import 'assets.dart'; ...@@ -22,6 +22,9 @@ import 'assets.dart';
import 'common.dart'; import 'common.dart';
import 'localizations.dart'; import 'localizations.dart';
/// Whether web builds should call the platform initialization logic.
const String kInitializePlatform = 'InitializePlatform';
/// Whether the application has web plugins. /// Whether the application has web plugins.
const String kHasWebPlugins = 'HasWebPlugins'; const String kHasWebPlugins = 'HasWebPlugins';
...@@ -86,6 +89,7 @@ class WebEntrypointTarget extends Target { ...@@ -86,6 +89,7 @@ class WebEntrypointTarget extends Target {
@override @override
Future<void> build(Environment environment) async { Future<void> build(Environment environment) async {
final String targetFile = environment.defines[kTargetFile]; final String targetFile = environment.defines[kTargetFile];
final bool shouldInitializePlatform = environment.defines[kInitializePlatform] == 'true';
final bool hasPlugins = environment.defines[kHasWebPlugins] == 'true'; final bool hasPlugins = environment.defines[kHasWebPlugins] == 'true';
final Uri importUri = environment.fileSystem.file(targetFile).absolute.uri; final Uri importUri = environment.fileSystem.file(targetFile).absolute.uri;
// TODO(jonahwilliams): support configuration of this file. // TODO(jonahwilliams): support configuration of this file.
...@@ -133,7 +137,9 @@ import '$mainImport' as entrypoint; ...@@ -133,7 +137,9 @@ import '$mainImport' as entrypoint;
Future<void> main() async { Future<void> main() async {
registerPlugins(webPluginRegistry); registerPlugins(webPluginRegistry);
await ui.webOnlyInitializePlatform(); if ($shouldInitializePlatform) {
await ui.webOnlyInitializePlatform();
}
entrypoint.main(); entrypoint.main();
} }
'''; ''';
...@@ -146,7 +152,9 @@ import 'dart:ui' as ui; ...@@ -146,7 +152,9 @@ import 'dart:ui' as ui;
import '$mainImport' as entrypoint; import '$mainImport' as entrypoint;
Future<void> main() async { Future<void> main() async {
await ui.webOnlyInitializePlatform(); if ($shouldInitializePlatform) {
await ui.webOnlyInitializePlatform();
}
entrypoint.main(); entrypoint.main();
} }
'''; ''';
......
...@@ -384,6 +384,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. ...@@ -384,6 +384,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
final FlutterDevice flutterDevice = await FlutterDevice.create( final FlutterDevice flutterDevice = await FlutterDevice.create(
device, device,
flutterProject: flutterProject,
fileSystemRoots: stringsArg('filesystem-root'), fileSystemRoots: stringsArg('filesystem-root'),
fileSystemScheme: stringArg('filesystem-scheme'), fileSystemScheme: stringArg('filesystem-scheme'),
target: stringArg('target'), target: stringArg('target'),
......
...@@ -25,6 +25,12 @@ class BuildWebCommand extends BuildSubCommand { ...@@ -25,6 +25,12 @@ class BuildWebCommand extends BuildSubCommand {
usesDartDefineOption(); usesDartDefineOption();
addEnableExperimentation(hide: !verboseHelp); addEnableExperimentation(hide: !verboseHelp);
addNullSafetyModeOptions(hide: !verboseHelp); addNullSafetyModeOptions(hide: !verboseHelp);
argParser.addFlag('web-initialize-platform',
defaultsTo: true,
negatable: true,
hide: true,
help: 'Whether to automatically invoke webOnlyInitializePlatform.',
);
argParser.addFlag('csp', argParser.addFlag('csp',
defaultsTo: false, defaultsTo: false,
negatable: false, negatable: false,
...@@ -86,6 +92,7 @@ class BuildWebCommand extends BuildSubCommand { ...@@ -86,6 +92,7 @@ class BuildWebCommand extends BuildSubCommand {
flutterProject, flutterProject,
target, target,
buildInfo, buildInfo,
boolArg('web-initialize-platform'),
boolArg('csp'), boolArg('csp'),
stringArg('pwa-strategy'), stringArg('pwa-strategy'),
boolArg('source-maps') boolArg('source-maps')
......
...@@ -466,6 +466,7 @@ class AppDomain extends Domain { ...@@ -466,6 +466,7 @@ class AppDomain extends Domain {
final FlutterDevice flutterDevice = await FlutterDevice.create( final FlutterDevice flutterDevice = await FlutterDevice.create(
device, device,
flutterProject: flutterProject,
target: target, target: target,
buildInfo: options.buildInfo, buildInfo: options.buildInfo,
platform: globals.platform, platform: globals.platform,
......
...@@ -847,6 +847,7 @@ class DebuggingOptions { ...@@ -847,6 +847,7 @@ class DebuggingOptions {
this.disablePortPublication = false, this.disablePortPublication = false,
this.deviceVmServicePort, this.deviceVmServicePort,
this.ddsPort, this.ddsPort,
this.initializePlatform = true,
this.hostname, this.hostname,
this.port, this.port,
this.webEnableExposeUrl, this.webEnableExposeUrl,
...@@ -861,6 +862,7 @@ class DebuggingOptions { ...@@ -861,6 +862,7 @@ class DebuggingOptions {
}) : debuggingEnabled = true; }) : debuggingEnabled = true;
DebuggingOptions.disabled(this.buildInfo, { DebuggingOptions.disabled(this.buildInfo, {
this.initializePlatform = true,
this.port, this.port,
this.hostname, this.hostname,
this.webEnableExposeUrl, this.webEnableExposeUrl,
...@@ -911,6 +913,8 @@ class DebuggingOptions { ...@@ -911,6 +913,8 @@ class DebuggingOptions {
final bool purgePersistentCache; final bool purgePersistentCache;
final bool useTestFonts; final bool useTestFonts;
final bool verboseSystemLogs; final bool verboseSystemLogs;
/// Whether to invoke webOnlyInitializePlatform in Flutter for web.
final bool initializePlatform;
final int hostVmServicePort; final int hostVmServicePort;
final int deviceVmServicePort; final int deviceVmServicePort;
final bool disablePortPublication; final bool disablePortPublication;
......
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:dds/dds.dart' as dds;
import 'package:file/file.dart';
import 'package:meta/meta.dart';
import 'package:vm_service/vm_service.dart' as vm_service;
import '../application_package.dart';
import '../base/common.dart';
import '../base/logger.dart';
import '../base/process.dart';
import '../build_info.dart';
import '../device.dart';
import '../vmservice.dart';
import 'web_driver_service.dart';
class FlutterDriverFactory {
FlutterDriverFactory({
@required ApplicationPackageFactory applicationPackageFactory,
@required Logger logger,
@required ProcessUtils processUtils,
@required String dartSdkPath,
}) : _applicationPackageFactory = applicationPackageFactory,
_logger = logger,
_processUtils = processUtils,
_dartSdkPath = dartSdkPath;
final ApplicationPackageFactory _applicationPackageFactory;
final Logger _logger;
final ProcessUtils _processUtils;
final String _dartSdkPath;
/// Create a driver service for running `flutter drive`.
DriverService createDriverService(bool web) {
if (web) {
return WebDriverService(
processUtils: _processUtils,
dartSdkPath: _dartSdkPath,
);
}
return FlutterDriverService(
logger: _logger,
processUtils: _processUtils,
dartSdkPath: _dartSdkPath,
applicationPackageFactory: _applicationPackageFactory,
);
}
}
/// An interface for the `flutter driver` integration test operations.
abstract class DriverService {
/// Install and launch the application for the provided [device].
Future<void> start(
BuildInfo buildInfo,
Device device,
DebuggingOptions debuggingOptions,
bool ipv6, {
File applicationBinary,
String route,
String userIdentifier,
String mainPath,
Map<String, Object> platformArgs = const <String, Object>{},
});
/// Start the test file with the provided [arguments] and [environment], returning
/// the test process exit code.
Future<int> startTest(
String testFile,
List<String> arguments,
Map<String, String> environment, {
bool headless,
String chromeBinary,
String browserName,
bool androidEmulator,
int driverPort,
List<String> browserDimension,
});
/// Stop the running application and uninstall it from the device.
///
/// If [writeSkslOnExit] is non-null, will connect to the VM Service
/// and write SkSL to the file. This is only supported on mobile and
/// desktop devices.
Future<void> stop({
File writeSkslOnExit,
String userIdentifier,
});
}
/// An implementation of the driver service that connects to mobile and desktop
/// applications.
class FlutterDriverService extends DriverService {
FlutterDriverService({
@required ApplicationPackageFactory applicationPackageFactory,
@required Logger logger,
@required ProcessUtils processUtils,
@required String dartSdkPath,
@visibleForTesting VMServiceConnector vmServiceConnector = connectToVmService,
}) : _applicationPackageFactory = applicationPackageFactory,
_logger = logger,
_processUtils = processUtils,
_dartSdkPath = dartSdkPath,
_vmServiceConnector = vmServiceConnector;
static const int _kLaunchAttempts = 3;
final ApplicationPackageFactory _applicationPackageFactory;
final Logger _logger;
final ProcessUtils _processUtils;
final String _dartSdkPath;
final VMServiceConnector _vmServiceConnector;
Device _device;
ApplicationPackage _applicationPackage;
String _vmServiceUri;
vm_service.VmService _vmService;
@override
Future<void> start(
BuildInfo buildInfo,
Device device,
DebuggingOptions debuggingOptions,
bool ipv6, {
File applicationBinary,
String route,
String userIdentifier,
Map<String, Object> platformArgs = const <String, Object>{},
String mainPath,
}) async {
if (buildInfo.isRelease) {
throwToolExit(
'Flutter Driver (non-web) does not support running in release mode.\n'
'\n'
'Use --profile mode for testing application performance.\n'
'Use --debug (default) mode for testing correctness (with assertions).'
);
}
_device = device;
final TargetPlatform targetPlatform = await device.targetPlatform;
_applicationPackage = await _applicationPackageFactory.getPackageForPlatform(
targetPlatform,
buildInfo: buildInfo,
applicationBinary: applicationBinary,
);
int attempt = 0;
LaunchResult result;
bool prebuiltApplication = applicationBinary != null;
while (attempt < _kLaunchAttempts) {
result = await device.startApp(
_applicationPackage,
mainPath: mainPath,
route: route,
debuggingOptions: debuggingOptions,
platformArgs: platformArgs,
userIdentifier: userIdentifier,
prebuiltApplication: prebuiltApplication,
);
if (result != null && result.started) {
break;
}
// On attempts past 1, assume the application is built correctly and re-use it.
attempt += 1;
prebuiltApplication = true;
_logger.printError('Application failed to start on attempt: $attempt');
}
if (result == null || !result.started) {
throwToolExit('Application failed to start. Will not run test. Quitting.', exitCode: 1);
}
_vmServiceUri = result.observatoryUri.toString();
try {
await device.dds.startDartDevelopmentService(
result.observatoryUri,
debuggingOptions.ddsPort,
ipv6,
debuggingOptions.disableServiceAuthCodes,
);
_vmServiceUri = device.dds.uri.toString();
} on dds.DartDevelopmentServiceException {
// If there's another flutter_tools instance still connected to the target
// application, DDS will already be running remotely and this call will fail.
// This can be ignored to continue to use the existing remote DDS instance.
}
_vmService = await _vmServiceConnector(Uri.parse(_vmServiceUri), device: _device);
final DeviceLogReader logReader = await device.getLogReader(app: _applicationPackage);
logReader.logLines.listen(_logger.printStatus);
final vm_service.VM vm = await _vmService.getVM();
logReader.appPid = vm.pid;
}
@override
Future<int> startTest(
String testFile,
List<String> arguments,
Map<String, String> environment, {
bool headless,
String chromeBinary,
String browserName,
bool androidEmulator,
int driverPort,
List<String> browserDimension,
}) async {
return _processUtils.stream(<String>[
_dartSdkPath,
...arguments,
testFile,
'-rexpanded',
], environment: <String, String>{
'VM_SERVICE_URL': _vmServiceUri,
...environment,
});
}
@override
Future<void> stop({
File writeSkslOnExit,
String userIdentifier,
}) async {
if (writeSkslOnExit != null) {
final FlutterView flutterView = (await _vmService.getFlutterViews()).first;
final Map<String, Object> result = await _vmService.getSkSLs(
viewId: flutterView.id
);
await sharedSkSlWriter(_device, result, outputFile: writeSkslOnExit, logger: _logger);
}
try {
if (!await _device.stopApp(_applicationPackage, userIdentifier: userIdentifier)) {
_logger.printError('Failed to stop app');
}
} on Exception catch (err) {
_logger.printError('Failed to stop app due to unhandled error: $err');
}
try {
if (!await _device.uninstallApp(_applicationPackage, userIdentifier: userIdentifier)) {
_logger.printError('Failed to uninstall app');
}
} on Exception catch (err) {
_logger.printError('Failed to uninstall app due to unhandled error: $err');
}
await _device.dispose();
}
}
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'dart:async';
import 'dart:math' as math;
import 'package:file/file.dart';
import 'package:meta/meta.dart';
import 'package:webdriver/async_io.dart' as async_io;
import '../base/common.dart';
import '../base/process.dart';
import '../build_info.dart';
import '../convert.dart';
import '../device.dart';
import '../globals.dart' as globals;
import '../project.dart';
import '../resident_runner.dart';
import '../web/web_runner.dart';
import 'drive_service.dart';
/// An implementation of the driver service for web debug and release applications.
class WebDriverService extends DriverService {
WebDriverService({
@required ProcessUtils processUtils,
@required String dartSdkPath,
}) : _processUtils = processUtils,
_dartSdkPath = dartSdkPath;
final ProcessUtils _processUtils;
final String _dartSdkPath;
ResidentRunner _residentRunner;
Uri _webUri;
@override
Future<void> start(
BuildInfo buildInfo,
Device device,
DebuggingOptions debuggingOptions,
bool ipv6, {
File applicationBinary,
String route,
String userIdentifier,
String mainPath,
Map<String, Object> platformArgs = const <String, Object>{},
}) async {
final FlutterDevice flutterDevice = await FlutterDevice.create(
device,
target: mainPath,
buildInfo: buildInfo,
platform: globals.platform,
);
_residentRunner = webRunnerFactory.createWebRunner(
flutterDevice,
target: mainPath,
ipv6: ipv6,
debuggingOptions: debuggingOptions,
stayResident: false,
urlTunneller: null,
flutterProject: FlutterProject.current(),
);
final Completer<void> appStartedCompleter = Completer<void>.sync();
final int result = await _residentRunner.run(
appStartedCompleter: appStartedCompleter,
route: route,
);
_webUri = _residentRunner.uri;
if (result != 0) {
throwToolExit(null);
}
}
@override
Future<int> startTest(String testFile, List<String> arguments, Map<String, String> environment, {
bool headless,
String chromeBinary,
String browserName,
bool androidEmulator,
int driverPort,
List<String> browserDimension,
}) async {
async_io.WebDriver webDriver;
final Browser browser = _browserNameToEnum(browserName);
try {
webDriver = await async_io.createDriver(
uri: Uri.parse('http://localhost:$driverPort/'),
desired: getDesiredCapabilities(browser, headless, chromeBinary),
spec: async_io.WebDriverSpec.Auto
);
} on Exception catch (ex) {
throwToolExit(
'Unable to start WebDriver Session for Flutter for Web testing. \n'
'Make sure you have the correct WebDriver Server running at $driverPort. \n'
'Make sure the WebDriver Server matches option --browser-name. \n'
'$ex'
);
}
final bool isAndroidChrome = browser == Browser.androidChrome;
// Do not set the window size for android chrome browser.
if (!isAndroidChrome) {
assert(browserDimension.length == 2);
int x;
int y;
try {
x = int.parse(browserDimension[0]);
y = int.parse(browserDimension[1]);
} on FormatException catch (ex) {
throwToolExit('Dimension provided to --browser-dimension is invalid: $ex');
}
final async_io.Window window = await webDriver.window;
await window.setLocation(const math.Point<int>(0, 0));
await window.setSize(math.Rectangle<int>(0, 0, x, y));
}
final int result = await _processUtils.stream(<String>[
_dartSdkPath,
...arguments,
testFile,
'-rexpanded',
], environment: <String, String>{
'VM_SERVICE_URL': _webUri.toString(),
..._additionalDriverEnvironment(webDriver, browserName, androidEmulator),
...environment,
});
await webDriver.quit();
return result;
}
@override
Future<void> stop({File writeSkslOnExit, String userIdentifier}) async {
await _residentRunner.cleanupAtFinish();
}
Map<String, String> _additionalDriverEnvironment(async_io.WebDriver webDriver, String browserName, bool androidEmulator) {
return <String, String>{
'DRIVER_SESSION_ID': webDriver.id,
'DRIVER_SESSION_URI': webDriver.uri.toString(),
'DRIVER_SESSION_SPEC': webDriver.spec.toString(),
'DRIVER_SESSION_CAPABILITIES': json.encode(webDriver.capabilities),
'SUPPORT_TIMELINE_ACTION': (_browserNameToEnum(browserName) == Browser.chrome).toString(),
'FLUTTER_WEB_TEST': 'true',
'ANDROID_CHROME_ON_EMULATOR': (_browserNameToEnum(browserName) == Browser.androidChrome && androidEmulator).toString(),
};
}
}
/// A list of supported browsers.
enum Browser {
/// Chrome on Android: https://developer.chrome.com/multidevice/android/overview
androidChrome,
/// Chrome: https://www.google.com/chrome/
chrome,
/// Edge: https://www.microsoft.com/en-us/windows/microsoft-edge
edge,
/// Firefox: https://www.mozilla.org/en-US/firefox/
firefox,
/// Safari in iOS: https://www.apple.com/safari/
iosSafari,
/// Safari in macOS: https://www.apple.com/safari/
safari,
}
/// Returns desired capabilities for given [browser], [headless] and
/// [chromeBinary].
@visibleForTesting
Map<String, dynamic> getDesiredCapabilities(Browser browser, bool headless, [String chromeBinary]) {
switch (browser) {
case Browser.chrome:
return <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'chrome',
'goog:loggingPrefs': <String, String>{ async_io.LogType.performance: 'ALL'},
'chromeOptions': <String, dynamic>{
if (chromeBinary != null)
'binary': chromeBinary,
'w3c': false,
'args': <String>[
'--bwsi',
'--disable-background-timer-throttling',
'--disable-default-apps',
'--disable-extensions',
'--disable-popup-blocking',
'--disable-translate',
'--no-default-browser-check',
'--no-sandbox',
'--no-first-run',
if (headless) '--headless'
],
'perfLoggingPrefs': <String, String>{
'traceCategories':
'devtools.timeline,'
'v8,blink.console,benchmark,blink,'
'blink.user_timing'
}
},
};
break;
case Browser.firefox:
return <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'firefox',
'moz:firefoxOptions' : <String, dynamic>{
'args': <String>[
if (headless) '-headless'
],
'prefs': <String, dynamic>{
'dom.file.createInChild': true,
'dom.timeout.background_throttling_max_budget': -1,
'media.autoplay.default': 0,
'media.gmp-manager.url': '',
'media.gmp-provider.enabled': false,
'network.captive-portal-service.enabled': false,
'security.insecure_field_warning.contextual.enabled': false,
'test.currentTimeOffsetSeconds': 11491200
},
'log': <String, String>{'level': 'trace'}
}
};
break;
case Browser.edge:
return <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'edge',
};
break;
case Browser.safari:
return <String, dynamic>{
'browserName': 'safari',
};
break;
case Browser.iosSafari:
return <String, dynamic>{
'platformName': 'ios',
'browserName': 'safari',
'safari:useSimulator': true
};
case Browser.androidChrome:
return <String, dynamic>{
'browserName': 'chrome',
'platformName': 'android',
'goog:chromeOptions': <String, dynamic>{
'androidPackage': 'com.android.chrome',
'args': <String>['--disable-fullscreen']
},
};
default:
throw UnsupportedError('Browser $browser not supported.');
}
}
/// Converts [browserName] string to [Browser]
Browser _browserNameToEnum(String browserName){
switch (browserName) {
case 'android-chrome': return Browser.androidChrome;
case 'chrome': return Browser.chrome;
case 'edge': return Browser.edge;
case 'firefox': return Browser.firefox;
case 'ios-safari': return Browser.iosSafari;
case 'safari': return Browser.safari;
}
throw UnsupportedError('Browser $browserName not supported');
}
...@@ -528,6 +528,7 @@ class _ResidentWebRunner extends ResidentWebRunner { ...@@ -528,6 +528,7 @@ class _ResidentWebRunner extends ResidentWebRunner {
flutterProject, flutterProject,
target, target,
debuggingOptions.buildInfo, debuggingOptions.buildInfo,
debuggingOptions.initializePlatform,
false, false,
kNoneWorker, kNoneWorker,
true, true,
...@@ -595,6 +596,7 @@ class _ResidentWebRunner extends ResidentWebRunner { ...@@ -595,6 +596,7 @@ class _ResidentWebRunner extends ResidentWebRunner {
flutterProject, flutterProject,
target, target,
debuggingOptions.buildInfo, debuggingOptions.buildInfo,
debuggingOptions.initializePlatform,
false, false,
kNoneWorker, kNoneWorker,
true, true,
......
...@@ -70,6 +70,7 @@ class FlutterDevice { ...@@ -70,6 +70,7 @@ class FlutterDevice {
/// Create a [FlutterDevice] with optional code generation enabled. /// Create a [FlutterDevice] with optional code generation enabled.
static Future<FlutterDevice> create( static Future<FlutterDevice> create(
Device device, { Device device, {
@required FlutterProject flutterProject,
@required String target, @required String target,
@required BuildInfo buildInfo, @required BuildInfo buildInfo,
@required Platform platform, @required Platform platform,
......
...@@ -8,7 +8,6 @@ import 'package:vm_service/vm_service.dart' as vm_service; ...@@ -8,7 +8,6 @@ import 'package:vm_service/vm_service.dart' as vm_service;
import 'base/context.dart'; import 'base/context.dart';
import 'base/io.dart' as io; import 'base/io.dart' as io;
import 'base/logger.dart';
import 'build_info.dart'; import 'build_info.dart';
import 'convert.dart'; import 'convert.dart';
import 'device.dart'; import 'device.dart';
...@@ -807,11 +806,9 @@ bool isPauseEvent(String kind) { ...@@ -807,11 +806,9 @@ bool isPauseEvent(String kind) {
// or delete it. // or delete it.
Future<String> sharedSkSlWriter(Device device, Map<String, Object> data, { Future<String> sharedSkSlWriter(Device device, Map<String, Object> data, {
File outputFile, File outputFile,
Logger logger,
}) async { }) async {
logger ??= globals.logger;
if (data.isEmpty) { if (data.isEmpty) {
logger.printStatus( globals.logger.printStatus(
'No data was received. To ensure SkSL data can be generated use a ' 'No data was received. To ensure SkSL data can be generated use a '
'physical device then:\n' 'physical device then:\n'
' 1. Pass "--cache-sksl" as an argument to flutter run.\n' ' 1. Pass "--cache-sksl" as an argument to flutter run.\n'
...@@ -847,7 +844,7 @@ Future<String> sharedSkSlWriter(Device device, Map<String, Object> data, { ...@@ -847,7 +844,7 @@ Future<String> sharedSkSlWriter(Device device, Map<String, Object> data, {
'data': data, 'data': data,
}; };
outputFile.writeAsStringSync(json.encode(manifest)); outputFile.writeAsStringSync(json.encode(manifest));
logger.printStatus('Wrote SkSL data to ${outputFile.path}.'); globals.logger.printStatus('Wrote SkSL data to ${outputFile.path}.');
return outputFile.path; return outputFile.path;
} }
......
...@@ -26,6 +26,7 @@ Future<void> buildWeb( ...@@ -26,6 +26,7 @@ Future<void> buildWeb(
FlutterProject flutterProject, FlutterProject flutterProject,
String target, String target,
BuildInfo buildInfo, BuildInfo buildInfo,
bool initializePlatform,
bool csp, bool csp,
String serviceWorkerStrategy, String serviceWorkerStrategy,
bool sourceMaps, bool sourceMaps,
...@@ -53,6 +54,7 @@ Future<void> buildWeb( ...@@ -53,6 +54,7 @@ Future<void> buildWeb(
defines: <String, String>{ defines: <String, String>{
kBuildMode: getNameForBuildMode(buildInfo.mode), kBuildMode: getNameForBuildMode(buildInfo.mode),
kTargetFile: target, kTargetFile: target,
kInitializePlatform: initializePlatform.toString(),
kHasWebPlugins: hasWebPlugins.toString(), kHasWebPlugins: hasWebPlugins.toString(),
kDartDefines: encodeDartDefines(buildInfo.dartDefines), kDartDefines: encodeDartDefines(buildInfo.dartDefines),
kCspMode: csp.toString(), kCspMode: csp.toString(),
......
...@@ -54,6 +54,7 @@ void main() { ...@@ -54,6 +54,7 @@ void main() {
fileSystem.path.join('lib', 'main.dart'), fileSystem.path.join('lib', 'main.dart'),
BuildInfo.debug, BuildInfo.debug,
false, false,
false,
null, null,
true, true,
), throwsToolExit()); ), throwsToolExit());
......
...@@ -77,6 +77,7 @@ void main() { ...@@ -77,6 +77,7 @@ void main() {
..writeAsStringSync('void main() {}'); ..writeAsStringSync('void main() {}');
environment.defines[kTargetFile] = mainFile.path; environment.defines[kTargetFile] = mainFile.path;
environment.defines[kHasWebPlugins] = 'true'; environment.defines[kHasWebPlugins] = 'true';
environment.defines[kInitializePlatform] = 'true';
await const WebEntrypointTarget().build(environment); await const WebEntrypointTarget().build(environment);
final String generated = environment.buildDir.childFile('main.dart').readAsStringSync(); final String generated = environment.buildDir.childFile('main.dart').readAsStringSync();
...@@ -85,6 +86,9 @@ void main() { ...@@ -85,6 +86,9 @@ void main() {
expect(generated, contains("import 'package:foo/generated_plugin_registrant.dart';")); expect(generated, contains("import 'package:foo/generated_plugin_registrant.dart';"));
expect(generated, contains('registerPlugins(webPluginRegistry);')); expect(generated, contains('registerPlugins(webPluginRegistry);'));
// Platform
expect(generated, contains('if (true) {'));
// Main // Main
expect(generated, contains('entrypoint.main();')); expect(generated, contains('entrypoint.main();'));
...@@ -179,6 +183,7 @@ void main() { ...@@ -179,6 +183,7 @@ void main() {
environment.defines[kTargetFile] = mainFile.path; environment.defines[kTargetFile] = mainFile.path;
environment.defines[kHasWebPlugins] = 'true'; environment.defines[kHasWebPlugins] = 'true';
environment.defines[kInitializePlatform] = 'true';
await const WebEntrypointTarget().build(environment); await const WebEntrypointTarget().build(environment);
final String generated = environment.buildDir.childFile('main.dart').readAsStringSync(); final String generated = environment.buildDir.childFile('main.dart').readAsStringSync();
...@@ -187,6 +192,9 @@ void main() { ...@@ -187,6 +192,9 @@ void main() {
expect(generated, contains("import 'package:foo/generated_plugin_registrant.dart';")); expect(generated, contains("import 'package:foo/generated_plugin_registrant.dart';"));
expect(generated, contains('registerPlugins(webPluginRegistry);')); expect(generated, contains('registerPlugins(webPluginRegistry);'));
// Platform
expect(generated, contains('if (true) {'));
// Main // Main
expect(generated, contains('entrypoint.main();')); expect(generated, contains('entrypoint.main();'));
...@@ -202,6 +210,7 @@ void main() { ...@@ -202,6 +210,7 @@ void main() {
..writeAsStringSync('void main() {}'); ..writeAsStringSync('void main() {}');
environment.defines[kTargetFile] = mainFile.path; environment.defines[kTargetFile] = mainFile.path;
environment.defines[kHasWebPlugins] = 'false'; environment.defines[kHasWebPlugins] = 'false';
environment.defines[kInitializePlatform] = 'true';
await const WebEntrypointTarget().build(environment); await const WebEntrypointTarget().build(environment);
final String generated = environment.buildDir.childFile('main.dart').readAsStringSync(); final String generated = environment.buildDir.childFile('main.dart').readAsStringSync();
...@@ -209,6 +218,32 @@ void main() { ...@@ -209,6 +218,32 @@ void main() {
// Plugins // Plugins
expect(generated, isNot(contains("import 'package:foo/generated_plugin_registrant.dart';"))); expect(generated, isNot(contains("import 'package:foo/generated_plugin_registrant.dart';")));
expect(generated, isNot(contains('registerPlugins(webPluginRegistry);'))); expect(generated, isNot(contains('registerPlugins(webPluginRegistry);')));
// Platform
expect(generated, contains('if (true) {'));
// Main
expect(generated, contains('entrypoint.main();'));
}));
test('WebEntrypointTarget generates an entrypoint with plugins and without init platform', () => testbed.run(() async {
final File mainFile = globals.fs.file(globals.fs.path.join('foo', 'lib', 'main.dart'))
..createSync(recursive: true)
..writeAsStringSync('void main() {}');
environment.defines[kTargetFile] = mainFile.path;
environment.defines[kHasWebPlugins] = 'true';
environment.defines[kInitializePlatform] = 'false';
await const WebEntrypointTarget().build(environment);
final String generated = environment.buildDir.childFile('main.dart').readAsStringSync();
// Plugins
expect(generated, contains("import 'package:foo/generated_plugin_registrant.dart';"));
expect(generated, contains('registerPlugins(webPluginRegistry);'));
// Platform
expect(generated, contains('if (false) {'));
// Main // Main
expect(generated, contains('entrypoint.main();')); expect(generated, contains('entrypoint.main();'));
})); }));
...@@ -247,6 +282,7 @@ void main() { ...@@ -247,6 +282,7 @@ void main() {
..writeAsStringSync('void main() {}'); ..writeAsStringSync('void main() {}');
environment.defines[kTargetFile] = mainFile.path; environment.defines[kTargetFile] = mainFile.path;
environment.defines[kHasWebPlugins] = 'false'; environment.defines[kHasWebPlugins] = 'false';
environment.defines[kInitializePlatform] = 'false';
await const WebEntrypointTarget().build(environment); await const WebEntrypointTarget().build(environment);
final String generated = environment.buildDir.childFile('main.dart').readAsStringSync(); final String generated = environment.buildDir.childFile('main.dart').readAsStringSync();
...@@ -255,6 +291,9 @@ void main() { ...@@ -255,6 +291,9 @@ void main() {
expect(generated, isNot(contains("import 'package:foo/generated_plugin_registrant.dart';"))); expect(generated, isNot(contains("import 'package:foo/generated_plugin_registrant.dart';")));
expect(generated, isNot(contains('registerPlugins(webPluginRegistry);'))); expect(generated, isNot(contains('registerPlugins(webPluginRegistry);')));
// Platform
expect(generated, contains('if (false) {'));
// Main // Main
expect(generated, contains('entrypoint.main();')); expect(generated, contains('entrypoint.main();'));
})); }));
......
...@@ -55,7 +55,7 @@ void main() { ...@@ -55,7 +55,7 @@ void main() {
testWithoutContext('Install and uninstall are no-ops that report success', () async { testWithoutContext('Install and uninstall are no-ops that report success', () async {
final FakeDesktopDevice device = setUpDesktopDevice(); final FakeDesktopDevice device = setUpDesktopDevice();
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
expect(await device.uninstallApp(package), true); expect(await device.uninstallApp(package), true);
expect(await device.isAppInstalled(package), true); expect(await device.isAppInstalled(package), true);
...@@ -71,7 +71,7 @@ void main() { ...@@ -71,7 +71,7 @@ void main() {
group('Starting and stopping application', () { group('Starting and stopping application', () {
testWithoutContext('Stop without start is a successful no-op', () async { testWithoutContext('Stop without start is a successful no-op', () async {
final FakeDesktopDevice device = setUpDesktopDevice(); final FakeDesktopDevice device = setUpDesktopDevice();
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
expect(await device.stopApp(package), true); expect(await device.stopApp(package), true);
}); });
...@@ -89,7 +89,7 @@ void main() { ...@@ -89,7 +89,7 @@ void main() {
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager, fileSystem: fileSystem); final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager, fileSystem: fileSystem);
final String executableName = device.executablePathForDevice(null, BuildMode.debug); final String executableName = device.executablePathForDevice(null, BuildMode.debug);
fileSystem.file(executableName).writeAsStringSync('\n'); fileSystem.file(executableName).writeAsStringSync('\n');
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
final LaunchResult result = await device.startApp( final LaunchResult result = await device.startApp(
package, package,
prebuiltApplication: true, prebuiltApplication: true,
...@@ -103,7 +103,7 @@ void main() { ...@@ -103,7 +103,7 @@ void main() {
testWithoutContext('Null executable path fails gracefully', () async { testWithoutContext('Null executable path fails gracefully', () async {
final BufferLogger logger = BufferLogger.test(); final BufferLogger logger = BufferLogger.test();
final DesktopDevice device = setUpDesktopDevice(nullExecutablePathForDevice: true, logger: logger); final DesktopDevice device = setUpDesktopDevice(nullExecutablePathForDevice: true, logger: logger);
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
final LaunchResult result = await device.startApp( final LaunchResult result = await device.startApp(
package, package,
prebuiltApplication: true, prebuiltApplication: true,
...@@ -124,7 +124,7 @@ void main() { ...@@ -124,7 +124,7 @@ void main() {
), ),
]); ]);
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager); final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager);
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
final LaunchResult result = await device.startApp( final LaunchResult result = await device.startApp(
package, package,
prebuiltApplication: true, prebuiltApplication: true,
...@@ -168,7 +168,7 @@ void main() { ...@@ -168,7 +168,7 @@ void main() {
), ),
]); ]);
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager); final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager);
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
final LaunchResult result = await device.startApp( final LaunchResult result = await device.startApp(
package, package,
prebuiltApplication: true, prebuiltApplication: true,
...@@ -191,6 +191,7 @@ void main() { ...@@ -191,6 +191,7 @@ void main() {
purgePersistentCache: true, purgePersistentCache: true,
useTestFonts: true, useTestFonts: true,
verboseSystemLogs: true, verboseSystemLogs: true,
initializePlatform: true,
nullAssertions: true, nullAssertions: true,
), ),
); );
...@@ -216,7 +217,7 @@ void main() { ...@@ -216,7 +217,7 @@ void main() {
), ),
]); ]);
final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager); final FakeDesktopDevice device = setUpDesktopDevice(processManager: processManager);
final FakeApplicationPackage package = FakeApplicationPackage(); final FakeAppplicationPackage package = FakeAppplicationPackage();
final LaunchResult result = await device.startApp( final LaunchResult result = await device.startApp(
package, package,
prebuiltApplication: true, prebuiltApplication: true,
...@@ -227,6 +228,7 @@ void main() { ...@@ -227,6 +228,7 @@ void main() {
BuildInfo.debug, BuildInfo.debug,
traceAllowlist: 'foo,bar', traceAllowlist: 'foo,bar',
cacheSkSL: true, cacheSkSL: true,
initializePlatform: true,
), ),
); );
...@@ -323,7 +325,7 @@ class FakeDesktopDevice extends DesktopDevice { ...@@ -323,7 +325,7 @@ class FakeDesktopDevice extends DesktopDevice {
} }
} }
class FakeApplicationPackage extends Fake implements ApplicationPackage {} class FakeAppplicationPackage extends Fake implements ApplicationPackage {}
class FakeOperatingSystemUtils extends Fake implements OperatingSystemUtils { class FakeOperatingSystemUtils extends Fake implements OperatingSystemUtils {
@override @override
String get name => 'Example'; String get name => 'Example';
......
// Copyright 2014 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'package:flutter_tools/src/drive/web_driver_service.dart';
import 'package:webdriver/sync_io.dart' as sync_io;
import '../../src/common.dart';
void main() {
testWithoutContext('getDesiredCapabilities Chrome with headless on', () {
final Map<String, dynamic> expected = <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'chrome',
'goog:loggingPrefs': <String, String>{ sync_io.LogType.performance: 'ALL'},
'chromeOptions': <String, dynamic>{
'w3c': false,
'args': <String>[
'--bwsi',
'--disable-background-timer-throttling',
'--disable-default-apps',
'--disable-extensions',
'--disable-popup-blocking',
'--disable-translate',
'--no-default-browser-check',
'--no-sandbox',
'--no-first-run',
'--headless'
],
'perfLoggingPrefs': <String, String>{
'traceCategories':
'devtools.timeline,'
'v8,blink.console,benchmark,blink,'
'blink.user_timing'
}
}
};
expect(getDesiredCapabilities(Browser.chrome, true), expected);
});
testWithoutContext('getDesiredCapabilities Chrome with headless off', () {
const String chromeBinary = 'random-binary';
final Map<String, dynamic> expected = <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'chrome',
'goog:loggingPrefs': <String, String>{ sync_io.LogType.performance: 'ALL'},
'chromeOptions': <String, dynamic>{
'binary': chromeBinary,
'w3c': false,
'args': <String>[
'--bwsi',
'--disable-background-timer-throttling',
'--disable-default-apps',
'--disable-extensions',
'--disable-popup-blocking',
'--disable-translate',
'--no-default-browser-check',
'--no-sandbox',
'--no-first-run',
],
'perfLoggingPrefs': <String, String>{
'traceCategories':
'devtools.timeline,'
'v8,blink.console,benchmark,blink,'
'blink.user_timing'
}
}
};
expect(getDesiredCapabilities(Browser.chrome, false, chromeBinary), expected);
});
testWithoutContext('getDesiredCapabilities Firefox with headless on', () {
final Map<String, dynamic> expected = <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'firefox',
'moz:firefoxOptions' : <String, dynamic>{
'args': <String>['-headless'],
'prefs': <String, dynamic>{
'dom.file.createInChild': true,
'dom.timeout.background_throttling_max_budget': -1,
'media.autoplay.default': 0,
'media.gmp-manager.url': '',
'media.gmp-provider.enabled': false,
'network.captive-portal-service.enabled': false,
'security.insecure_field_warning.contextual.enabled': false,
'test.currentTimeOffsetSeconds': 11491200
},
'log': <String, String>{'level': 'trace'}
}
};
expect(getDesiredCapabilities(Browser.firefox, true), expected);
});
testWithoutContext('getDesiredCapabilities Firefox with headless off', () {
final Map<String, dynamic> expected = <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'firefox',
'moz:firefoxOptions' : <String, dynamic>{
'args': <String>[],
'prefs': <String, dynamic>{
'dom.file.createInChild': true,
'dom.timeout.background_throttling_max_budget': -1,
'media.autoplay.default': 0,
'media.gmp-manager.url': '',
'media.gmp-provider.enabled': false,
'network.captive-portal-service.enabled': false,
'security.insecure_field_warning.contextual.enabled': false,
'test.currentTimeOffsetSeconds': 11491200
},
'log': <String, String>{'level': 'trace'}
}
};
expect(getDesiredCapabilities(Browser.firefox, false), expected);
});
testWithoutContext('getDesiredCapabilities Edge', () {
final Map<String, dynamic> expected = <String, dynamic>{
'acceptInsecureCerts': true,
'browserName': 'edge',
};
expect(getDesiredCapabilities(Browser.edge, false), expected);
});
testWithoutContext('getDesiredCapabilities macOS Safari', () {
final Map<String, dynamic> expected = <String, dynamic>{
'browserName': 'safari',
};
expect(getDesiredCapabilities(Browser.safari, false), expected);
});
testWithoutContext('getDesiredCapabilities iOS Safari', () {
final Map<String, dynamic> expected = <String, dynamic>{
'platformName': 'ios',
'browserName': 'safari',
'safari:useSimulator': true
};
expect(getDesiredCapabilities(Browser.iosSafari, false), expected);
});
testWithoutContext('getDesiredCapabilities android chrome', () {
final Map<String, dynamic> expected = <String, dynamic>{
'browserName': 'chrome',
'platformName': 'android',
'goog:chromeOptions': <String, dynamic>{
'androidPackage': 'com.android.chrome',
'args': <String>['--disable-fullscreen']
},
};
expect(getDesiredCapabilities(Browser.androidChrome, false), expected);
});
}
...@@ -24,6 +24,7 @@ import 'package:flutter_tools/src/convert.dart'; ...@@ -24,6 +24,7 @@ import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/devfs.dart'; import 'package:flutter_tools/src/devfs.dart';
import 'package:flutter_tools/src/device.dart'; import 'package:flutter_tools/src/device.dart';
import 'package:flutter_tools/src/globals.dart' as globals; import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/project.dart';
import 'package:flutter_tools/src/reporting/reporting.dart'; import 'package:flutter_tools/src/reporting/reporting.dart';
import 'package:flutter_tools/src/resident_runner.dart'; import 'package:flutter_tools/src/resident_runner.dart';
import 'package:flutter_tools/src/run_cold.dart'; import 'package:flutter_tools/src/run_cold.dart';
...@@ -2385,6 +2386,7 @@ void main() { ...@@ -2385,6 +2386,7 @@ void main() {
treeShakeIcons: false, treeShakeIcons: false,
nullSafetyMode: NullSafetyMode.unsound, nullSafetyMode: NullSafetyMode.unsound,
), ),
flutterProject: FlutterProject.current(),
target: null, target: null,
platform: FakePlatform(operatingSystem: 'linux'), platform: FakePlatform(operatingSystem: 'linux'),
)).generator as DefaultResidentCompiler; )).generator as DefaultResidentCompiler;
...@@ -2419,6 +2421,7 @@ void main() { ...@@ -2419,6 +2421,7 @@ void main() {
treeShakeIcons: false, treeShakeIcons: false,
extraFrontEndOptions: <String>['--enable-experiment=non-nullable'], extraFrontEndOptions: <String>['--enable-experiment=non-nullable'],
), ),
flutterProject: FlutterProject.current(),
target: null, target: null,
platform: FakePlatform(operatingSystem: 'linux'), platform: FakePlatform(operatingSystem: 'linux'),
)).generator as DefaultResidentCompiler; )).generator as DefaultResidentCompiler;
...@@ -2453,6 +2456,7 @@ void main() { ...@@ -2453,6 +2456,7 @@ void main() {
treeShakeIcons: false, treeShakeIcons: false,
extraFrontEndOptions: <String>[], extraFrontEndOptions: <String>[],
), ),
flutterProject: FlutterProject.current(),
target: null, platform: null, target: null, platform: null,
)).generator as DefaultResidentCompiler; )).generator as DefaultResidentCompiler;
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
import 'package:file_testing/file_testing.dart'; import 'package:file_testing/file_testing.dart';
import 'package:flutter_tools/src/base/io.dart'; import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import '../src/common.dart'; import '../src/common.dart';
import 'test_utils.dart'; import 'test_utils.dart';
...@@ -60,7 +61,7 @@ void main() { ...@@ -60,7 +61,7 @@ void main() {
final String outputFilePath = line.split(iosDebugMessage).last.trim(); final String outputFilePath = line.split(iosDebugMessage).last.trim();
expect(fileSystem.file(fileSystem.path.join(woringDirectory, outputFilePath)), exists); expect(fileSystem.file(fileSystem.path.join(woringDirectory, outputFilePath)), exists);
expect(result.exitCode, 0); expect(result.exitCode, 0);
}, skip: true); // Extremely flaky due to https://github.com/flutter/flutter/issues/68144 }, skip: !const LocalPlatform().isMacOS); // Only supported on macOS
testWithoutContext('--analyze-size is only supported in release mode', () async { testWithoutContext('--analyze-size is only supported in release mode', () async {
final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter'); final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter');
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment