Unverified Commit 447e3d3f authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] remove globals from compilers (#59184)

Refactors KernelCompiler and ResidentCompiler to no longer use globals (except as a fallback for g3 migration). Improves the compilation error when running flutter test on a package without a flutter_test dependency.

Updates machine mode to output trace text to stderr
parent e1f4cfb4
<<skip until matching line>>
<<stderr>>
<<skip until matching line>>
Failed to load test harness\. +Are you missing a dependency on flutter_test\?
\ No newline at end of file
Error: cannot run without a dependency on "package:flutter_test". Ensure the following lines are present in your pubspec.yaml:
dev_dependencies:
flutter_test:
sdk: flutter
......@@ -128,7 +128,15 @@ Future<void> main(List<String> args) async {
// The mustache dependency is different in google3
TemplateRenderer: () => const MustacheTemplateRenderer(),
if (daemon)
Logger: () => NotifyingLogger(verbose: verbose)
Logger: () => NotifyingLogger(
verbose: verbose,
parent: VerboseLogger(StdoutLogger(
timeoutConfiguration: timeoutConfiguration,
stdio: globals.stdio,
terminal: globals.terminal,
outputPreferences: globals.outputPreferences,
),
))
else if (verbose && !muteCommandLogging)
Logger: () => VerboseLogger(StdoutLogger(
timeoutConfiguration: timeoutConfiguration,
......
......@@ -159,6 +159,11 @@ class EngineBuildPaths {
// Manages the engine artifacts of Flutter.
abstract class Artifacts {
/// A test-specific implementation of artifacts that returns stable paths for
/// all artifacts.
@visibleForTesting
factory Artifacts.test() = _TestArtifacts;
static LocalEngineArtifacts getLocalEngine(EngineBuildPaths engineBuildPaths) {
return LocalEngineArtifacts(
engineBuildPaths.targetEngine,
......@@ -183,7 +188,7 @@ abstract class Artifacts {
/// Manages the engine artifacts downloaded to the local cache.
class CachedArtifacts extends Artifacts {
class CachedArtifacts implements Artifacts {
CachedArtifacts({
@required FileSystem fileSystem,
@required Platform platform,
......@@ -449,7 +454,7 @@ HostPlatform _currentHostPlatformAsHost(Platform platform) {
}
/// Manages the artifacts of a locally built engine.
class LocalEngineArtifacts extends Artifacts {
class LocalEngineArtifacts implements Artifacts {
LocalEngineArtifacts(
this.engineOutPath,
this._hostEngineOutPath, {
......@@ -648,3 +653,26 @@ class OverrideArtifacts implements Artifacts {
String _dartSdkPath(FileSystem fileSystem) {
return fileSystem.path.join(Cache.flutterRoot, 'bin', 'cache', 'dart-sdk');
}
class _TestArtifacts implements Artifacts {
@override
String getArtifactPath(Artifact artifact, {TargetPlatform platform, BuildMode mode}) {
final StringBuffer buffer = StringBuffer();
buffer.write(artifact);
if (platform != null) {
buffer.write('.$platform');
}
if (mode != null) {
buffer.write('.$mode');
}
return buffer.toString();
}
@override
String getEngineType(TargetPlatform platform, [ BuildMode mode ]) {
return 'test-engine';
}
@override
bool get isLocalEngine => false;
}
......@@ -4,8 +4,12 @@
import 'package:meta/meta.dart';
import 'package:package_config/package_config.dart';
import 'package:process/process.dart';
import 'artifacts.dart';
import 'base/context.dart';
import 'base/file_system.dart';
import 'base/logger.dart';
import 'build_info.dart';
import 'compile.dart';
import 'globals.dart' as globals;
......@@ -63,10 +67,19 @@ abstract class CodegenDaemon {
/// supported here. Using the build pipeline implies a fixed multi-root
/// filesystem and requires a pubspec.
class CodeGeneratingKernelCompiler implements KernelCompiler {
const CodeGeneratingKernelCompiler();
static const KernelCompiler _delegate = KernelCompiler();
CodeGeneratingKernelCompiler({
@required FileSystem fileSystem,
@required Artifacts artifacts,
@required ProcessManager processManager,
@required Logger logger,
}) : _delegate = KernelCompiler(
logger: logger,
artifacts: artifacts,
processManager: processManager,
fileSystem: fileSystem,
);
final KernelCompiler _delegate;
@override
Future<CompilerOutput> compile({
String mainPath,
......
......@@ -209,7 +209,7 @@ class AttachCommand extends FlutterCommand {
stdoutCommandResponse,
notifyingLogger: (globals.logger is NotifyingLogger)
? globals.logger as NotifyingLogger
: NotifyingLogger(verbose: globals.logger.isVerbose),
: NotifyingLogger(verbose: globals.logger.isVerbose, parent: globals.logger),
logToStdout: true,
)
: null;
......
......@@ -918,13 +918,14 @@ dynamic _toJsonable(dynamic obj) {
}
class NotifyingLogger extends Logger {
NotifyingLogger({ @required this.verbose }) {
NotifyingLogger({ @required this.verbose, @required this.parent }) {
_messageController = StreamController<LogMessage>.broadcast(
onListen: _onListen,
);
}
final bool verbose;
final Logger parent;
final List<LogMessage> messageBuffer = <LogMessage>[];
StreamController<LogMessage> _messageController;
......@@ -968,7 +969,7 @@ class NotifyingLogger extends Logger {
if (!verbose) {
return;
}
_sendMessage(LogMessage('trace', message));
parent.printError(message);
}
@override
......
......@@ -417,7 +417,7 @@ class RunCommand extends RunCommandBase {
stdoutCommandResponse,
notifyingLogger: (globals.logger is NotifyingLogger)
? globals.logger as NotifyingLogger
: NotifyingLogger(verbose: globals.logger.isVerbose),
: NotifyingLogger(verbose: globals.logger.isVerbose, parent: globals.logger),
logToStdout: true,
);
AppInstance app;
......
This diff is collapsed.
......@@ -148,7 +148,12 @@ Future<T> runInContext<T>(
xcode: globals.xcode,
),
IOSWorkflow: () => const IOSWorkflow(),
KernelCompilerFactory: () => const KernelCompilerFactory(),
KernelCompilerFactory: () => KernelCompilerFactory(
logger: globals.logger,
processManager: globals.processManager,
artifacts: globals.artifacts,
fileSystem: globals.fs,
),
Logger: () => globals.platform.isWindows
? WindowsStdoutLogger(
terminal: globals.terminal,
......
......@@ -18,7 +18,6 @@ import 'base/file_system.dart';
import 'base/io.dart' as io;
import 'base/logger.dart';
import 'base/signals.dart';
import 'base/terminal.dart';
import 'base/utils.dart';
import 'build_info.dart';
import 'build_system/build_system.dart';
......@@ -63,6 +62,9 @@ class FlutterDevice {
dartDefines: buildInfo.dartDefines,
packagesPath: globalPackagesPath,
extraFrontEndOptions: buildInfo.extraFrontEndOptions,
artifacts: globals.artifacts,
processManager: globals.processManager,
logger: globals.logger,
);
/// Create a [FlutterDevice] with optional code generation enabled.
......@@ -99,9 +101,6 @@ class FlutterDevice {
// Override the filesystem scheme so that the frontend_server can find
// the generated entrypoint code.
fileSystemScheme: 'org-dartlang-app',
compilerMessageConsumer:
(String message, {bool emphasis, TerminalColor color, }) =>
globals.printTrace(message),
initializeFromDill: getDefaultCachedKernelPath(
trackWidgetCreation: buildInfo.trackWidgetCreation,
dartDefines: buildInfo.dartDefines,
......@@ -115,6 +114,9 @@ class FlutterDevice {
librariesSpec: globals.fs.file(globals.artifacts
.getArtifactPath(Artifact.flutterWebLibrariesJson)).uri.toString(),
packagesPath: globalPackagesPath,
artifacts: globals.artifacts,
processManager: globals.processManager,
logger: globals.logger,
);
} else {
generator = ResidentCompiler(
......@@ -135,6 +137,9 @@ class FlutterDevice {
dartDefines: buildInfo.dartDefines,
),
packagesPath: globalPackagesPath,
artifacts: globals.artifacts,
processManager: globals.processManager,
logger: globals.logger,
);
}
......
......@@ -9,7 +9,6 @@ import 'package:package_config/package_config.dart';
import '../artifacts.dart';
import '../base/file_system.dart';
import '../base/terminal.dart';
import '../build_info.dart';
import '../bundle.dart';
import '../codegen.dart';
......@@ -71,11 +70,12 @@ class TestCompiler {
ResidentCompiler compiler;
File outputDill;
// Whether to report compiler messages.
bool _suppressOutput = false;
Future<String> compile(Uri mainDart) {
final Completer<String> completer = Completer<String>();
if (compilerController.isClosed) {
return null;
}
compilerController.add(_CompilationRequest(mainDart, completer));
return completer.future;
}
......@@ -99,9 +99,11 @@ class TestCompiler {
Future<ResidentCompiler> createCompiler() async {
final ResidentCompiler residentCompiler = ResidentCompiler(
globals.artifacts.getArtifactPath(Artifact.flutterPatchedSdkPath),
artifacts: globals.artifacts,
logger: globals.logger,
processManager: globals.processManager,
buildMode: buildMode,
trackWidgetCreation: trackWidgetCreation,
compilerMessageConsumer: _reportCompilerMessage,
initializeFromDill: testFilePath,
unsafePackageSerialization: false,
dartDefines: const <String>[],
......@@ -129,10 +131,28 @@ class TestCompiler {
if (!isEmpty) {
return;
}
_packageConfig ??= await loadPackageConfigWithLogging(
globals.fs.file(globalPackagesPath),
logger: globals.logger,
);
if (_packageConfig == null) {
_packageConfig ??= await loadPackageConfigWithLogging(
globals.fs.file(globalPackagesPath),
logger: globals.logger,
);
// Compilation will fail if there is no flutter_test dependency, since
// this library is imported by the generated entrypoint script.
if (_packageConfig['flutter_test'] == null) {
globals.printError(
'\n'
'Error: cannot run without a dependency on "package:flutter_test". '
'Ensure the following lines are present in your pubspec.yaml:'
'\n\n'
'dev_dependencies:\n'
' flutter_test:\n'
' sdk: flutter\n',
);
request.result.complete(null);
await compilerController.close();
return;
}
}
while (compilationQueue.isNotEmpty) {
final _CompilationRequest request = compilationQueue.first;
globals.printTrace('Compiling ${request.mainUri}');
......@@ -142,7 +162,6 @@ class TestCompiler {
compiler = await createCompiler();
firstCompile = true;
}
_suppressOutput = false;
final CompilerOutput compilerOutput = await compiler.recompile(
request.mainUri,
<Uri>[request.mainUri],
......@@ -179,20 +198,4 @@ class TestCompiler {
compilationQueue.removeAt(0);
}
}
void _reportCompilerMessage(String message, {bool emphasis, TerminalColor color}) {
if (_suppressOutput) {
return;
}
if (message.startsWith("Error: Could not resolve the package 'flutter_test'")) {
globals.printTrace(message);
globals.printError('\n\nFailed to load test harness. Are you missing a dependency on flutter_test?\n',
emphasis: emphasis,
color: color,
);
_suppressOutput = true;
return;
}
globals.printError(message);
}
}
......@@ -21,10 +21,12 @@ import '../../src/mocks.dart';
void main() {
Daemon daemon;
NotifyingLogger notifyingLogger;
BufferLogger bufferLogger;
group('daemon', () {
setUp(() {
notifyingLogger = NotifyingLogger(verbose: false);
bufferLogger = BufferLogger.test();
notifyingLogger = NotifyingLogger(verbose: false, parent: bufferLogger);
});
tearDown(() {
......@@ -299,19 +301,15 @@ void main() {
});
testUsingContext('notifyingLogger outputs trace messages in verbose mode', () async {
final NotifyingLogger logger = NotifyingLogger(verbose: true);
final NotifyingLogger logger = NotifyingLogger(verbose: true, parent: bufferLogger);
final Future<LogMessage> messageResult = logger.onMessage.first;
logger.printTrace('test');
final LogMessage message = await messageResult;
expect(message.level, 'trace');
expect(message.message, 'test');
expect(bufferLogger.errorText, contains('test'));
});
testUsingContext('notifyingLogger ignores trace messages in non-verbose mode', () async {
final NotifyingLogger logger = NotifyingLogger(verbose: false);
final NotifyingLogger logger = NotifyingLogger(verbose: false, parent: bufferLogger);
final Future<LogMessage> messageResult = logger.onMessage.first;
logger.printTrace('test');
......@@ -321,10 +319,11 @@ void main() {
expect(message.level, 'status');
expect(message.message, 'hello');
expect(bufferLogger.errorText, contains('test'));
});
testUsingContext('notifyingLogger buffers messages sent before a subscription', () async {
final NotifyingLogger logger = NotifyingLogger(verbose: false);
final NotifyingLogger logger = NotifyingLogger(verbose: false, parent: bufferLogger);
logger.printStatus('hello');
......
......@@ -4,14 +4,13 @@
import 'dart:async';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart';
import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:mockito/mockito.dart';
import 'package:package_config/package_config.dart';
import 'package:process/process.dart';
......@@ -27,13 +26,21 @@ void main() {
MockStdIn mockFrontendServerStdIn;
MockStream mockFrontendServerStdErr;
StreamController<String> stdErrStreamController;
BufferLogger testLogger;
setUp(() {
generator = ResidentCompiler('sdkroot', buildMode: BuildMode.debug);
testLogger = BufferLogger.test();
mockProcessManager = MockProcessManager();
mockFrontendServer = MockProcess();
mockFrontendServerStdIn = MockStdIn();
mockFrontendServerStdErr = MockStream();
generator = ResidentCompiler(
'sdkroot',
buildMode: BuildMode.debug,
artifacts: Artifacts.test(),
processManager: mockProcessManager,
logger: testLogger,
);
when(mockFrontendServer.stdin).thenReturn(mockFrontendServerStdIn);
when(mockFrontendServer.stderr)
......@@ -52,13 +59,14 @@ void main() {
);
});
testUsingContext('compile expression fails if not previously compiled', () async {
testWithoutContext('compile expression fails if not previously compiled', () async {
final CompilerOutput result = await generator.compileExpression(
'2+2', null, null, null, null, false);
expect(result, isNull);
});
testUsingContext('compile expression can compile single expression', () async {
testWithoutContext('compile expression can compile single expression', () async {
final Completer<List<int>> compileResponseCompleter =
Completer<List<int>>();
final Completer<List<int>> compileExpressionResponseCompleter =
......@@ -76,7 +84,7 @@ void main() {
)));
await generator.recompile(
globals.fs.file('/path/to/main.dart').uri,
Uri.file('/path/to/main.dart'),
null, /* invalidatedFiles */
outputPath: '/build/',
packageConfig: PackageConfig.empty,
......@@ -101,14 +109,9 @@ void main() {
}
);
});
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
testUsingContext('compile expressions without awaiting', () async {
testWithoutContext('compile expressions without awaiting', () async {
final Completer<List<int>> compileResponseCompleter = Completer<List<int>>();
final Completer<List<int>> compileExpressionResponseCompleter1 = Completer<List<int>>();
final Completer<List<int>> compileExpressionResponseCompleter2 = Completer<List<int>>();
......@@ -174,10 +177,6 @@ void main() {
)));
expect(await lastExpressionCompleted.future, isTrue);
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
}
......
......@@ -4,10 +4,10 @@
import 'dart:async';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/async_guard.dart';
import 'package:flutter_tools/src/base/io.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/compile.dart';
import 'package:flutter_tools/src/convert.dart';
......@@ -26,13 +26,21 @@ void main() {
MockStdIn mockFrontendServerStdIn;
MockStream mockFrontendServerStdErr;
StreamController<String> stdErrStreamController;
BufferLogger testLogger;
setUp(() {
generator = ResidentCompiler('sdkroot', buildMode: BuildMode.debug);
testLogger = BufferLogger.test();
mockProcessManager = MockProcessManager();
mockFrontendServer = MockProcess();
mockFrontendServerStdIn = MockStdIn();
mockFrontendServerStdErr = MockStream();
generator = ResidentCompiler(
'sdkroot',
buildMode: BuildMode.debug,
logger: testLogger,
processManager: mockProcessManager,
artifacts: Artifacts.test(),
);
when(mockFrontendServer.stdin).thenReturn(mockFrontendServerStdIn);
when(mockFrontendServer.stderr)
......@@ -50,7 +58,7 @@ void main() {
);
});
testUsingContext('incremental compile single dart compile', () async {
testWithoutContext('incremental compile single dart compile', () async {
when(mockFrontendServer.stdout)
.thenAnswer((Invocation invocation) => Stream<List<int>>.fromFuture(
Future<List<int>>.value(utf8.encode(
......@@ -68,13 +76,9 @@ void main() {
verifyNoMoreInteractions(mockFrontendServerStdIn);
expect(testLogger.errorText, equals('line1\nline2\n'));
expect(output.outputFilename, equals('/path/to/main.dart.dill'));
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
testUsingContext('incremental compile single dart compile abnormally terminates', () async {
testWithoutContext('incremental compile single dart compile abnormally terminates', () async {
when(mockFrontendServer.stdout)
.thenAnswer((Invocation invocation) => const Stream<List<int>>.empty()
);
......@@ -85,13 +89,9 @@ void main() {
outputPath: '/build/',
packageConfig: PackageConfig.empty,
)), throwsToolExit());
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
testUsingContext('incremental compile single dart compile abnormally terminates via exitCode', () async {
testWithoutContext('incremental compile single dart compile abnormally terminates via exitCode', () async {
when(mockFrontendServer.exitCode)
.thenAnswer((Invocation invocation) async => 1);
when(mockFrontendServer.stdout)
......@@ -104,13 +104,9 @@ void main() {
outputPath: '/build/',
packageConfig: PackageConfig.empty,
)), throwsToolExit());
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
testUsingContext('incremental compile and recompile', () async {
testWithoutContext('incremental compile and recompile', () async {
final StreamController<List<int>> streamController = StreamController<List<int>>();
when(mockFrontendServer.stdout)
.thenAnswer((Invocation invocation) => streamController.stream);
......@@ -145,13 +141,9 @@ void main() {
'line1\nline2\n'
'line1\nline2\n'
));
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
testUsingContext('incremental compile can suppress errors', () async {
testWithoutContext('incremental compile can suppress errors', () async {
final StreamController<List<int>> stdoutController = StreamController<List<int>>();
when(mockFrontendServer.stdout)
.thenAnswer((Invocation invocation) => stdoutController.stream);
......@@ -179,17 +171,14 @@ void main() {
// Compiler message is not printed with suppressErrors: true above.
expect(testLogger.errorText, isNot(equals(
'line0\nline1\n'
'line1\nline2\n'
'line1\nline2\n'
)));
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
expect(testLogger.traceText, contains(
'line1\nline2\n'
));
});
testUsingContext('incremental compile and recompile twice', () async {
testWithoutContext('incremental compile and recompile twice', () async {
final StreamController<List<int>> streamController = StreamController<List<int>>();
when(mockFrontendServer.stdout)
.thenAnswer((Invocation invocation) => streamController.stream);
......@@ -216,10 +205,6 @@ void main() {
'line1\nline2\n'
'line2\nline3\n'
));
}, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager,
OutputPreferences: () => OutputPreferences(showColor: false),
Platform: kNoColorTerminalPlatform,
});
}
......
......@@ -2,14 +2,14 @@
// 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/logger.dart';
import 'package:flutter_tools/src/compile.dart';
import '../src/common.dart';
import '../src/context.dart';
void main() {
testUsingContext('StdOutHandler test', () async {
final StdoutHandler stdoutHandler = StdoutHandler();
testWithoutContext('StdoutHandler can produce output message', () async {
final StdoutHandler stdoutHandler = StdoutHandler(logger: BufferLogger.test());
stdoutHandler.handler('result 12345');
expect(stdoutHandler.boundaryKey, '12345');
stdoutHandler.handler('12345');
......@@ -19,7 +19,7 @@ void main() {
expect(output.outputFilename, 'message');
});
test('TargetModel values', () {
testWithoutContext('TargetModel values', () {
expect(TargetModel('vm'), TargetModel.vm);
expect(TargetModel.vm.toString(), 'vm');
......
......@@ -12,6 +12,7 @@ import 'package:flutter_tools/src/test/test_compiler.dart';
import 'package:mockito/mockito.dart';
import '../src/common.dart';
import '../src/context.dart';
import '../src/testbed.dart';
final Platform linuxPlatform = FakePlatform(
......@@ -32,7 +33,7 @@ void main() {
},
setup: () async {
globals.fs.file('pubspec.yaml').createSync();
globals.fs.file('.packages').createSync();
globals.fs.file('.packages').writeAsStringSync('flutter_test:flutter_test/');
globals.fs.file('test/foo.dart').createSync(recursive: true);
residentCompiler = MockResidentCompiler();
testCompiler = FakeTestCompiler(
......@@ -86,6 +87,19 @@ void main() {
expect(testCompiler.compilerController.isClosed, true);
verify(residentCompiler.shutdown()).called(1);
}));
test('Reports an error when there is no dependency on flutter_test', () => testbed.run(() async {
globals.fs.file('.packages').writeAsStringSync('\n');
expect(await testCompiler.compile(Uri.parse('test/foo.dart')), null);
expect(testLogger.errorText, contains('Error: cannot run without a dependency on "package:flutter_test"'));
verifyNever(residentCompiler.recompile(
any,
<Uri>[Uri.parse('test/foo.dart')],
outputPath: testCompiler.outputDill.path,
packageConfig: anyNamed('packageConfig'),
));
}));
});
}
......
......@@ -244,11 +244,11 @@ class FakeProcess implements Process {
FakeProcess({
this.pid = 1,
Future<int> exitCode,
Stream<List<int>> stdin,
IOSink stdin,
this.stdout = const Stream<List<int>>.empty(),
this.stderr = const Stream<List<int>>.empty(),
}) : exitCode = exitCode ?? Future<int>.value(0),
stdin = stdin as IOSink ?? MemoryIOSink();
stdin = stdin ?? MemoryIOSink();
@override
final int pid;
......
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