Migrate Remoting Webapp Unittests to use QUnit 2.0 syntax.

Migrate remoting webapp unittests to use QUnit 2.0 syntax.

Summary of changes:

Global renaming:
  test     => QUnit.test()
  equal    => assert.equal()
  notEqual => assert.notEqual()
  deepEqual=> assert.deepEqual()
  ok       => assert.ok()
  module   => QUnit.module()
  setup    => beforeEach
  teardown => afterEach

Async test migration:
  asyncTest     => QUnit.test(function(assert) {
                     ...
                   });
  QUnit.start() => var done = assert.async();
                   done();
  return promise directly for async tests.

BUG=467817
Committed: https://crrev.com/831563d9c72e009bdae47c174421da85faad3aa6
Cr-Commit-Position: refs/heads/master@{#320996}

[kelvinp, 2015-03-17 03:40:38]

kelvinp@chromium.org changed reviewers:
+ jamiewalch@chromium.org

[kelvinp, 2015-03-17 03:47:46]

kelvinp@chromium.org changed reviewers:
+ jrw@chromium.org

[kelvinp, 2015-03-17 03:47:47]

PTAL

[Jamie, 2015-03-17 17:57:16]

LGTM once my comments are addressed.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/base_event_hook_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base_event_hook_unittest.js:59: tearDown: function() {
afterEach?

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/ipc_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/ipc_unittest.js:57: QUnit.ok(false);
ok(false) is not very useful without an associated error message.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/error_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/error_unittest.js:13: }
Can you just remove these empty functions?

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/error_unittest.js:37:
QUnit.ok(!error.hasTag(remoting.Error.Tag.HOST_IS_OFFLINE));
Indentation.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/xhr_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/xhr_unittest.js:18: }
Remove?

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/js_prot...
File remoting/webapp/js_proto/qunit_proto.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/js_prot...
remoting/webapp/js_proto/qunit_proto.js:53: *   beforeEach: function(void),
Is it not valid to omit beforeEach?

[kelvinp, 2015-03-17 18:13:53]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-03-17 18:13:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1017613002/#ps40001
(title: "Reviewer's feedback")

[kelvinp, 2015-03-17 18:13:58]

FYI

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/base_event_hook_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/base_event_hook_unittest.js:59: tearDown: function() {
On 2015/03/17 17:57:15, Jamie wrote:
> afterEach?

Done.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
File remoting/webapp/base/js/ipc_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/base/js...
remoting/webapp/base/js/ipc_unittest.js:57: QUnit.ok(false);
On 2015/03/17 17:57:15, Jamie wrote:
> ok(false) is not very useful without an associated error message.

Done.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/error_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/error_unittest.js:13: }
On 2015/03/17 17:57:15, Jamie wrote:
> Can you just remove these empty functions?

Done.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/error_unittest.js:37:
QUnit.ok(!error.hasTag(remoting.Error.Tag.HOST_IS_OFFLINE));
On 2015/03/17 17:57:16, Jamie wrote:
> Indentation.

Done.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/xhr_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/xhr_unittest.js:18: }
On 2015/03/17 17:57:16, Jamie wrote:
> Remove?

Done.

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/js_prot...
File remoting/webapp/js_proto/qunit_proto.js (right):

https://codereview.chromium.org/1017613002/diff/20001/remoting/webapp/js_prot...
remoting/webapp/js_proto/qunit_proto.js:53: *   beforeEach: function(void),
On 2015/03/17 17:57:16, Jamie wrote:
> Is it not valid to omit beforeEach?

Done.

[commit-bot: I haz the power, 2015-03-17 18:14:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1017613002/40001

[kelvinp, 2015-03-17 18:20:58]

The CQ bit was unchecked by kelvinp@chromium.org

[John Williams, 2015-03-17 19:22:37]

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/base/js...
File remoting/webapp/base/js/base_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/base/js...
remoting/webapp/base/js/base_unittest.js:153: }).catch(function() {
Nit: If you start one line with a dot, you should probably start all of them in
the same expression with a dot.  In this case I would usually put
".then(function() {" on line 150, but I don't have any style I'm married to for
promises.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/base/js...
remoting/webapp/base/js/base_unittest.js:177: QUnit.ok(false, 'The reject
handler should not be invoked.');
I don't think the 2nd argument to 'then' is necessary here.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/base/js...
File remoting/webapp/base/js/ipc_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/base/js...
remoting/webapp/base/js/ipc_unittest.js:47: });
Nit: put the closing '}' in the same comlumn as the start of the 'function'
keyword.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/apps_v2_migration_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/apps_v2_migration_unittest.js:96:
QUnit.equal(result.fullName, 'v1userName');
Not part of your change, but there's a mismatch here between 'name' and
'fullName'.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/xhr_unittest.js (left):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/xhr_unittest.js:14: module('xhr', {
A call to QUnit module is still needed here to set the module name.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
File remoting/webapp/crd/js/xmpp_connection_unittest.js (right):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/crd/js/...
remoting/webapp/crd/js/xmpp_connection_unittest.js:115: 'xmpp.example.com:123',
'testUsername@gmail.com', 'testToken');
Would it still work to put this call before the creation of the promise? I think
it would read much better.

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/js_prot...
File remoting/webapp/js_proto/qunit_proto.js (right):

https://codereview.chromium.org/1017613002/diff/40001/remoting/webapp/js_prot...
remoting/webapp/js_proto/qunit_proto.js:81: * @param {function(QUnit.Assert=)} f
Just {function(QUnit.Assert)}; using {function(QUnit.Assert=)} implies that f
may be called with or without an argument.  In this case JSCompiler is OK with
functions that take fewer arguments than the type says.

[kelvinp, 2015-03-17 21:24:22]

The CQ bit was checked by kelvinp@chromium.org

[kelvinp, 2015-03-17 21:24:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
jamiewalch@chromium.org
Link to the patchset: https://codereview.chromium.org/1017613002/#ps60001
(title: "Migrate to assert")

[commit-bot: I haz the power, 2015-03-17 21:26:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1017613002/60001

[commit-bot: I haz the power, 2015-03-17 22:27:46]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-03-17 22:28:24]

Patchset 4 (id:??) landed as
https://crrev.com/831563d9c72e009bdae47c174421da85faad3aa6
Cr-Commit-Position: refs/heads/master@{#320996}