finished testing functions and added events to bookmark tests.

finished testing functions and added events to bookmark tests.
added infrastructure to better support events and cleaned up callback testing.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=25623

[Erik does not do reviews, 2009-09-04 21:25:15]

I haven't gone through and changed the other tests to use my changes yet.  I
thought it would be good to get an initial review on this first.

[asargent_no_longer_on_chrome, 2009-09-04 22:06:23]

http://codereview.chromium.org/192028/diff/1/6
File chrome/renderer/resources/extension_apitest.js (right):

http://codereview.chromium.org/192028/diff/1/6#newcode66
Line 66: totalCallbacks = 0;
Any reason you don't set totalCallbacks to 0 before calling chrome.test.succeed
here? If we didn't use setTimeout() inside chrome.test.succeed to schedule the
next test run, wouldn't this cause a failure?

http://codereview.chromium.org/192028/diff/1/6#newcode152
Line 152: chrome.test.callback = function(func, expectedError) {
Maybe add an assertion that typeof(func) == 'function', and that (expectedError
== null || typeof(expectedErorr) == 'boolean')

http://codereview.chromium.org/192028/diff/1/6#newcode165
Line 165: chrome.test.listenOnce = function(event, func) {
Nice! I like this guy.

[Erik does not do reviews, 2009-09-04 22:34:59]

http://codereview.chromium.org/192028/diff/1/6
File chrome/renderer/resources/extension_apitest.js (right):

http://codereview.chromium.org/192028/diff/1/6#newcode66
Line 66: totalCallbacks = 0;
On 2009/09/04 22:06:23, Antony Sargent wrote:
> Any reason you don't set totalCallbacks to 0 before calling
chrome.test.succeed
> here? If we didn't use setTimeout() inside chrome.test.succeed to schedule the
> next test run, wouldn't this cause a failure?

I did it so that I could log the total callbacks for debugging.  However, I
think your point is a good one, so I moved the log.

http://codereview.chromium.org/192028/diff/1/6#newcode152
Line 152: chrome.test.callback = function(func, expectedError) {
On 2009/09/04 22:06:23, Antony Sargent wrote:
> Maybe add an assertion that typeof(func) == 'function', and that
(expectedError
> == null || typeof(expectedErorr) == 'boolean')

actually, expectedError is supposed to be a string (you're expecting a
particular error message).  I added the assertions though.

This also made me realize that I was missing some sample code for the
fail()/expectedError case, so I added one of those too (and fixed a bug).

http://codereview.chromium.org/192028/diff/1/6#newcode165
Line 165: chrome.test.listenOnce = function(event, func) {
On 2009/09/04 22:06:23, Antony Sargent wrote:
> Nice! I like this guy.

Thanks.  Incidentally, I was looking at the tabs test, and this doesn't appear
to work cleanly in all of your tests, so it may need some work.

[asargent_no_longer_on_chrome, 2009-09-04 23:21:24]

LGTM