Convert asynchronous closure test in cr.ui framework to a browser test.

Convert asynchronous closure test in cr.ui framework to a browser test.
BUG=249104
TEST=WebUIResourceBrowserTest

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=207929

[kevers, 2013-06-20 14:37:09]

Erik and Dan,

Can you please have a look at this CL.

[arv (Not doing code reviews), 2013-06-20 15:46:48]

Lets take a cue from modern testing frameworks like Mocha and look at length of
the test function to determine if the test is async or not.

function testThisIsAsync(done) {
  setTimeout(function() {
    assertTrue(true);
    done();
  });
}

The done function is the continuation function.

[arv (Not doing code reviews), 2013-06-20 15:49:38]

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/parse_html_subset_test.html (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:104: function testOnError() {
function testOnError(done) {

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:107: runningAsynchronousTest
= true;
then this goes away.

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:114: continueTesting();
done();

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:128: if (typeof window[name] ==
'function' && /^test/.test(name))
var isAsyncTest = window[name].length;

[kevers, 2013-06-20 17:34:18]

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/parse_html_subset_test.html (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:104: function testOnError() {
On 2013/06/20 15:49:38, arv wrote:
> function testOnError(done) {

Added testDoneCallback as argument to an asynchronous test.

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:107: runningAsynchronousTest
= true;
On 2013/06/20 15:49:38, arv wrote:
> then this goes away.

Done.

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/parse_html_subset_test.html:114: continueTesting();
On 2013/06/20 15:49:38, arv wrote:
> done();

Done.

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:128: if (typeof window[name] ==
'function' && /^test/.test(name))
On 2013/06/20 15:49:38, arv wrote:
> var isAsyncTest = window[name].length;

Added isAyncTest in the continueTesting function based on the name of the test
ending in 'Async'.

[arv (Not doing code reviews), 2013-06-20 17:45:15]

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:7: * @type {Array.<strring>}  List
of function names for tests to run.
Do we want to hide these in an IIFE?

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:136: function continueTesting() {
Does continueTesting need to be a global?

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:141: if(testCases.length > 0) {
if (

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:143: var isAsyncTest =
/Async$/.test(fn);
I still prefer fn.length instead

[kevers, 2013-06-20 18:22:12]

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:7: * @type {Array.<strring>}  List
of function names for tests to run.
On 2013/06/20 17:45:15, arv wrote:
> Do we want to hide these in an IIFE?

Done.

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:136: function continueTesting() {
On 2013/06/20 17:45:15, arv wrote:
> Does continueTesting need to be a global?

Mo longer a global.

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:141: if(testCases.length > 0) {
On 2013/06/20 17:45:15, arv wrote:
> if (

Done.

https://chromiumcodereview.appspot.com/16831021/diff/11001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:143: var isAsyncTest =
/Async$/.test(fn);
On 2013/06/20 17:45:15, arv wrote:
> I still prefer fn.length instead

Done.

[Dan Beam, 2013-06-20 18:31:46]

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/webui_resource_browsertest.cc (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_browsertest.cc:49: }
nit: use a do { } while () instead

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:7: * @type {Array.<strring>}  List
of function names for tests to run.
!Array.<string>

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:27: var runningAsynchronousTest =
false;
nit: move right above continueTesting() IMO

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:143: runningAsynchronousTest =
false;
nit: \n

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:147: }
nit: \n

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:148: if(testCases.length > 0) {
if (

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:152: window.setUp();
nit: \n

https://chromiumcodereview.appspot.com/16831021/diff/5001/chrome/test/data/we...
chrome/test/data/webui/webui_resource_test.js:165: }
nit: \n

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:106: var testCases_ = [];
nit: these aren't @private member variables, so they probably shouldn't have an
_

[arv (Not doing code reviews), 2013-06-20 18:33:02]

LGTM with nits

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:104: * @type {Array.<strring>} 
List of function names for tests to run.
typo

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:104: * @type {Array.<strring>} 
List of function names for tests to run.
Only one space after }

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:106: var testCases_ = [];
no underscore

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:116: * @type {?Function}
@type {Function}

Function is already nullable

[kevers, 2013-06-20 18:52:12]

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:104: * @type {Array.<strring>} 
List of function names for tests to run.
On 2013/06/20 18:33:03, arv wrote:
> Only one space after }

Done.

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:104: * @type {Array.<strring>} 
List of function names for tests to run.
On 2013/06/20 18:33:03, arv wrote:
> typo

Fixed.

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:106: var testCases_ = [];
On 2013/06/20 18:33:03, arv wrote:
> no underscore

Done.

https://chromiumcodereview.appspot.com/16831021/diff/15001/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:116: * @type {?Function}
On 2013/06/20 18:33:03, arv wrote:
> @type {Function}
> 
> Function is already nullable

Done.

[Dan Beam, 2013-06-20 21:42:14]

lgtm

https://chromiumcodereview.appspot.com/16831021/diff/22007/chrome/test/data/w...
File chrome/test/data/webui/webui_resource_test.js (right):

https://chromiumcodereview.appspot.com/16831021/diff/22007/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:104: * @type {Array.<string>} List
of function names for tests to run.
!Array, please read
https://developers.google.com/closure/compiler/docs/js-for-compiler#types

https://chromiumcodereview.appspot.com/16831021/diff/22007/chrome/test/data/w...
chrome/test/data/webui/webui_resource_test.js:130: console.error('\nFailed to
find test cases.');
remove '\n'

[kevers, 2013-06-20 21:57:00]

+sky for OWNERS on content/public/test.

[sky, 2013-06-20 23:26:26]

https://chromiumcodereview.appspot.com/16831021/diff/22007/content/public/tes...
File content/public/test/browser_test_utils.cc (right):

https://chromiumcodereview.appspot.com/16831021/diff/22007/content/public/tes...
content/public/test/browser_test_utils.cc:509: message_queue_.pop();
Should the pop happen regardless of message being non-null?

[kevers, 2013-06-21 13:45:39]

https://chromiumcodereview.appspot.com/16831021/diff/22007/content/public/tes...
File content/public/test/browser_test_utils.cc (right):

https://chromiumcodereview.appspot.com/16831021/diff/22007/content/public/tes...
content/public/test/browser_test_utils.cc:509: message_queue_.pop();
On 2013/06/20 23:26:26, sky wrote:
> Should the pop happen regardless of message being non-null?

Good catch. Fixed.

[sky, 2013-06-21 15:26:45]

LGTM

[commit-bot: I haz the power, 2013-06-21 16:56:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kevers@chromium.org/16831021/33001

[commit-bot: I haz the power, 2013-06-21 22:03:53]

Change committed as 207929