Small-scale clean ups in test.dart, mainly around test_progress.

tools/testing/dart/test_progress.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library test_progress;
 
+import "dart:convert" show JSON;
 import "dart:io";
-import "dart:io" as io;
-import "dart:convert" show JSON;
+
 import "path.dart";
 import "status_file_parser.dart";
 import "summary_report.dart";
 import "test_runner.dart";
 import "test_suite.dart";
 import "utils.dart";
 
-String _pad(String s, int length) {
-  StringBuffer buffer = new StringBuffer();
-  for (int i = s.length; i < length; i++) {
-    buffer.write(' ');
-  }
-  buffer.write(s);
-  return buffer.toString();
+/// Controls how message strings are processed before being displayed.
+class Formatter {
+  /// Messages are left as-is.
+  static const normal = const Formatter._();
+
+  /// Messages are wrapped in ANSI escape codes to color them for display on a
+  /// terminal.
+  static const color = const _ColorFormatter();
+
+  const Formatter._();
+
+  /// Formats a success message.
+  String passed(String message) => message;
+
+  /// Formats a failure message.
+  String failed(String message) => message;
 }
 
-String _padTime(int time) {
-  if (time == 0) {
-    return '00';
-  } else if (time < 10) {
-    return '0$time';
-  } else {
-    return '$time';
-  }
-}
+class _ColorFormatter extends Formatter {
+  static const _green = 32;
+  static const _red = 31;
+  static const _escape = '\u001b';
 
-String _timeString(Duration d) {
-  var min = d.inMinutes;
-  var sec = d.inSeconds % 60;
-  return '${_padTime(min)}:${_padTime(sec)}';
-}
+  const _ColorFormatter() : super._();
 
-class Formatter {
-  const Formatter();
-  String passed(msg) => msg;
-  String failed(msg) => msg;
-}
+  String passed(String message) => _color(message, _green);
+  String failed(String message) => _color(message, _red);
 
-class ColorFormatter extends Formatter {
-  static int BOLD = 1;
-  static int GREEN = 32;
-  static int RED = 31;
-  static int NONE = 0;
-  static String ESCAPE = decodeUtf8([27]);
-
-  String passed(String msg) => _color(msg, GREEN);
-  String failed(String msg) => _color(msg, RED);
-
-  static String _color(String msg, int color) {
-    return "$ESCAPE[${color}m$msg$ESCAPE[0m";
-  }
-}
-
-List<String> _buildFailureOutput(TestCase test,
-    [Formatter formatter = const Formatter()]) {
-  List<String> getLinesWithoutCarriageReturn(List<int> output) {
-    return decodeUtf8(output)
-        .replaceAll('\r\n', '\n')
-        .replaceAll('\r', '\n')
-        .split('\n');
-  }
-
-  List<String> output = new List<String>();
-  output.add('');
-  output.add(formatter.failed('FAILED: ${test.configurationString}'
-      ' ${test.displayName}'));
-  StringBuffer expected = new StringBuffer();
-  expected.write('Expected: ');
-  for (var expectation in test.expectedOutcomes) {
-    expected.write('$expectation ');
-  }
-  output.add(expected.toString());
-  output.add('Actual: ${test.result}');
-  if (!test.lastCommandOutput.hasTimedOut) {
-    if (test.commandOutputs.length != test.commands.length &&
-        !test.expectCompileError) {
-      output.add('Unexpected compile-time error.');
-    } else {
-      if (test.expectCompileError) {
-        output.add('Compile-time error expected.');
-      }
-      if (test.hasRuntimeError) {
-        output.add('Runtime error expected.');
-      }
-      if (test.configuration['checked'] && test.isNegativeIfChecked) {
-        output.add('Dynamic type error expected.');
-      }
-    }
-  }
-  for (var i = 0; i < test.commands.length; i++) {
-    var command = test.commands[i];
-    var commandOutput = test.commandOutputs[command];
-    if (commandOutput != null) {
-      output.add("CommandOutput[${command.displayName}]:");
-      if (!commandOutput.diagnostics.isEmpty) {
-        String prefix = 'diagnostics:';
-        for (var s in commandOutput.diagnostics) {
-          output.add('$prefix ${s}');
-          prefix = '   ';
-        }
-      }
-      if (!commandOutput.stdout.isEmpty) {
-        output.add('');
-        output.add('stdout:');
-        output.addAll(getLinesWithoutCarriageReturn(commandOutput.stdout));
-      }
-      if (!commandOutput.stderr.isEmpty) {
-        output.add('');
-        output.add('stderr:');
-        output.addAll(getLinesWithoutCarriageReturn(commandOutput.stderr));
-      }
-    }
-  }
-  if (test is BrowserTestCase) {
-    // Additional command for rerunning the steps locally after the fact.
-    var command = test.configuration["_servers_"].httpServerCommandline();
-    output.add('');
-    output.add('To retest, run:  $command');
-  }
-  for (var i = 0; i < test.commands.length; i++) {
-    var command = test.commands[i];
-    var commandOutput = test.commandOutputs[command];
-    output.add('');
-    output.add('Command[${command.displayName}]: $command');
-    if (commandOutput != null) {
-      output.add('Took ${commandOutput.time}');
-    } else {
-      output.add('Did not run');
-    }
-  }
-
-  var arguments = ['python', 'tools/test.py'];
-  arguments.addAll(test.configuration['_reproducing_arguments_']);
-  arguments.add(test.displayName);
-  var testPyCommandline = arguments.map(escapeCommandLineArgument).join(' ');
-
-  output.add('');
-  output.add('Short reproduction command (experimental):');
-  output.add("    $testPyCommandline");
-  return output;
-}
-
-String _buildSummaryEnd(int failedTests) {
-  if (failedTests == 0) {
-    return '\n===\n=== All tests succeeded\n===\n';
-  } else {
-    var pluralSuffix = failedTests != 1 ? 's' : '';
-    return '\n===\n=== ${failedTests} test$pluralSuffix failed\n===\n';
-  }
+  static String _color(String message, int color) =>
+      "$_escape[${color}m$message$_escape[0m";
 }
 
 class EventListener {
   void testAdded() {}
   void done(TestCase test) {}
   void allTestsKnown() {}
   void allDone() {}
 }
 
 class ExitCodeSetter extends EventListener {
   void done(TestCase test) {
     if (test.unexpectedOutput) {
-      io.exitCode = 1;
+      exitCode = 1;
     }
   }
 }
 
 class IgnoredTestMonitor extends EventListener {
   static final int maxIgnored = 5;
 
   int countIgnored = 0;
 
   void done(TestCase test) {
@@ skipping 21 matching lines @@
     if (test.isFlaky && test.result != Expectation.PASS) {
       var buf = new StringBuffer();
       for (var l in _buildFailureOutput(test)) {
         buf.write("$l\n");
       }
       _appendToFlakyFile(buf.toString());
     }
   }
 
   void _appendToFlakyFile(String msg) {
-    var file = new File(TestUtils.flakyFileName());
+    var file = new File(TestUtils.flakyFileName);
     var fd = file.openSync(mode: FileMode.APPEND);
     fd.writeStringSync(msg);
     fd.closeSync();
   }
 }
 
 class TestOutcomeLogWriter extends EventListener {
   /*
    * The ".test-outcome.log" file contains one line for every executed test.
    * Such a line is an encoded JSON data structure of the following form:
@@ skipping 76 matching lines @@
       },
     });
   }
 
   void allDone() {
     if (_sink != null) _sink.close();
   }
 
   void _writeTestOutcomeRecord(Map record) {
     if (_sink == null) {
-      _sink = new File(TestUtils.testOutcomeFileName())
+      _sink = new File(TestUtils.testOutcomeFileName)
           .openWrite(mode: FileMode.APPEND);
     }
     _sink.write("${JSON.encode(record)}\n");
   }
 }
 
 class UnexpectedCrashLogger extends EventListener {
   final archivedBinaries = <String, String>{};
 
   void done(TestCase test) {
@@ skipping 168 matching lines @@
 
   void done(TestCase test) {
     for (var commandOutput in test.commandOutputs.values) {
       if (commandOutput.compilationSkipped) _skippedCompilations++;
     }
   }
 
   void allDone() {
     if (_skippedCompilations > 0) {
       print('\n$_skippedCompilations compilations were skipped because '
-          'the previous output was already up to date\n');
+          'the previous output was already up to date.\n');
     }
   }
 }
 
 class LineProgressIndicator extends EventListener {
   void done(TestCase test) {
     var status = 'pass';
     if (test.unexpectedOutput) {
       status = 'fail';
     }
     print('Done ${test.configurationString} ${test.displayName}: $status');
   }
 }
 
 class TestFailurePrinter extends EventListener {
-  bool _printSummary;
-  var _formatter;
-  var _failureSummary = <String>[];
-  var _failedTests = 0;
+  final bool _printSummary;
+  final Formatter _formatter;
+  final _failureSummary = <String>[];
+  int _failedTests = 0;
 
-  TestFailurePrinter(this._printSummary, [this._formatter = const Formatter()]);
+  TestFailurePrinter(this._printSummary, [this._formatter = Formatter.normal]);
 
   void done(TestCase test) {
     if (test.unexpectedOutput) {
       _failedTests++;
       var lines = _buildFailureOutput(test, _formatter);
       for (var line in lines) {
         print(line);
       }
       print('');
       if (_printSummary) {
         _failureSummary.addAll(lines);
         _failureSummary.add('');
       }
     }
   }
 
   void allDone() {
     if (_printSummary) {
       if (!_failureSummary.isEmpty) {
         print('\n=== Failure summary:\n');
-        for (String line in _failureSummary) {
+        for (var line in _failureSummary) {
           print(line);
         }
         print('');
 
         print(_buildSummaryEnd(_failedTests));
       }
     }
   }
 }
 
 class ProgressIndicator extends EventListener {
   ProgressIndicator(this._startTime);
 
+  static EventListener fromName(
+      String name, DateTime startTime, Formatter formatter) {
+    switch (name) {
+      case 'compact':
+        return new CompactProgressIndicator(startTime, formatter);
+      case 'line':
+        return new LineProgressIndicator();
+      case 'verbose':
+        return new VerboseProgressIndicator(startTime);
+      case 'status':
+        return new ProgressIndicator(startTime);
+      case 'buildbot':
+        return new BuildbotProgressIndicator(startTime);
+      default:
+        throw new ArgumentError('Unknown progress indicator "$name".');
+    }
+  }
+
   void testAdded() {
     _foundTests++;
   }
 
   void done(TestCase test) {
     if (test.unexpectedOutput) {
       _failedTests++;
     } else {
       _passedTests++;
     }
@@ skipping 25 matching lines @@
     }
     print('');
   }
 
   void _printDoneProgress(TestCase test) => _printProgress();
 
   void _printProgress();
 }
 
 class CompactProgressIndicator extends CompactIndicator {
-  Formatter _formatter;
+  final Formatter _formatter;
 
   CompactProgressIndicator(DateTime startTime, this._formatter)
       : super(startTime);
 
   void _printProgress() {
     var percent = ((_completedTests() / _foundTests) * 100).toInt().toString();
-    var progressPadded = _pad(_allTestsKnown ? percent : '--', 3);
-    var passedPadded = _pad(_passedTests.toString(), 5);
-    var failedPadded = _pad(_failedTests.toString(), 5);
-    Duration d = (new DateTime.now()).difference(_startTime);
-    var progressLine = '\r[${_timeString(d)} | $progressPadded% | '
+    var progressPadded = (_allTestsKnown ? percent : '--').padLeft(3);
+    var passedPadded = _passedTests.toString().padLeft(5);
+    var failedPadded = _failedTests.toString().padLeft(5);
+    var elapsed = (new DateTime.now()).difference(_startTime);
+    var progressLine = '\r[${_timeString(elapsed)} | $progressPadded% | '
         '+${_formatter.passed(passedPadded)} | '
         '-${_formatter.failed(failedPadded)}]';
     stdout.write(progressLine);
   }
 }
 
 class VerboseProgressIndicator extends ProgressIndicator {
   VerboseProgressIndicator(DateTime startTime) : super(startTime);
 
   void _printDoneProgress(TestCase test) {
     var status = 'pass';
     if (test.unexpectedOutput) {
       status = 'fail';
     }
     print('Done ${test.configurationString} ${test.displayName}: $status');
   }
 }
 
 class BuildbotProgressIndicator extends ProgressIndicator {
   static String stepName;
-  var _failureSummary = <String>[];
+  final _failureSummary = <String>[];
 
   BuildbotProgressIndicator(DateTime startTime) : super(startTime);
 
   void done(TestCase test) {
     super.done(test);
     if (test.unexpectedOutput) {
       _failureSummary.addAll(_buildFailureOutput(test));
     }
   }
 
@@ skipping 16 matching lines @@
       }
       for (String line in _failureSummary) {
         print(line);
       }
       print('');
     }
     print(_buildSummaryEnd(_failedTests));
   }
 }
 
-EventListener progressIndicatorFromName(
-    String name, DateTime startTime, Formatter formatter) {
-  switch (name) {
-    case 'compact':
-      return new CompactProgressIndicator(startTime, formatter);
-    case 'line':
-      return new LineProgressIndicator();
-    case 'verbose':
-      return new VerboseProgressIndicator(startTime);
-    case 'status':
-      return new ProgressIndicator(startTime);
-    case 'buildbot':
-      return new BuildbotProgressIndicator(startTime);
-    default:
-      assert(false);
-      return null;
+String _timeString(Duration duration) {
+  var min = duration.inMinutes;
+  var sec = duration.inSeconds % 60;
+  return '${min.toString().padLeft(2, '0')}:${sec.toString().padLeft(2, '0')}';
+}
+
+List<String> _linesWithoutCarriageReturn(List<int> output) {
+  return decodeUtf8(output)
+      .replaceAll('\r\n', '\n')
+      .replaceAll('\r', '\n')
+      .split('\n');
+}
+
+List<String> _buildFailureOutput(TestCase test,
+    [Formatter formatter = Formatter.normal]) {
+  var output = [
+    '',
+    formatter.failed('FAILED: ${test.configurationString}${test.displayName}')

[ahe, 2017/05/09 07:30:00]

There's a space missing between ${test.configurationString} and
${test.displayName}

[Bob Nystrom, 2017/05/09 17:32:36]

Good catch, thanks!

https://codereview.chromium.org/2866273002/

+  ];
+
+  var expected = new StringBuffer();
+  expected.write('Expected: ');
+  for (var expectation in test.expectedOutcomes) {
+    expected.write('$expectation ');
+  }
+
+  output.add(expected.toString());
+  output.add('Actual: ${test.result}');
+  if (!test.lastCommandOutput.hasTimedOut) {
+    if (test.commandOutputs.length != test.commands.length &&
+        !test.expectCompileError) {
+      output.add('Unexpected compile-time error.');
+    } else {
+      if (test.expectCompileError) {
+        output.add('Compile-time error expected.');
+      }
+      if (test.hasRuntimeError) {
+        output.add('Runtime error expected.');
+      }
+      if (test.configuration['checked'] && test.isNegativeIfChecked) {
+        output.add('Dynamic type error expected.');
+      }
+    }
+  }
+
+  for (var i = 0; i < test.commands.length; i++) {
+    var command = test.commands[i];
+    var commandOutput = test.commandOutputs[command];
+    if (commandOutput != null) {
+      output.add("CommandOutput[${command.displayName}]:");
+      if (!commandOutput.diagnostics.isEmpty) {
+        String prefix = 'diagnostics:';
+        for (var s in commandOutput.diagnostics) {
+          output.add('$prefix ${s}');
+          prefix = '   ';
+        }
+      }
+      if (!commandOutput.stdout.isEmpty) {
+        output.add('');
+        output.add('stdout:');
+        output.addAll(_linesWithoutCarriageReturn(commandOutput.stdout));
+      }
+      if (!commandOutput.stderr.isEmpty) {
+        output.add('');
+        output.add('stderr:');
+        output.addAll(_linesWithoutCarriageReturn(commandOutput.stderr));
+      }
+    }
+  }
+
+  if (test is BrowserTestCase) {
+    // Additional command for rerunning the steps locally after the fact.
+    var command = test.configuration["_servers_"].httpServerCommandLine();
+    output.add('');
+    output.add('To retest, run:  $command');
+  }
+
+  for (var i = 0; i < test.commands.length; i++) {
+    var command = test.commands[i];
+    var commandOutput = test.commandOutputs[command];
+    output.add('');
+    output.add('Command[${command.displayName}]: $command');
+    if (commandOutput != null) {
+      output.add('Took ${commandOutput.time}');
+    } else {
+      output.add('Did not run');
+    }
+  }
+
+  var arguments = ['python', 'tools/test.py'];
+  arguments.addAll(test.configuration['_reproducing_arguments_']);
+  arguments.add(test.displayName);
+  var testPyCommandline = arguments.map(escapeCommandLineArgument).join(' ');
+
+  output.add('');
+  output.add('Short reproduction command (experimental):');
+  output.add("    $testPyCommandline");
+  return output;
+}
+
+String _buildSummaryEnd(int failedTests) {
+  if (failedTests == 0) {
+    return '\n===\n=== All tests succeeded\n===\n';
+  } else {
+    var pluralSuffix = failedTests != 1 ? 's' : '';
+    return '\n===\n=== ${failedTests} test$pluralSuffix failed\n===\n';
   }
 }