Fixing audit.js: should().beCloseTo() and task.run('taskName')

Fixing audit.js: should().beCloseTo() and task.run('taskName')

This CL fixes two trivial bugs in audit.js:
The issue 672203: task.run('taskName')
The issue 672191: should().beCloseTo()

BUG=672203, 672191
Committed: https://crrev.com/f0426811873244e80feac8e0de79e1eb0633cb21
Cr-Commit-Position: refs/heads/master@{#437312}

[hongchan, 2016-12-07 23:58:08]

hongchan@chromium.org changed reviewers:
+ rtoy@chromium.org

[hongchan, 2016-12-07 23:58:08]

PTAL.

[Raymond Toy, 2016-12-08 00:06:02]

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/audit.js (right):

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:168: if (name ===
'numberOfErrors'
Why do you have to do this?

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:669: let absExpected
= this._expected ? Math.abs(this._expected) : 1;
Because we got confused here when looking at it, we should comment that
beCloseTo threshold is relative, except when expected is zero, in which case
it's absolute.

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html (right):

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html:52:
should(0).beCloseTo(0, { threshold: 0 });
Rather than modifying this test, I think you should add this as a new test,
preserving the old one.

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html:85:
audit.run('numerical', 'basic');
I'm ok with this for this CL, but we really need to have a test that audit.run()
runs all tests, and another test for audit.run() with a subset.

[Raymond Toy, 2016-12-08 00:06:26]

lgtm with nits

[hongchan, 2016-12-08 17:52:12]

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/audit.js (right):

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:168: if (name ===
'numberOfErrors'
On 2016/12/08 00:06:02, Raymond Toy wrote:
> Why do you have to do this?

Because we don't want to print out 'numberOfErrors' and etc in the logging. This
skips the loop for them, so it saves few iterations.

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audit.js:669: let absExpected
= this._expected ? Math.abs(this._expected) : 1;
On 2016/12/08 00:06:02, Raymond Toy wrote:
> Because we got confused here when looking at it, we should comment that
> beCloseTo threshold is relative, except when expected is zero, in which case
> it's absolute.

Done.

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html (right):

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html:52:
should(0).beCloseTo(0, { threshold: 0 });
On 2016/12/08 00:06:02, Raymond Toy wrote:
> Rather than modifying this test, I think you should add this as a new test,
> preserving the old one.

Done.

https://codereview.chromium.org/2561573006/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-tests/audit.html:85:
audit.run('numerical', 'basic');
On 2016/12/08 00:06:02, Raymond Toy wrote:
> I'm ok with this for this CL, but we really need to have a test that
audit.run()
> runs all tests, and another test for audit.run() with a subset.

Sure. I will add another test (for the failure case) for the run() with no
argument.

[hongchan, 2016-12-08 17:52:15]

The CQ bit was checked by hongchan@chromium.org

[hongchan, 2016-12-08 17:52:16]

The patchset sent to the CQ was uploaded after l-g-t-m from rtoy@chromium.org
Link to the patchset: https://codereview.chromium.org/2561573006/#ps80001
(title: "Addressing feedback after l-g-t-m")

[commit-bot: I haz the power, 2016-12-08 17:52:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 20:04:14]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481219535868320,
"parent_rev": "d98b96f48f94ead5039322f9212896d62ee9d784", "commit_rev":
"0d0d11bfda9fa9a3e0c305a4d44b0bb38b97ed55"}

[commit-bot: I haz the power, 2016-12-08 20:04:45]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-08 20:06:59]

Description was changed from

==========
Fixing audit.js: should().beCloseTo() and task.run('taskName')

This CL fixes two trivial bugs in audit.js:
The issue 672203: task.run('taskName')
The issue 672191: should().beCloseTo()

BUG=672203, 672191
==========

to

==========
Fixing audit.js: should().beCloseTo() and task.run('taskName')

This CL fixes two trivial bugs in audit.js:
The issue 672203: task.run('taskName')
The issue 672191: should().beCloseTo()

BUG=672203, 672191

Committed: https://crrev.com/f0426811873244e80feac8e0de79e1eb0633cb21
Cr-Commit-Position: refs/heads/master@{#437312}
==========

[commit-bot: I haz the power, 2016-12-08 20:07:00]

Patchset 5 (id:??) landed as
https://crrev.com/f0426811873244e80feac8e0de79e1eb0633cb21
Cr-Commit-Position: refs/heads/master@{#437312}