Unverified Commit 4c99da6c authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Avoid printing blank lines between "Another exception was thrown:" messages. (#119587)

parent 8f5949ed
...@@ -169,8 +169,9 @@ Future<void> run(List<String> arguments) async { ...@@ -169,8 +169,9 @@ Future<void> run(List<String> arguments) async {
// Try with the --watch analyzer, to make sure it returns success also. // Try with the --watch analyzer, to make sure it returns success also.
// The --benchmark argument exits after one run. // The --benchmark argument exits after one run.
// We specify a failureMessage so that the actual output is muted in the case where _runFlutterAnalyze above already failed.
printProgress('Dart analysis (with --watch)...'); printProgress('Dart analysis (with --watch)...');
await _runFlutterAnalyze(flutterRoot, options: <String>[ await _runFlutterAnalyze(flutterRoot, failureMessage: 'Dart analyzer failed when --watch was used.', options: <String>[
'--flutter-repo', '--flutter-repo',
'--watch', '--watch',
'--benchmark', '--benchmark',
...@@ -196,7 +197,7 @@ Future<void> run(List<String> arguments) async { ...@@ -196,7 +197,7 @@ Future<void> run(List<String> arguments) async {
], ],
workingDirectory: flutterRoot, workingDirectory: flutterRoot,
); );
await _runFlutterAnalyze(outDir.path, options: <String>[ await _runFlutterAnalyze(outDir.path, failureMessage: 'Dart analyzer failed on mega_gallery benchmark.', options: <String>[
'--watch', '--watch',
'--benchmark', '--benchmark',
...arguments, ...arguments,
...@@ -1942,11 +1943,13 @@ Diff at character #$indexOfDifference ...@@ -1942,11 +1943,13 @@ Diff at character #$indexOfDifference
Future<CommandResult> _runFlutterAnalyze(String workingDirectory, { Future<CommandResult> _runFlutterAnalyze(String workingDirectory, {
List<String> options = const <String>[], List<String> options = const <String>[],
String? failureMessage,
}) async { }) async {
return runCommand( return runCommand(
flutter, flutter,
<String>['analyze', ...options], <String>['analyze', ...options],
workingDirectory: workingDirectory, workingDirectory: workingDirectory,
failureMessage: failureMessage,
); );
} }
......
...@@ -190,13 +190,24 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, { ...@@ -190,13 +190,24 @@ Future<CommandResult> runCommand(String executable, List<String> arguments, {
print(result.flattenedStderr); print(result.flattenedStderr);
break; break;
} }
String allOutput;
if (failureMessage == null) {
allOutput = '${result.flattenedStdout}\n${result.flattenedStderr}';
if (allOutput.split('\n').length > 10) {
allOutput = '(stdout/stderr output was more than 10 lines)';
}
} else {
allOutput = '';
}
foundError(<String>[ foundError(<String>[
if (failureMessage != null) if (failureMessage != null)
failureMessage failureMessage,
else
'$bold${red}Command exited with exit code ${result.exitCode} but expected: ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset',
'${bold}Command: $green$commandDescription$reset', '${bold}Command: $green$commandDescription$reset',
'${bold}Relative working directory: $cyan$relativeWorkingDir$reset', if (failureMessage == null)
'$bold${red}Command exited with exit code ${result.exitCode} but expected ${expectNonZeroExit ? (expectedExitCode ?? 'non-zero') : 'zero'} exit code.$reset',
'${bold}Working directory: $cyan${path.absolute(relativeWorkingDir)}$reset',
if (allOutput.isNotEmpty)
'${bold}stdout and stderr output:\n$allOutput',
]); ]);
} else { } else {
print('ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset'); print('ELAPSED TIME: ${prettyPrintDuration(result.elapsedTime)} for $green$commandDescription$reset in $cyan$relativeWorkingDir$reset');
......
...@@ -82,8 +82,7 @@ VoidCallback? onError; ...@@ -82,8 +82,7 @@ VoidCallback? onError;
bool get hasError => _hasError; bool get hasError => _hasError;
bool _hasError = false; bool _hasError = false;
Iterable<String> get errorMessages => _errorMessages; List<List<String>> _errorMessages = <List<String>>[];
List<String> _errorMessages = <String>[];
final List<String> _pendingLogs = <String>[]; final List<String> _pendingLogs = <String>[];
Timer? _hideTimer; // When this is null, the output is verbose. Timer? _hideTimer; // When this is null, the output is verbose.
...@@ -104,7 +103,7 @@ void foundError(List<String> messages) { ...@@ -104,7 +103,7 @@ void foundError(List<String> messages) {
// another error. // another error.
_pendingLogs.forEach(_printLoudly); _pendingLogs.forEach(_printLoudly);
_pendingLogs.clear(); _pendingLogs.clear();
_errorMessages.addAll(messages); _errorMessages.add(messages);
_hasError = true; _hasError = true;
if (onError != null) { if (onError != null) {
onError!(); onError!();
...@@ -124,8 +123,18 @@ Never reportErrorsAndExit() { ...@@ -124,8 +123,18 @@ Never reportErrorsAndExit() {
_hideTimer?.cancel(); _hideTimer?.cancel();
_hideTimer = null; _hideTimer = null;
print(redLine); print(redLine);
print('For your convenience, the error messages reported above are repeated here:'); print('${red}For your convenience, the error messages reported above are repeated here:$reset');
_errorMessages.forEach(print); final bool printSeparators = _errorMessages.any((List<String> messages) => messages.length > 1);
if (printSeparators) {
print(' 🙙 🙛 ');
}
for (int index = 0; index < _errorMessages.length * 2 - 1; index += 1) {
if (index.isEven) {
_errorMessages[index ~/ 2].forEach(print);
} else if (printSeparators) {
print(' 🙙 🙛 ');
}
}
print(redLine); print(redLine);
system.exit(1); system.exit(1);
} }
......
...@@ -1329,9 +1329,25 @@ abstract class ResidentRunner extends ResidentHandlers { ...@@ -1329,9 +1329,25 @@ abstract class ResidentRunner extends ResidentHandlers {
void printStructuredErrorLog(vm_service.Event event) { void printStructuredErrorLog(vm_service.Event event) {
if (event.extensionKind == 'Flutter.Error' && !machine) { if (event.extensionKind == 'Flutter.Error' && !machine) {
final Map<dynamic, dynamic>? json = event.extensionData?.data; final Map<String, Object?>? json = event.extensionData?.data;
if (json != null && json.containsKey('renderedErrorText')) { if (json != null && json.containsKey('renderedErrorText')) {
globals.printStatus('\n${json['renderedErrorText']}'); final int errorsSinceReload;
if (json.containsKey('errorsSinceReload') && json['errorsSinceReload'] is int) {
errorsSinceReload = json['errorsSinceReload']! as int;
} else {
errorsSinceReload = 0;
}
if (errorsSinceReload == 0) {
// We print a blank line around the first error, to more clearly emphasize it
// in the output. (Other errors don't get this.)
globals.printStatus('');
}
globals.printStatus('${json['renderedErrorText']}');
if (errorsSinceReload == 0) {
globals.printStatus('');
}
} else {
globals.printError('Received an invalid ${globals.logger.terminal.bolden("Flutter.Error")} message from app: $json');
} }
} }
} }
......
...@@ -285,11 +285,11 @@ Future<vm_service.VmService> setUpVmService( ...@@ -285,11 +285,11 @@ Future<vm_service.VmService> setUpVmService(
} }
if (printStructuredErrorLogMethod != null) { if (printStructuredErrorLogMethod != null) {
vmService.onExtensionEvent.listen(printStructuredErrorLogMethod); vmService.onExtensionEvent.listen(printStructuredErrorLogMethod);
// It is safe to ignore this error because we expect an error to be
// thrown if we're already subscribed.
registrationRequests.add(vmService registrationRequests.add(vmService
.streamListen(vm_service.EventStreams.kExtension) .streamListen(vm_service.EventStreams.kExtension)
.then<vm_service.Success?>((vm_service.Success success) => success) .then<vm_service.Success?>((vm_service.Success success) => success)
// It is safe to ignore this error because we expect an error to be
// thrown if we're already subscribed.
.catchError((Object? error) => null, test: (Object? error) => error is vm_service.RPCError) .catchError((Object? error) => null, test: (Object? error) => error is vm_service.RPCError)
); );
} }
......
...@@ -429,49 +429,105 @@ void main() { ...@@ -429,49 +429,105 @@ void main() {
() async { () async {
final ResidentRunner residentWebRunner = final ResidentRunner residentWebRunner =
setUpResidentRunner(flutterDevice, logger: testLogger); setUpResidentRunner(flutterDevice, logger: testLogger);
final Map<String, String> extensionData = <String, String>{ final List<VmServiceExpectation> requests = <VmServiceExpectation>[
'test': 'data',
'renderedErrorText': 'error text',
};
final Map<String, String> emptyExtensionData = <String, String>{
'test': 'data',
'renderedErrorText': '',
};
final Map<String, String> nonStructuredErrorData = <String, String>{
'other': 'other stuff',
};
fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[
...kAttachExpectations, ...kAttachExpectations,
// This is the first error message of a session.
FakeVmServiceStreamResponse( FakeVmServiceStreamResponse(
streamId: 'Extension', streamId: 'Extension',
event: vm_service.Event( event: vm_service.Event(
timestamp: 0, timestamp: 0,
extensionKind: 'Flutter.Error', extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(extensionData), extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'errorsSinceReload': 0,
'renderedErrorText': 'first',
}),
kind: vm_service.EventStreams.kExtension, kind: vm_service.EventStreams.kExtension,
), ),
), ),
// Empty error text should not break anything. // This is the second error message of a session.
FakeVmServiceStreamResponse( FakeVmServiceStreamResponse(
streamId: 'Extension', streamId: 'Extension',
event: vm_service.Event( event: vm_service.Event(
timestamp: 0, timestamp: 0,
extensionKind: 'Flutter.Error', extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(emptyExtensionData), extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'errorsSinceReload': 1,
'renderedErrorText': 'second',
}),
kind: vm_service.EventStreams.kExtension, kind: vm_service.EventStreams.kExtension,
), ),
), ),
// This is not Flutter.Error kind data, so it should not be logged. // This is not Flutter.Error kind data, so it should not be logged, even though it has a renderedErrorText field.
FakeVmServiceStreamResponse( FakeVmServiceStreamResponse(
streamId: 'Extension', streamId: 'Extension',
event: vm_service.Event( event: vm_service.Event(
timestamp: 0, timestamp: 0,
extensionKind: 'Other', extensionKind: 'Other',
extensionData: vm_service.ExtensionData.parse(nonStructuredErrorData), extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'errorsSinceReload': 2,
'renderedErrorText': 'not an error',
}),
kind: vm_service.EventStreams.kExtension, kind: vm_service.EventStreams.kExtension,
), ),
), ),
]); // This is the third error message of a session.
FakeVmServiceStreamResponse(
streamId: 'Extension',
event: vm_service.Event(
timestamp: 0,
extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'errorsSinceReload': 2,
'renderedErrorText': 'third',
}),
kind: vm_service.EventStreams.kExtension,
),
),
// This is bogus error data.
FakeVmServiceStreamResponse(
streamId: 'Extension',
event: vm_service.Event(
timestamp: 0,
extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'other': 'bad stuff',
}),
kind: vm_service.EventStreams.kExtension,
),
),
// Empty error text should not break anything.
FakeVmServiceStreamResponse(
streamId: 'Extension',
event: vm_service.Event(
timestamp: 0,
extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'test': 'data',
'renderedErrorText': '',
}),
kind: vm_service.EventStreams.kExtension,
),
),
// Messages without errorsSinceReload should act as if errorsSinceReload: 0
FakeVmServiceStreamResponse(
streamId: 'Extension',
event: vm_service.Event(
timestamp: 0,
extensionKind: 'Flutter.Error',
extensionData: vm_service.ExtensionData.parse(<String, Object?>{
'test': 'data',
'renderedErrorText': 'error text',
}),
kind: vm_service.EventStreams.kExtension,
),
),
// When adding things here, make sure the last one is supposed to output something
// to the statusLog, otherwise you won't be able to distinguish the absence of output
// due to it passing the test from absence due to it not running the test.
];
// We use requests below, so make a copy of it here (FakeVmServiceHost will
// clear its copy internally, which would affect our pumping below).
fakeVmServiceHost = FakeVmServiceHost(requests: requests.toList());
setupMocks(); setupMocks();
final Completer<DebugConnectionInfo> connectionInfoCompleter = final Completer<DebugConnectionInfo> connectionInfoCompleter =
...@@ -480,10 +536,34 @@ void main() { ...@@ -480,10 +536,34 @@ void main() {
connectionInfoCompleter: connectionInfoCompleter, connectionInfoCompleter: connectionInfoCompleter,
)); ));
await connectionInfoCompleter.future; await connectionInfoCompleter.future;
await null; assert(requests.length > 5, 'requests was modified');
for (final Object _ in requests) {
// pump the task queue once for each message
await null;
}
expect(testLogger.statusText, contains('\nerror text')); expect(testLogger.statusText,
expect(testLogger.statusText, isNot(contains('other stuff'))); 'Launching lib/main.dart on FakeDevice in debug mode...\n'
'Waiting for connection from debug service on FakeDevice...\n'
'Debug service listening on ws://127.0.0.1/abcd/\n'
'\n'
'💪 Running with sound null safety 💪\n'
'\n'
'first\n'
'\n'
'second\n'
'third\n'
'\n'
'\n' // the empty message
'\n'
'\n'
'error text\n'
'\n'
);
expect(testLogger.errorText,
'Received an invalid Flutter.Error message from app: {other: bad stuff}\n'
);
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
FileSystem: () => fileSystem, FileSystem: () => fileSystem,
ProcessManager: () => processManager, ProcessManager: () => processManager,
......
...@@ -548,6 +548,7 @@ void main() { ...@@ -548,6 +548,7 @@ void main() {
' verticalDirection: down', ' verticalDirection: down',
'◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤', '◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤◢◤',
'════════════════════════════════════════════════════════════════════════════════════════════════════', '════════════════════════════════════════════════════════════════════════════════════════════════════',
'',
startsWith('Reloaded 0 libraries in '), startsWith('Reloaded 0 libraries in '),
'', '',
'Application finished.', 'Application finished.',
......
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