From 23a3d1014faffa3d1a87104f1b4c90f95daa1ce7 Mon Sep 17 00:00:00 2001 From: Zachary Anderson Date: Mon, 13 Jan 2020 10:12:06 -0800 Subject: [PATCH] [flutter_tool] Make BotDetector context free (#48605) --- packages/flutter_tools/lib/runner.dart | 39 ++++++------ .../lib/src/base/bot_detector.dart | 60 +++++++++++++++++++ .../flutter_tools/lib/src/base/utils.dart | 51 ---------------- .../flutter_tools/lib/src/context_runner.dart | 2 - packages/flutter_tools/lib/src/dart/pub.dart | 6 +- .../lib/src/reporting/reporting.dart | 2 +- .../lib/src/reporting/usage.dart | 2 +- .../permeable/packages_test.dart | 2 +- ...utils_test.dart => bot_detector_test.dart} | 27 ++------- .../test/general.shard/dart/pub_get_test.dart | 2 +- 10 files changed, 93 insertions(+), 100 deletions(-) create mode 100644 packages/flutter_tools/lib/src/base/bot_detector.dart rename packages/flutter_tools/test/general.shard/base/{utils_test.dart => bot_detector_test.dart} (57%) diff --git a/packages/flutter_tools/lib/runner.dart b/packages/flutter_tools/lib/runner.dart index 61ae09bd26b..9f2e059b607 100644 --- a/packages/flutter_tools/lib/runner.dart +++ b/packages/flutter_tools/lib/runner.dart @@ -9,6 +9,7 @@ import 'package:intl/intl.dart' as intl; import 'package:intl/intl_standalone.dart' as intl_standalone; import 'package:meta/meta.dart'; +import 'src/base/bot_detector.dart'; import 'src/base/common.dart'; import 'src/base/context.dart'; import 'src/base/file_system.dart'; @@ -19,7 +20,7 @@ import 'src/base/terminal.dart'; import 'src/base/utils.dart'; import 'src/context_runner.dart'; import 'src/doctor.dart'; -import 'src/globals.dart'; +import 'src/globals.dart' as globals; import 'src/reporting/github_template.dart'; import 'src/reporting/reporting.dart'; import 'src/runner/flutter_command.dart'; @@ -37,7 +38,7 @@ Future run( String flutterVersion, Map overrides, }) { - reportCrashes ??= !isRunningOnBot; + reportCrashes ??= !isRunningOnBot(globals.platform); if (muteCommandLogging) { // Remove the verbose option; for help and doctor, users don't need to see @@ -90,16 +91,16 @@ Future _handleToolError( String getFlutterVersion(), ) async { if (error is UsageException) { - printError('${error.message}\n'); - printError("Run 'flutter -h' (or 'flutter -h') for available flutter commands and options."); + globals.printError('${error.message}\n'); + globals.printError("Run 'flutter -h' (or 'flutter -h') for available flutter commands and options."); // Argument error exit code. return _exit(64); } else if (error is ToolExit) { if (error.message != null) { - printError(error.message); + globals.printError(error.message); } if (verbose) { - printError('\n$stackTrace\n'); + globals.printError('\n$stackTrace\n'); } return _exit(error.exitCode ?? 1); } else if (error is ProcessExit) { @@ -131,7 +132,7 @@ Future _handleToolError( ); final String errorString = error.toString(); - printError('Oops; flutter has exited unexpectedly: "$errorString".'); + globals.printError('Oops; flutter has exited unexpectedly: "$errorString".'); try { await _informUserOfCrash(args, error, stackTrace, errorString); @@ -155,15 +156,15 @@ Future _informUserOfCrash(List args, dynamic error, StackTrace sta final String doctorText = await _doctorText(); final File file = await _createLocalCrashReport(args, error, stackTrace, doctorText); - printError('A crash report has been written to ${file.path}.'); - printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true); + globals.printError('A crash report has been written to ${file.path}.'); + globals.printStatus('This crash may already be reported. Check GitHub for similar crashes.', emphasis: true); final GitHubTemplateCreator gitHubTemplateCreator = context.get() ?? GitHubTemplateCreator(); final String similarIssuesURL = await gitHubTemplateCreator.toolCrashSimilarIssuesGitHubURL(errorString); - printStatus('$similarIssuesURL\n', wrap: false); - printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true); - printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false); - printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true); + globals.printStatus('$similarIssuesURL\n', wrap: false); + globals.printStatus('To report your crash to the Flutter team, first read the guide to filing a bug.', emphasis: true); + globals.printStatus('https://flutter.dev/docs/resources/bug-reports\n', wrap: false); + globals.printStatus('Create a new GitHub issue by pasting this link into your browser and completing the issue template. Thank you!', emphasis: true); final String command = _crashCommand(args); final String gitHubTemplateURL = await gitHubTemplateCreator.toolCrashIssueTemplateGitHubURL( @@ -173,7 +174,7 @@ Future _informUserOfCrash(List args, dynamic error, StackTrace sta stackTrace, doctorText ); - printStatus('$gitHubTemplateURL\n', wrap: false); + globals.printStatus('$gitHubTemplateURL\n', wrap: false); } String _crashCommand(List args) => 'flutter ${args.join(' ')}'; @@ -214,8 +215,8 @@ Future _createLocalCrashReport(List args, dynamic error, StackTrac try { crashFile.writeAsStringSync(buffer.toString()); } on FileSystemException catch (e) { - printError('Could not write crash report to disk: $e'); - printError(buffer.toString()); + globals.printError('Could not write crash report to disk: $e'); + globals.printError(buffer.toString()); } } @@ -225,7 +226,7 @@ Future _createLocalCrashReport(List args, dynamic error, StackTrac Future _doctorText() async { try { final BufferLogger logger = BufferLogger( - terminal: terminal, + terminal: globals.terminal, outputPreferences: outputPreferences, ); @@ -251,7 +252,7 @@ Future _exit(int code) async { if (flutterUsage.enabled) { final Stopwatch stopwatch = Stopwatch()..start(); await flutterUsage.ensureAnalyticsSent(); - printTrace('ensureAnalyticsSent: ${stopwatch.elapsedMilliseconds}ms'); + globals.printTrace('ensureAnalyticsSent: ${stopwatch.elapsedMilliseconds}ms'); } // Run shutdown hooks before flushing logs @@ -262,7 +263,7 @@ Future _exit(int code) async { // Give the task / timer queue one cycle through before we hard exit. Timer.run(() { try { - printTrace('exiting with code $code'); + globals.printTrace('exiting with code $code'); exit(code); completer.complete(); } catch (error, stackTrace) { diff --git a/packages/flutter_tools/lib/src/base/bot_detector.dart b/packages/flutter_tools/lib/src/base/bot_detector.dart new file mode 100644 index 00000000000..9660d678724 --- /dev/null +++ b/packages/flutter_tools/lib/src/base/bot_detector.dart @@ -0,0 +1,60 @@ +// 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:meta/meta.dart'; +import 'package:platform/platform.dart'; + +import 'context.dart'; + +class BotDetector { + BotDetector({ + @required Platform platform, + }) : _platform = platform; + + final Platform _platform; + + bool get isRunningOnBot { + if ( + // Explicitly stated to not be a bot. + _platform.environment['BOT'] == 'false' + + // Set by the IDEs to the IDE name, so a strong signal that this is not a bot. + || _platform.environment.containsKey('FLUTTER_HOST') + // When set, GA logs to a local file (normally for tests) so we don't need to filter. + || _platform.environment.containsKey('FLUTTER_ANALYTICS_LOG_FILE') + ) { + return false; + } + + return _platform.environment['BOT'] == 'true' + + // https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables + || _platform.environment['TRAVIS'] == 'true' + || _platform.environment['CONTINUOUS_INTEGRATION'] == 'true' + || _platform.environment.containsKey('CI') // Travis and AppVeyor + + // https://www.appveyor.com/docs/environment-variables/ + || _platform.environment.containsKey('APPVEYOR') + + // https://cirrus-ci.org/guide/writing-tasks/#environment-variables + || _platform.environment.containsKey('CIRRUS_CI') + + // https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html + || (_platform.environment.containsKey('AWS_REGION') && + _platform.environment.containsKey('CODEBUILD_INITIATOR')) + + // https://wiki.jenkins.io/display/JENKINS/Building+a+software+project#Buildingasoftwareproject-belowJenkinsSetEnvironmentVariables + || _platform.environment.containsKey('JENKINS_URL') + + // Properties on Flutter's Chrome Infra bots. + || _platform.environment['CHROME_HEADLESS'] == '1' + || _platform.environment.containsKey('BUILDBOT_BUILDERNAME') + || _platform.environment.containsKey('SWARMING_TASK_ID'); + } +} + +bool isRunningOnBot(Platform platform) { + final BotDetector botDetector = context.get() ?? BotDetector(platform: platform); + return botDetector.isRunningOnBot; +} diff --git a/packages/flutter_tools/lib/src/base/utils.dart b/packages/flutter_tools/lib/src/base/utils.dart index 7bb93c8ba77..8a241210c5a 100644 --- a/packages/flutter_tools/lib/src/base/utils.dart +++ b/packages/flutter_tools/lib/src/base/utils.dart @@ -10,61 +10,10 @@ import 'package:meta/meta.dart'; import '../convert.dart'; import '../globals.dart' as globals; -import 'context.dart'; import 'file_system.dart'; import 'io.dart' as io; import 'terminal.dart'; -const BotDetector _kBotDetector = BotDetector(); - -class BotDetector { - const BotDetector(); - - bool get isRunningOnBot { - if ( - // Explicitly stated to not be a bot. - globals.platform.environment['BOT'] == 'false' - - // Set by the IDEs to the IDE name, so a strong signal that this is not a bot. - || globals.platform.environment.containsKey('FLUTTER_HOST') - // When set, GA logs to a local file (normally for tests) so we don't need to filter. - || globals.platform.environment.containsKey('FLUTTER_ANALYTICS_LOG_FILE') - ) { - return false; - } - - return globals.platform.environment['BOT'] == 'true' - - // https://docs.travis-ci.com/user/environment-variables/#Default-Environment-Variables - || globals.platform.environment['TRAVIS'] == 'true' - || globals.platform.environment['CONTINUOUS_INTEGRATION'] == 'true' - || globals.platform.environment.containsKey('CI') // Travis and AppVeyor - - // https://www.appveyor.com/docs/environment-variables/ - || globals.platform.environment.containsKey('APPVEYOR') - - // https://cirrus-ci.org/guide/writing-tasks/#environment-variables - || globals.platform.environment.containsKey('CIRRUS_CI') - - // https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html - || (globals.platform.environment.containsKey('AWS_REGION') && - globals.platform.environment.containsKey('CODEBUILD_INITIATOR')) - - // https://wiki.jenkins.io/display/JENKINS/Building+a+software+project#Buildingasoftwareproject-belowJenkinsSetEnvironmentVariables - || globals.platform.environment.containsKey('JENKINS_URL') - - // Properties on Flutter's Chrome Infra bots. - || globals.platform.environment['CHROME_HEADLESS'] == '1' - || globals.platform.environment.containsKey('BUILDBOT_BUILDERNAME') - || globals.platform.environment.containsKey('SWARMING_TASK_ID'); - } -} - -bool get isRunningOnBot { - final BotDetector botDetector = context.get() ?? _kBotDetector; - return botDetector.isRunningOnBot; -} - /// Convert `foo_bar` to `fooBar`. String camelCase(String str) { int index = str.indexOf('_'); diff --git a/packages/flutter_tools/lib/src/context_runner.dart b/packages/flutter_tools/lib/src/context_runner.dart index a6e7e5b756c..d6e7c6d5029 100644 --- a/packages/flutter_tools/lib/src/context_runner.dart +++ b/packages/flutter_tools/lib/src/context_runner.dart @@ -22,7 +22,6 @@ import 'base/signals.dart'; import 'base/terminal.dart'; import 'base/time.dart'; import 'base/user_messages.dart'; -import 'base/utils.dart'; import 'build_system/build_system.dart'; import 'cache.dart'; import 'compile.dart'; @@ -75,7 +74,6 @@ Future runInContext( ApplicationPackageFactory: () => ApplicationPackageFactory(), Artifacts: () => CachedArtifacts(), AssetBundleFactory: () => AssetBundleFactory.defaultInstance, - BotDetector: () => const BotDetector(), BuildSystem: () => const BuildSystem(), Cache: () => Cache(), ChromeLauncher: () => const ChromeLauncher(), diff --git a/packages/flutter_tools/lib/src/dart/pub.dart b/packages/flutter_tools/lib/src/dart/pub.dart index a2a5c4e6361..699b6da639c 100644 --- a/packages/flutter_tools/lib/src/dart/pub.dart +++ b/packages/flutter_tools/lib/src/dart/pub.dart @@ -6,13 +6,13 @@ import 'dart:async'; import 'package:meta/meta.dart'; +import '../base/bot_detector.dart'; import '../base/common.dart'; import '../base/context.dart'; import '../base/file_system.dart'; import '../base/io.dart' as io; import '../base/logger.dart'; import '../base/process.dart'; -import '../base/utils.dart'; import '../cache.dart'; import '../globals.dart' as globals; import '../reporting/reporting.dart'; @@ -232,7 +232,7 @@ class _DefaultPub implements Pub { @required bool retry, bool showTraceForErrors, }) async { - showTraceForErrors ??= isRunningOnBot; + showTraceForErrors ??= isRunningOnBot(globals.platform); String lastPubMessage = 'no message'; bool versionSolvingFailed = false; @@ -368,7 +368,7 @@ String _getPubEnvironmentValue(PubContext pubContext) { final String existing = globals.platform.environment[_pubEnvironmentKey]; final List values = [ if (existing != null && existing.isNotEmpty) existing, - if (isRunningOnBot) 'flutter_bot', + if (isRunningOnBot(globals.platform)) 'flutter_bot', 'flutter_cli', ...pubContext._values, ]; diff --git a/packages/flutter_tools/lib/src/reporting/reporting.dart b/packages/flutter_tools/lib/src/reporting/reporting.dart index c8113033503..9e0ba484945 100644 --- a/packages/flutter_tools/lib/src/reporting/reporting.dart +++ b/packages/flutter_tools/lib/src/reporting/reporting.dart @@ -10,12 +10,12 @@ import 'package:http/http.dart' as http; import 'package:meta/meta.dart'; import 'package:usage/usage_io.dart'; +import '../base/bot_detector.dart'; import '../base/context.dart'; import '../base/file_system.dart'; import '../base/io.dart'; import '../base/os.dart'; import '../base/time.dart'; -import '../base/utils.dart'; import '../doctor.dart'; import '../features.dart'; import '../globals.dart' as globals; diff --git a/packages/flutter_tools/lib/src/reporting/usage.dart b/packages/flutter_tools/lib/src/reporting/usage.dart index 656a59b4c81..85df6123bc2 100644 --- a/packages/flutter_tools/lib/src/reporting/usage.dart +++ b/packages/flutter_tools/lib/src/reporting/usage.dart @@ -176,7 +176,7 @@ class _DefaultUsage implements Usage { // Many CI systems don't do a full git checkout. version.endsWith('/unknown') || // Ignore bots. - isRunningOnBot || + isRunningOnBot(globals.platform) || // Ignore when suppressed by FLUTTER_SUPPRESS_ANALYTICS. suppressEnvFlag )) { diff --git a/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart b/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart index b48c9d57e3f..578acf04d65 100644 --- a/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart +++ b/packages/flutter_tools/test/commands.shard/permeable/packages_test.dart @@ -5,9 +5,9 @@ import 'dart:async'; import 'package:args/command_runner.dart'; +import 'package:flutter_tools/src/base/bot_detector.dart'; import 'package:flutter_tools/src/base/file_system.dart' hide IOSink; import 'package:flutter_tools/src/base/io.dart'; -import 'package:flutter_tools/src/base/utils.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/commands/packages.dart'; import 'package:flutter_tools/src/dart/pub.dart'; diff --git a/packages/flutter_tools/test/general.shard/base/utils_test.dart b/packages/flutter_tools/test/general.shard/base/bot_detector_test.dart similarity index 57% rename from packages/flutter_tools/test/general.shard/base/utils_test.dart rename to packages/flutter_tools/test/general.shard/base/bot_detector_test.dart index c4e5f1d9a02..023df27855f 100644 --- a/packages/flutter_tools/test/general.shard/base/utils_test.dart +++ b/packages/flutter_tools/test/general.shard/base/bot_detector_test.dart @@ -2,13 +2,10 @@ // 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/base/io.dart'; - -import 'package:flutter_tools/src/base/utils.dart'; +import 'package:flutter_tools/src/base/bot_detector.dart'; import 'package:platform/platform.dart'; import '../../src/common.dart'; -import '../../src/context.dart'; import '../../src/mocks.dart'; void main() { @@ -20,45 +17,33 @@ void main() { setUp(() { fakePlatform = FakePlatform()..environment = {}; mockStdio = MockStdio(); - botDetector = const BotDetector(); + botDetector = BotDetector(platform: fakePlatform); }); group('isRunningOnBot', () { - testUsingContext('returns false unconditionally if BOT=false is set', () async { + testWithoutContext('returns false unconditionally if BOT=false is set', () async { fakePlatform.environment['BOT'] = 'false'; fakePlatform.environment['TRAVIS'] = 'true'; expect(botDetector.isRunningOnBot, isFalse); - }, overrides: { - Stdio: () => mockStdio, - Platform: () => fakePlatform, }); - testUsingContext('returns false unconditionally if FLUTTER_HOST is set', () async { + testWithoutContext('returns false unconditionally if FLUTTER_HOST is set', () async { fakePlatform.environment['FLUTTER_HOST'] = 'foo'; fakePlatform.environment['TRAVIS'] = 'true'; expect(botDetector.isRunningOnBot, isFalse); - }, overrides: { - Stdio: () => mockStdio, - Platform: () => fakePlatform, }); - testUsingContext('returns false with and without a terminal attached', () async { + testWithoutContext('returns false with and without a terminal attached', () async { mockStdio.stdout.hasTerminal = true; expect(botDetector.isRunningOnBot, isFalse); mockStdio.stdout.hasTerminal = false; expect(botDetector.isRunningOnBot, isFalse); - }, overrides: { - Stdio: () => mockStdio, - Platform: () => fakePlatform, }); - testUsingContext('can test analytics outputs on bots when outputting to a file', () async { + testWithoutContext('can test analytics outputs on bots when outputting to a file', () async { fakePlatform.environment['TRAVIS'] = 'true'; fakePlatform.environment['FLUTTER_ANALYTICS_LOG_FILE'] = '/some/file'; expect(botDetector.isRunningOnBot, isFalse); - }, overrides: { - Stdio: () => mockStdio, - Platform: () => fakePlatform, }); }); }); diff --git a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart index 3e6911c9bb1..ff95577f203 100644 --- a/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart +++ b/packages/flutter_tools/test/general.shard/dart/pub_get_test.dart @@ -7,11 +7,11 @@ import 'dart:collection'; import 'package:file/file.dart'; import 'package:file/memory.dart'; +import 'package:flutter_tools/src/base/bot_detector.dart'; import 'package:flutter_tools/src/base/common.dart'; import 'package:flutter_tools/src/base/context.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; -import 'package:flutter_tools/src/base/utils.dart'; import 'package:flutter_tools/src/cache.dart'; import 'package:flutter_tools/src/dart/pub.dart'; import 'package:flutter_tools/src/reporting/reporting.dart';