Unverified Commit 33b183e6 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Fix extra blank lines in logger output (#81607)

parent 96e4b47f
...@@ -323,7 +323,6 @@ class AndroidGradleBuilder implements AndroidBuilder { ...@@ -323,7 +323,6 @@ class AndroidGradleBuilder implements AndroidBuilder {
final Status status = _logger.startProgress( final Status status = _logger.startProgress(
"Running Gradle task '$assembleTask'...", "Running Gradle task '$assembleTask'...",
multilineOutput: true,
); );
final List<String> command = <String>[ final List<String> command = <String>[
...@@ -636,7 +635,6 @@ class AndroidGradleBuilder implements AndroidBuilder { ...@@ -636,7 +635,6 @@ class AndroidGradleBuilder implements AndroidBuilder {
final String aarTask = getAarTaskFor(buildInfo); final String aarTask = getAarTaskFor(buildInfo);
final Status status = _logger.startProgress( final Status status = _logger.startProgress(
"Running Gradle task '$aarTask'...", "Running Gradle task '$aarTask'...",
multilineOutput: true,
); );
final String flutterRoot = _fileSystem.path.absolute(Cache.flutterRoot); final String flutterRoot = _fileSystem.path.absolute(Cache.flutterRoot);
......
...@@ -21,7 +21,7 @@ class ToolExit implements Exception { ...@@ -21,7 +21,7 @@ class ToolExit implements Exception {
final int? exitCode; final int? exitCode;
@override @override
String toString() => 'Exception: $message'; String toString() => 'Exception: $message'; // TODO(ianh): Really this should say "Error".
} }
/// Indicates to the linter that the given future is intentionally not awaited. /// Indicates to the linter that the given future is intentionally not awaited.
......
...@@ -81,6 +81,10 @@ abstract class Terminal { ...@@ -81,6 +81,10 @@ abstract class Terminal {
/// Whether the current terminal can display emoji. /// Whether the current terminal can display emoji.
bool get supportsEmoji; bool get supportsEmoji;
/// When we have a choice of styles (e.g. animated spinners), this selects the
/// style to use.
int get preferredStyle;
/// Whether we are interacting with the flutter tool via the terminal. /// Whether we are interacting with the flutter tool via the terminal.
/// ///
/// If not set, defaults to false. /// If not set, defaults to false.
...@@ -143,12 +147,15 @@ class AnsiTerminal implements Terminal { ...@@ -143,12 +147,15 @@ class AnsiTerminal implements Terminal {
AnsiTerminal({ AnsiTerminal({
required io.Stdio stdio, required io.Stdio stdio,
required Platform platform, required Platform platform,
DateTime? now, // Time used to determine preferredStyle. Defaults to 0001-01-01 00:00.
}) })
: _stdio = stdio, : _stdio = stdio,
_platform = platform; _platform = platform,
_now = now ?? DateTime(1);
final io.Stdio _stdio; final io.Stdio _stdio;
final Platform _platform; final Platform _platform;
final DateTime _now;
static const String bold = '\u001B[1m'; static const String bold = '\u001B[1m';
static const String resetAll = '\u001B[0m'; static const String resetAll = '\u001B[0m';
...@@ -187,6 +194,15 @@ class AnsiTerminal implements Terminal { ...@@ -187,6 +194,15 @@ class AnsiTerminal implements Terminal {
bool get supportsEmoji => !_platform.isWindows bool get supportsEmoji => !_platform.isWindows
|| _platform.environment.containsKey('WT_SESSION'); || _platform.environment.containsKey('WT_SESSION');
@override
int get preferredStyle {
const int workdays = DateTime.friday;
if (_now.weekday <= workdays) {
return _now.weekday - 1;
}
return _now.hour + workdays;
}
final RegExp _boldControls = RegExp( final RegExp _boldControls = RegExp(
'(${RegExp.escape(resetBold)}|${RegExp.escape(bold)})', '(${RegExp.escape(resetBold)}|${RegExp.escape(bold)})',
); );
...@@ -355,6 +371,9 @@ class _TestTerminal implements Terminal { ...@@ -355,6 +371,9 @@ class _TestTerminal implements Terminal {
@override @override
final bool supportsEmoji; final bool supportsEmoji;
@override
int get preferredStyle => 0;
@override @override
bool get stdinHasTerminal => false; bool get stdinHasTerminal => false;
......
...@@ -163,6 +163,7 @@ AnsiTerminal get terminal { ...@@ -163,6 +163,7 @@ AnsiTerminal get terminal {
final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal( final AnsiTerminal _defaultAnsiTerminal = AnsiTerminal(
stdio: stdio, stdio: stdio,
platform: platform, platform: platform,
now: DateTime.now(),
); );
/// The global Stdio wrapper. /// The global Stdio wrapper.
......
...@@ -173,12 +173,53 @@ void main() { ...@@ -173,12 +173,53 @@ void main() {
..stdinHasTerminal = false; ..stdinHasTerminal = false;
final AnsiTerminal ansiTerminal = AnsiTerminal( final AnsiTerminal ansiTerminal = AnsiTerminal(
stdio: stdio, stdio: stdio,
platform: const LocalPlatform() platform: const LocalPlatform(),
); );
expect(() => ansiTerminal.singleCharMode = true, returnsNormally); expect(() => ansiTerminal.singleCharMode = true, returnsNormally);
}); });
}); });
testWithoutContext('AnsiTerminal.preferredStyle', () {
final Stdio stdio = FakeStdio();
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform()).preferredStyle, 0); // Defaults to 0 for backwards compatibility.
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 1)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 2)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 3)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 4)).preferredStyle, 3);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 5)).preferredStyle, 4);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 6)).preferredStyle, 5);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 7)).preferredStyle, 5);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 8)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 9)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 10)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 11)).preferredStyle, 3);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 1, 1)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 2, 1)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 3, 1)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 4, 1)).preferredStyle, 3);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 5, 1)).preferredStyle, 4);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 6, 1)).preferredStyle, 6);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 7, 1)).preferredStyle, 6);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 8, 1)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 9, 1)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 10, 1)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 11, 1)).preferredStyle, 3);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 1, 23)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 2, 23)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 3, 23)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 4, 23)).preferredStyle, 3);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 5, 23)).preferredStyle, 4);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 6, 23)).preferredStyle, 28);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 7, 23)).preferredStyle, 28);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 8, 23)).preferredStyle, 0);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 9, 23)).preferredStyle, 1);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 10, 23)).preferredStyle, 2);
expect(AnsiTerminal(stdio: stdio, platform: const LocalPlatform(), now: DateTime(2018, 1, 11, 23)).preferredStyle, 3);
});
} }
late Stream<String> mockStdInStream; late Stream<String> mockStdInStream;
...@@ -187,7 +228,8 @@ class TestTerminal extends AnsiTerminal { ...@@ -187,7 +228,8 @@ class TestTerminal extends AnsiTerminal {
TestTerminal({ TestTerminal({
Stdio? stdio, Stdio? stdio,
Platform platform = const LocalPlatform(), Platform platform = const LocalPlatform(),
}) : super(stdio: stdio ?? Stdio(), platform: platform); DateTime? now,
}) : super(stdio: stdio ?? Stdio(), platform: platform, now: now ?? DateTime(2018));
@override @override
Stream<String> get keystrokes { Stream<String> get keystrokes {
...@@ -195,6 +237,9 @@ class TestTerminal extends AnsiTerminal { ...@@ -195,6 +237,9 @@ class TestTerminal extends AnsiTerminal {
} }
bool singleCharMode = false; bool singleCharMode = false;
@override
int get preferredStyle => 0;
} }
class FakeStdio extends Fake implements Stdio { class FakeStdio extends Fake implements Stdio {
......
...@@ -682,4 +682,7 @@ class TestTerminal extends AnsiTerminal { ...@@ -682,4 +682,7 @@ class TestTerminal extends AnsiTerminal {
Stream<String> get keystrokes { Stream<String> get keystrokes {
return mockTerminalStdInStream; return mockTerminalStdInStream;
} }
@override
int get preferredStyle => 0;
} }
...@@ -280,6 +280,8 @@ Future<ProcessTestResult> runFlutter( ...@@ -280,6 +280,8 @@ Future<ProcessTestResult> runFlutter(
return ProcessTestResult(exitCode, stdoutLog, stderrLog); return ProcessTestResult(exitCode, stdoutLog, stderrLog);
} }
const int progressMessageWidth = 64;
void main() { void main() {
testWithoutContext('flutter run writes and clears pidfile appropriately', () async { testWithoutContext('flutter run writes and clears pidfile appropriately', () async {
final String tempDirectory = fileSystem.systemTempDirectory.createTempSync('flutter_overall_experience_test.').resolveSymbolicLinksSync(); final String tempDirectory = fileSystem.systemTempDirectory.createTempSync('flutter_overall_experience_test.').resolveSymbolicLinksSync();
...@@ -326,18 +328,18 @@ void main() { ...@@ -326,18 +328,18 @@ void main() {
<String>['run', '-dflutter-tester', '--report-ready', '--pid-file', pidFile, '--no-devtools', testScript], <String>['run', '-dflutter-tester', '--report-ready', '--pid-file', pidFile, '--no-devtools', testScript],
testDirectory, testDirectory,
<Transition>[ <Transition>[
Barrier('Flutter run key commands.', handler: (String line) { Multiple(<Pattern>['Flutter run key commands.', 'called paint'], handler: (String line) {
pid = int.parse(fileSystem.file(pidFile).readAsStringSync()); pid = int.parse(fileSystem.file(pidFile).readAsStringSync());
processManager.killPid(pid, ProcessSignal.sigusr1); processManager.killPid(pid, ProcessSignal.sigusr1);
return null; return null;
}), }),
Barrier(RegExp(r'^Performing hot reload\.\.\.'), logging: true), // sometimes this includes the "called reassemble" message Barrier('Performing hot reload...'.padRight(progressMessageWidth), logging: true),
Multiple(<Pattern>[RegExp(r'^Reloaded 0 libraries in [0-9]+ms\.$'), /*'called reassemble', (see TODO below)*/ 'called paint'], handler: (String line) { Multiple(<Pattern>[RegExp(r'^Reloaded 0 libraries in [0-9]+ms\.$'), 'called reassemble', 'called paint'], handler: (String line) {
processManager.killPid(pid, ProcessSignal.sigusr2); processManager.killPid(pid, ProcessSignal.sigusr2);
return null; return null;
}), }),
Barrier(RegExp(r'^Performing hot restart\.\.\.')), // sametimes this includes the "called main" message Barrier('Performing hot restart...'.padRight(progressMessageWidth)),
Multiple(<Pattern>[RegExp(r'^Restarted application in [0-9]+ms.$'), /*'called main', (see TODO below)*/ 'called paint'], handler: (String line) { Multiple(<Pattern>[RegExp(r'^Restarted application in [0-9]+ms.$'), 'called main', 'called paint'], handler: (String line) {
return 'q'; return 'q';
}), }),
const Barrier('Application finished.'), const Barrier('Application finished.'),
...@@ -347,25 +349,20 @@ void main() { ...@@ -347,25 +349,20 @@ void main() {
// We check the output from the app (all starts with "called ...") and the output from the tool // We check the output from the app (all starts with "called ...") and the output from the tool
// (everything else) separately, because their relative timing isn't guaranteed. Their rough timing // (everything else) separately, because their relative timing isn't guaranteed. Their rough timing
// is verified by the expected transitions above. // is verified by the expected transitions above.
// TODO(ianh): Fix the tool so that the output isn't garbled (right now we're putting debug output from expect(result.stdout.where((String line) => line.startsWith('called ')), <Object>[
// the app on the line where we're spinning the busy signal, rather than adding a newline).
expect(result.stdout.where((String line) => line.startsWith('called ') &&
line != 'called reassemble' /* see todo above*/ &&
line != 'called main' /* see todo above*/), <Object>[
// logs start after we receive the response to sending SIGUSR1 // logs start after we receive the response to sending SIGUSR1
// SIGUSR1: // SIGUSR1:
// 'called reassemble', // see todo above, this only sometimes gets included, other times it's on the "performing..." line 'called reassemble',
'called paint', 'called paint',
// SIGUSR2: // SIGUSR2:
// 'called main', // see todo above, this is sometimes on the "performing..." line 'called main',
'called paint', 'called paint',
]); ]);
expect(result.stdout.where((String line) => !line.startsWith('called ')), <Object>[ expect(result.stdout.where((String line) => !line.startsWith('called ')), <Object>[
// logs start after we receive the response to sending SIGUSR1 // logs start after we receive the response to sending SIGUSR1
startsWith('Performing hot reload...'), // see todo above, this sometimes ends with "called reassemble" 'Performing hot reload...'.padRight(progressMessageWidth),
'', // this newline is probably the misplaced one for the reassemble; see todo above
startsWith('Reloaded 0 libraries in '), startsWith('Reloaded 0 libraries in '),
'Performing hot restart... ', 'Performing hot restart...'.padRight(progressMessageWidth),
startsWith('Restarted application in '), startsWith('Restarted application in '),
'', // this newline is the one for after we hit "q" '', // this newline is the one for after we hit "q"
'Application finished.', 'Application finished.',
...@@ -386,14 +383,14 @@ void main() { ...@@ -386,14 +383,14 @@ void main() {
<String>['run', '-dflutter-tester', '--report-ready', '--no-devtools', testScript], <String>['run', '-dflutter-tester', '--report-ready', '--no-devtools', testScript],
testDirectory, testDirectory,
<Transition>[ <Transition>[
Multiple(<Pattern>['Flutter run key commands.', 'called main'], handler: (String line) { Multiple(<Pattern>['Flutter run key commands.', 'called main', 'called paint'], handler: (String line) {
return 'r'; return 'r';
}), }),
Barrier(RegExp(r'^Performing hot reload\.\.\.'), logging: true), Barrier('Performing hot reload...'.padRight(progressMessageWidth), logging: true),
Multiple(<Pattern>['ready', /*'reassemble', (see todo below)*/ 'called paint'], handler: (String line) { Multiple(<Pattern>['ready', 'called reassemble', 'called paint'], handler: (String line) {
return 'R'; return 'R';
}), }),
Barrier(RegExp(r'^Performing hot restart\.\.\.')), Barrier('Performing hot restart...'.padRight(progressMessageWidth)),
Multiple(<Pattern>['ready', 'called main', 'called paint'], handler: (String line) { Multiple(<Pattern>['ready', 'called main', 'called paint'], handler: (String line) {
return 'p'; return 'p';
}), }),
...@@ -410,11 +407,10 @@ void main() { ...@@ -410,11 +407,10 @@ void main() {
// We check the output from the app (all starts with "called ...") and the output from the tool // We check the output from the app (all starts with "called ...") and the output from the tool
// (everything else) separately, because their relative timing isn't guaranteed. Their rough timing // (everything else) separately, because their relative timing isn't guaranteed. Their rough timing
// is verified by the expected transitions above. // is verified by the expected transitions above.
// TODO(ianh): Fix the tool so that the output isn't garbled (right now we're putting debug output from expect(result.stdout.where((String line) => line.startsWith('called ')), <Object>[
// the app on the line where we're spinning the busy signal, rather than adding a newline). // logs start after we initiate the hot reload
expect(result.stdout.where((String line) => line.startsWith('called ') && line != 'called reassemble' /* see todo above*/), <Object>[
// hot reload: // hot reload:
// 'called reassemble', // see todo above, this sometimes gets placed on the "Performing hot reload..." line 'called reassemble',
'called paint', 'called paint',
// hot restart: // hot restart:
'called main', 'called main',
...@@ -427,12 +423,11 @@ void main() { ...@@ -427,12 +423,11 @@ void main() {
]); ]);
expect(result.stdout.where((String line) => !line.startsWith('called ')), <Object>[ expect(result.stdout.where((String line) => !line.startsWith('called ')), <Object>[
// logs start after we receive the response to hitting "r" // logs start after we receive the response to hitting "r"
startsWith('Performing hot reload...'), // see todo above, this sometimes ends with "called reassemble" 'Performing hot reload...'.padRight(progressMessageWidth),
'', // this newline is probably the misplaced one for the reassemble; see todo above
startsWith('Reloaded 0 libraries in '), startsWith('Reloaded 0 libraries in '),
'ready', 'ready',
'', // this newline is the one for after we hit "R" '', // this newline is the one for after we hit "R"
'Performing hot restart... ', 'Performing hot restart...'.padRight(progressMessageWidth),
startsWith('Restarted application in '), startsWith('Restarted application in '),
'ready', 'ready',
'', // newline for after we hit "p" the first time '', // newline for after we hit "p" the first time
...@@ -462,7 +457,7 @@ void main() { ...@@ -462,7 +457,7 @@ void main() {
Barrier(RegExp(r'^The Flutter DevTools debugger and profiler on Flutter test device is available at: '), handler: (String line) { Barrier(RegExp(r'^The Flutter DevTools debugger and profiler on Flutter test device is available at: '), handler: (String line) {
return 'r'; return 'r';
}), }),
Barrier(RegExp(r'^Performing hot reload\.\.\.'), logging: true), Barrier('Performing hot reload...'.padRight(progressMessageWidth), logging: true),
Barrier(RegExp(r'^Reloaded 0 libraries in [0-9]+ms.'), handler: (String line) { Barrier(RegExp(r'^Reloaded 0 libraries in [0-9]+ms.'), handler: (String line) {
return 'q'; return 'q';
}), }),
...@@ -472,6 +467,7 @@ void main() { ...@@ -472,6 +467,7 @@ void main() {
expect(result.exitCode, 0); expect(result.exitCode, 0);
expect(result.stdout, <Object>[ expect(result.stdout, <Object>[
startsWith('Performing hot reload...'), startsWith('Performing hot reload...'),
'',
'══╡ EXCEPTION CAUGHT BY RENDERING LIBRARY ╞═════════════════════════════════════════════════════════', '══╡ EXCEPTION CAUGHT BY RENDERING LIBRARY ╞═════════════════════════════════════════════════════════',
'The following assertion was thrown during layout:', 'The following assertion was thrown during layout:',
'A RenderFlex overflowed by 69200 pixels on the right.', 'A RenderFlex overflowed by 69200 pixels on the right.',
...@@ -506,7 +502,6 @@ void main() { ...@@ -506,7 +502,6 @@ 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