pkg/unittest: cleanup to logMessage

pkg/unittest/lib/src/config.dart

 // Copyright (c) 2011, 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.
 
 part of unittest;
 
 /**
  * Hooks to configure the unittest library for different platforms. This class
  * implements the API in a platform-independent way. Tests that want to take
  * advantage of the platform can create a subclass and override methods from
  * this class.
  */
 
 class Configuration {
   // The VM won't shut down if a receive port is open. Use this to make sure
   // we correctly wait for asynchronous tests.
   ReceivePort _receivePort;
 
   /**
    * Subclasses can override this with something useful for diagnostics.
    * Particularly useful in cases where we have parent/child configurations
    * such as layout tests.
    */
-  String get name => 'Configuration';
+  final String name = 'Configuration';
 
   /**
    * If true, then tests are started automatically (otherwise [runTests]
    * must be called explicitly after the tests are set up.
    */
-  bool get autoStart => true;
+  final bool autoStart = true;
 
   /**
    * Called as soon as the unittest framework becomes initialized. This is done
    * even before tests are added to the test framework. It might be used to
    * determine/debug errors that occur before the test harness starts executing.
    * It is also used to tell the vm or browser that tests are going to be run
    * asynchronously and that the process should wait until they are done.
    */
   void onInit() {
     _receivePort = new ReceivePort();
@@ skipping 22 matching lines @@
   /**
    * Called when an already completed test changes state; for example a test
    * that was marked as passing may later be marked as being in error because
    * it still had callbacks being invoked.
    */
   void onTestResultChanged(TestCase testCase) {
     assert(testCase != null);
   }
 
   /**
-   * Can be called by tests to log status. Tests should use this
-   * instead of print. Subclasses should not override this; they
-   * should instead override logMessage which is passed the test case.
-   */
-  void logMessage(String message) {
-    if (currentTestCase == null) {
-      // Before or after tests run. In this case we pass null for the test
-      // case reference and let the config decide what to do with this.
-      logTestCaseMessage(null, message);
-    } else {
-      logTestCaseMessage(currentTestCase, message);
-    }
-  }
-
-  /**
    * Handles the logging of messages by a test case. The default in
    * this base configuration is to call print();
    */
-  void logTestCaseMessage(TestCase testCase, String message) {
+  void onLogMessage(TestCase testCase, String message) {

[kevmoo-old, 2013/03/29 02:41:43]

While we're breaking things, this should look more like a "protected" method.

Designed to be overridden, not called directly.

     print(message);
   }
 
   /**
    * Called with the result of all test cases. The default implementation prints
    * the result summary using the built-in [print] command. Browser tests
    * commonly override this to reformat the output.
    *
    * When [uncaughtError] is not null, it contains an error that occured outside
    * of tests (e.g. setting up the test).
    */
   void onSummary(int passed, int failed, int errors, List<TestCase> results,
       String uncaughtError) {
     // Print each test's result.
-    for (final t in testCases) {
+    for (final t in results) {
       var resultString = "${t.result}".toUpperCase();
       print('$resultString: ${t.description}');
 
       if (t.message != '') {
         print(_indent(t.message));
       }
 
       if (t.stackTrace != null && t.stackTrace != '') {
         print(_indent(t.stackTrace));
       }
     }
 
     // Show the summary.
     print('');
 
-    var success = false;
     if (passed == 0 && failed == 0 && errors == 0 && uncaughtError == null) {
       print('No tests found.');
       // This is considered a failure too.
     } else if (failed == 0 && errors == 0 && uncaughtError == null) {
       print('All $passed tests passed.');
-      success = true;

[Andrei Mouravski, 2013/03/29 04:27:48]

Why not instead change the signature and return a boolean for success? 

[kevmoo-old, 2013/03/29 13:20:41]

Because the caller is the unit test framework. It already knows. :-)

     } else {
       if (uncaughtError != null) {
         print('Top-level uncaught error: $uncaughtError');
       }
       print('$passed PASSED, $failed FAILED, $errors ERRORS');
     }
   }
 
   /**
    * Called when the unittest framework is done running. [success] indicates
@@ skipping 21 matching lines @@
   // Currently e.message works in dartium, but not in dartc.
   void handleExternalError(e, String message, [String stack = '']) =>
       _reportTestError('$message\nCaught $e', stack);
 
   _postMessage(String message) {
     // In dart2js browser tests, the JavaScript-based test controller
     // intercepts calls to print and listens for "secret" messages.
     print(message);
   }
 }