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

chrome/test/data/webui/test_api.js

Index: chrome/test/data/webui/test_api.js
diff --git a/chrome/test/data/webui/test_api.js b/chrome/test/data/webui/test_api.js
index dc3a8a0e3abc2652e6926f8682eac1da47fe3934..1d0144ea06e020330500af6656ef15eccc415f78 100644
--- a/chrome/test/data/webui/test_api.js
+++ b/chrome/test/data/webui/test_api.js
@@ -217,6 +217,91 @@ var currentTestCase = null;
       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.
@@ -230,13 +315,9 @@ var currentTestCase = null;
    * @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);
   }
 
   /**
@@ -246,7 +327,9 @@ var currentTestCase = null;
    * @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);
   }
 
   /**
@@ -256,7 +339,9 @@ var currentTestCase = null;
    * @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);
   }
 
   /**
@@ -267,14 +352,15 @@ var currentTestCase = null;
    * @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);
     }
   }
 
@@ -284,7 +370,8 @@ var currentTestCase = null;
    * @throws {Error} always.
    **/
   function assertNotReached(message) {
-    throw new Error(message);
+    helper.registerCall();
+    throw new Error(helper.getCall());
   }
 
   /**
@@ -304,13 +391,16 @@ var currentTestCase = null;
    * @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;
   }
 
   /**
@@ -325,25 +415,31 @@ var currentTestCase = null;
    * @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;
   }
 
   /**