Added guts to pull call signatures when assertions & expectations fail.

chrome/test/data/webui/test_api.js

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview Library providing basic test framework functionality.
  **/
 
 /**
  * Namespace for |Test|.
@@ skipping 56 matching lines @@
 
     /**
      * When set to a function, will be called in the context of the test
      * generation inside the function, and before any generated C++.
      * @type {function(string,string)}
      **/
     testGenPreamble: null,
 
     /**
      * When set to a function, will be called in the context of the test
      * generation inside the function, and before any generated C++.

[David Tseng, 2011/07/18 18:31:59]

nit: after?

[Sheridan Rawlins, 2011/07/19 01:19:25]

Done.

      * @type {function(string,string)}
      **/
     testGenPostamble: null,
 
     /**
      * When set to a non-null String, auto-generate typedef before generating
      * TEST*: {@code typedef typedefCppFixture testFixture}.
      * @type {String}
      **/
     typedefCppFixture: 'WebUIBrowserTest',
@@ skipping 58 matching lines @@
     },
 
     /**
      * Runs this test case.
      * @type {Function}
      **/
     Run: function() {
       if (this.fixture)
         this.fixture.SetUp();
       if (this.body)
         this.body.call(this.fixture);

[David Tseng, 2011/07/18 18:31:59]

Dnit: Does the |body| expect |fixture| to be non-null?

[Sheridan Rawlins, 2011/07/19 01:19:25]

It doesn't need to, but it may, if you are writing TEST_F cases.  For TEST
cases, the implementation _may_ be null or undefined and still run fine as long
as the body doesn't depend upon this - test author should know.

I added a better comment.

On 2011/07/18 18:31:59, David Tseng wrote:

       if (this.fixture)
         this.fixture.TearDown();
     },
   };
 
   /**
    * Registry of javascript-defined callbacks for {@code chrome.send}.
    * @type {Object}
    **/
   var sendCallbacks = {};
@@ skipping 43 matching lines @@
    **/
   function send(messageName) {
     var callback = sendCallbacks[messageName];
     var args = Array.prototype.slice.call(arguments, 1);
     if (callback != undefined)
       callback[1].apply(callback[0], args);
     else
       oldChrome.send.apply(oldChrome, args);
   }
 
+  function CallHelper() {

[David Tseng, 2011/07/18 18:31:59]

Document.

[Sheridan Rawlins, 2011/07/19 01:19:25]

Done.

+    this.__proto__ = CallHelper.prototype;
+  }
+
+  CallHelper.prototype = {
+    'counts_': {},

[David Tseng, 2011/07/18 18:31:59]

nit: should document this since it's not obvious you're using this to map the
function signatures to a count.

[Sheridan Rawlins, 2011/07/19 01:19:25]

Done.

+    'getStackInfo': function(caller) {

[David Tseng, 2011/07/18 18:31:59]

nit: clearer to call this 'getCallerInfo", or 'getCallerSignature'?

[Sheridan Rawlins, 2011/07/19 01:19:25]

used getCallerInfo.  Done.

On 2011/07/18 18:31:59, David Tseng wrote:

+      var callerName = caller.name;
+      var callerCaller = caller.caller;
+      if (callerCaller['isExpect']) {
+        callerName = callerCaller.expectName;
+        callerCaller = callerCaller.caller;
+      }
+      var callerCallerString = callerCaller.toString();
+      return {
+        callerName: callerName,
+        callerCallerString: callerCallerString,
+      };
+    },
+    'registerCall': function() {
+      var stackInfo = this.getStackInfo(arguments.callee.caller);
+      if (!(stackInfo.callerCallerString in this.counts_))
+        this.counts_[stackInfo.callerCallerString] = {};
+      if (!(stackInfo.callerName in this.counts_[stackInfo.callerCallerString]))
+        this.counts_[stackInfo.callerCallerString][stackInfo.callerName] = 0;

[David Tseng, 2011/07/18 18:31:59]

could we just combine the two strings
stackInfo.callerCallerString, stackInfo.callerName

as the key to count map?  Perhaps add a "key" property to the "stackInfo"
object?  Then, we could combine the two blocks above.

[Sheridan Rawlins, 2011/07/19 01:19:25]

I could, but then we'd need to use a separator character (which could be ''). I
don't like it as much as just indexing on two variables - it is really a
2-dimensional array, so why not code it as such?

On 2011/07/18 18:31:59, David Tseng wrote:

+      ++this.counts_[stackInfo.callerCallerString][stackInfo.callerName];
+    },
+    'matchedParamsIndex': function(string, index) {

[David Tseng, 2011/07/18 18:31:59]

nit: up to you, but it seems like we can do this much much more easily with
String.match and a regex.

[Sheridan Rawlins, 2011/07/19 01:19:25]

Hah, that's what you'd think. Unfortunately, it's not true - read the following
link for more info.

http://www.texttoolkit.com/index.php?option=com_content&view=article&id=63:pa...

On 2011/07/18 18:31:59, David Tseng wrote:

[David Tseng, 2011/07/19 16:53:03]

True, but are we not just looking for strings of the pattern
foo(...);
i.e.
functionName + "(" + "\s\S" + ");"

Also, the article correctly prints out that this is hard to do with a true
language semantic parser because of cases like:
foo("I'm a string with ( a paren!");
On 2011/07/19 01:19:25, Sheridan Rawlins wrote:

[Sheridan Rawlins, 2011/07/19 16:57:40]

What about
  assertTrue(expected, function(foo, bar))?

I also wouldn't want to come back and debug if someone did something like you
mentioned:
  assertTrue("(", getParen());

-SCR

On 2011/07/19 16:53:03, David Tseng wrote:

[David Tseng, 2011/07/19 16:58:13]

I don't think the code as is handles this case.

[David Tseng, 2011/07/19 17:46:50]

You can make reg ex's lazy by using "?".  Also note that we're matching on ");"
and not just ")" for any reg ex's.
This example breaks the current patch doesn't it?

Again, I'm ok with this patch if it works and we can revisit later  if needed.

[Sheridan Rawlins, 2011/07/21 02:05:27]

Mea Culpa; good catch, David.

Yeah, short of being able to get the AST (I don't think that API is available
within Javascript), any parsing is going to have gotchas.  I like the regex of
ending on ); but even that could have assertEquals(");", foo);

Matching without being a parser is hard, and as long as we have to pick the
lesser of two evils, maybe regex is easier to read/maintain.

I just uploaded the RegExp version - it does seem much easier to understand.

Done.

On 2011/07/19 17:46:50, David Tseng wrote:

+      var args = [];
+      var parenCount = 1;
+      var argStart = index + 1;
+      for(index = argStart;
+          parenCount && index < string.length;
+          ++index) {
+        if (string[index] == '(') {
+          ++parenCount;
+        } else if (string[index] == ')') {
+          if (--parenCount == 0)
+            args.push(string.substring(argStart, index));
+        } else if (string[index] == ',' && parenCount == 1) {
+          args.push(string.substring(argStart, index));
+          argStart = index + 1;
+        }
+      }
+      return [index, args];
+    },
+    'getParams': function(caller_, count_) {
+      var caller = (caller_ == undefined) ?
+          arguments.callee.caller : caller_;
+      var stackInfo = this.getStackInfo(caller);
+      var count = (count_ == undefined) ?
+          this.counts_[stackInfo.callerCallerString][stackInfo.callerName] :
+          count_;
+      var searchStart = 0;
+      var args;
+      for(var i = 0;
+          i < count && searchStart < stackInfo.callerCallerString.length;
+          ++i) {
+        searchStart = stackInfo.callerCallerString.indexOf(
+            stackInfo.callerName, searchStart);
+        if (searchStart == -1) {
+          if (i && count_ == undefined) {
+            return this.getParams(caller, ((count - 1) % i) + 1);
+          } else {
+            console.error('bad count ' + count);
+            return undefined;
+          }
+        }
+        searchStart += stackInfo.callerName.length;
+        var matched = this.matchedParamsIndex(stackInfo.callerCallerString,
+                                              searchStart);
+        args = matched[1];
+        searchStart = matched[0];
+      }
+      return args;
+    },
+    'getCall': function() {
+      var caller = arguments.callee.caller;
+      var stackInfo = this.getStackInfo(caller);
+      return stackInfo.callerName + '(' + this.getParams(caller) + ')';
+    },
+  };
+
+  var helper = new CallHelper();
+
   // Asserts.
   // Use the following assertions to verify a condition within a test.
   // If assertion fails, the C++ backend will be immediately notified.
   // If assertion passes, no notification will be sent to the C++ backend.
 
   /**
    * When |test| !== |expected|, aborts the current test.
    * @param {Boolean} test The predicate to check against |expected|.
    * @param {Boolean} expected The expected value of |test|.
    * @param {String=} message The message to include in the Error thrown.
    * @throws {Error} upon failure.
    **/
   function assertBool(test, expected, message) {
-    if (test !== expected) {
-      if (message)
-        message = test + '\n' + message;
-      else
-        message = test;
-      throw new Error(message);
-    }
+    helper.registerCall();
+    if (test !== expected)
+      throw new Error('Test Error ' + helper.getCall() + ': ' + test);
   }
 
   /**
    * When |test| !== true, aborts the current test.
    * @param {Boolean} test The predicate to check against |expected|.
    * @param {String=} message The message to include in the Error thrown.
    * @throws {Error} upon failure.
    **/
   function assertTrue(test, message) {
-    assertBool(test, true, message);
+    helper.registerCall();
+    if (test !== true)
+      throw new Error('Test Error ' + helper.getCall() + ': ' + test);
   }
 
   /**
    * When |test| !== false, aborts the current test.
    * @param {Boolean} test The predicate to check against |expected|.
    * @param {String=} message The message to include in the Error thrown.
    * @throws {Error} upon failure.
    **/
   function assertFalse(test, message) {
-    assertBool(test, false, message);
+    helper.registerCall();
+    if (test !== false)
+      throw new Error('Test Error ' + helper.getCall() + ': ' + test);
   }
 
   /**
    * When |expected| !== |actual|, aborts the current test.
    * @param {*} expected The predicate to check against |expected|.
    * @param {*} actual The expected value of |test|.
    * @param {String=} message The message to include in the Error thrown.
    * @throws {Error} upon failure.
    **/
   function assertEquals(expected, actual, message) {
+    helper.registerCall();
     if (expected != actual) {
-      throw new Error('Test Error. Actual: ' + actual + '\nExpected: ' +
-                       expected + '\n' + message);
+      throw new Error('Test Error ' + helper.getCall() + '\nActual: ' + actual +
+                      '\nExpected: ' + expected);
     }
     if (typeof expected != typeof actual) {
-      throw new Error('Test Error' +
-                      ' (type mismatch)\nActual Type: ' + typeof actual +
-                      '\nExpected Type:' + typeof expected + '\n' + message);
+      throw new Error('Test Error (type mismatch) ' + helper.getCall() +
+                      '\nActual Type: ' + typeof actual +
+                      '\nExpected Type:' + typeof expected);
     }
   }
 
   /**
    * Always aborts the current test.
    * @param {String=} message The message to include in the Error thrown.
    * @throws {Error} always.
    **/
   function assertNotReached(message) {
-    throw new Error(message);
+    helper.registerCall();
+    throw new Error(helper.getCall());
   }
 
   /**
    * Holds the errors, if any, caught by expects so that the test case can fail.
    * @type {Array.<Error>}
    **/
   var errors = [];
 
   /**
    * Creates a function based upon a function that thows an exception on
    * failure. The new function stuffs any errors into the |errors| array for
    * checking by runTest. This allows tests to continue running other checks,
    * while failing the overal test if any errors occurrred.
    * @param {Function} assertFunc The function which may throw an Error.
    * @return {Function} A function that applies its arguments to |assertFunc|.
    * @see errors
    * @see runTest
    **/
   function createExpect(assertFunc) {
-    return function() {
+    var expectFunc = function() {
       try {
         assertFunc.apply(null, arguments);
       } catch (e) {
         errors.push(e);
       }
     };
+    expectFunc.isExpect = true;
+    expectFunc.expectName = assertFunc.name.replace(/^assert/, 'expect');
+    return expectFunc;
   }
 
   /**
    * This is the starting point for tests run by WebUIBrowserTest. It clears
    * |errors|, runs the test surrounded by an expect to catch Errors. If
    * |errors| is non-empty, it reports a failure and a message by joining
    * |errors|.
    * @param {String} testFunction The function name to call.
    * @param {Array} testArguments The arguments to call |testFunction| with.
    * @return {Array.<Boolean, String>} [test-succeeded, message-if-failed]
    * @see errors
    * @see createExpect
    **/
   function runTest(testFunction, testArguments) {
-    errors = [];
+    errors.splice(0, errors.length);
     // Avoid eval() if at all possible, since it will not work on pages
     // that have enabled content-security-policy.
     var testBody = this[testFunction];    // global object -- not a method.
     if (typeof testBody === "undefined")
       testBody = eval(testFunction);
-    if (testBody != RUN_TEST_F)
-      console.log('Running test ' + testBody.name);
+    if (testBody != RUN_TEST_F) {
+      var testName =
+          testFunction.name ? testFunction.name : testBody.toString();
+      console.log('Running test ' + testName);
+    }
     createExpect(testBody).apply(null, testArguments);
 
+    var result = [true];
     if (errors.length) {
+      var message = '';
       for (var i = 0; i < errors.length; ++i) {
-        console.log('Failed: ' + testFunction + '(' +
-                    testArguments.toString() + ')\n' + errors[i].stack);
+        message += 'Failed: ' + testFunction + '(' +
+                   testArguments.map(JSON.stringify) +
+                   ')\n' + errors[i].stack;
       }
-      return [false, errors.join('\n')];
-    } else {
-      return [true];
+      errors.splice(0, errors.length);
+      result = [false, message];
     }
+    return result;
   }
 
   /**
    * Creates a new test case for the given |testFixture| and |testName|. Assumes
    * |testFixture| describes a globally available subclass of type Test.
    * @param {String} testFixture The fixture for this test case.
    * @param {String} testName The name for this test case.
    * @return {TestCase} A newly created TestCase.
    **/
   function createTestCase(testFixture, testName) {
@@ skipping 134 matching lines @@
   window.registerMockMessageCallbacks = registerMockMessageCallbacks;
   window.runTest = runTest;
   window.preloadJavascriptLibraries = preloadJavascriptLibraries;
   window.TEST = TEST;
   window.TEST_F = TEST_F;
   window.GEN = GEN;
 
   // Import the Mock4JS helpers.
   Mock4JS.addMockSupport(window);
 })();