Modifying audio-testing.js to support testharness.js.

Modifying audio-testing.js to support testharness.js.

This CL is to support the new testharness.js within audio-testing.js,
but also keep the backward compatibility for js-test.js.

The summary on changes:

* Should() utility: _testPassed, _testFailed, and _assert now call
  assert_true() method from testharness.js when it's available. The
  success or error messages are printed out properly.

* Audit task runner: now it initializes and finalizes the test suite
  for testharness.js. See the unit test included in the CL to see how
  it works.

TEST=webaudio/unit-test-audio-testing.html
BUG=634359
Committed: https://crrev.com/f908584a623a0983b8eae54a2ca7590585922301
Cr-Commit-Position: refs/heads/master@{#409948}

[hongchan, 2016-08-04 17:44:01]

Patchset #1 (id:1) has been deleted

[hongchan, 2016-08-04 17:44:50]

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

[hongchan, 2016-08-04 17:44:51]

PTAL.

[Raymond Toy, 2016-08-04 18:07:08]

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js (right):

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:329:
window.jsTestIsAsync = true;
Are you sure we always want to set this?  Thus far, I thought each test file set
this appropriately.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:344:
finishJSTest();
Won't this break existing tests?  I've always been calling finishJSTest in the
last task.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:479:
assert_true(true);
This is different from what you showed me this morning.  Is the right stuff
still produced?

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:492:
assert_true(false, that.desc + ' ' + msg + '.');
Why is this different from line 479?

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:516:
assert_true(false, reason + ' (' + value + ')');
Hard to tell, but why is this different from just using failureMessage?

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html
(right):

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html:8:
testharness.js -->
Is this because audio-testing.js checks for testharness right away?  Wouldn't it
be better to check for testharness when we create the Audit object in
Audit.createTaskRunner()?

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html:42:
audit.runTasks();
This is a breaking change.  We used to have to have a final task that called
finishJSTest(). It's not clear to me that audit should always automatically do
this.

[hongchan, 2016-08-04 18:31:37]

This is the result from the sample test here. I uncommented failure cases for
the demonstration:
---

This is a testharness.js-based test.
PASS Zero is equal to 0. 
PASS One is not equal to 0. 
PASS Expected SNR (110) is greater than or equal to 100. 
PASS Maximum error value (0.000001) is less than or equal to 0.00001. 
FAIL max error assert_true: max error (0.000001) is not less than or equal to
-1. expected true got false
PASS One point double zero one is 1 within a relative error of 0.1. 
FAIL Two assert_true: Two is not 1 within a relative error of 0.1: 2 with
relative error 1. expected true got false
PASS [2, 2, 2] contains only the constant 2. 
PASS [1, 2, 3] is identical to the array [1,2,3]. 
PASS My array equals [0.1,0.2] with an element-wise tolerance of 0.02. 
PASS My random array contains all the expected values in the correct order:
[1,3,2]. 
Harness: the test ran to completion.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js (right):

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:329:
window.jsTestIsAsync = true;
On 2016/08/04 18:07:08, Raymond Toy wrote:
> Are you sure we always want to set this?  Thus far, I thought each test file
set
> this appropriately.

If the value is already set, this doesn't do anything. If you're using Audit,
this is a must. Or I can simply not care this case at all.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:344:
finishJSTest();
On 2016/08/04 18:07:08, Raymond Toy wrote:
> Won't this break existing tests?  I've always been calling finishJSTest in the
> last task.

I've tested this against our existing test suites. It works fine, but I can
remove this if this bothers you.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:479:
assert_true(true);
On 2016/08/04 18:07:08, Raymond Toy wrote:
> This is different from what you showed me this morning.  Is the right stuff
> still produced?

When the assert passes, we should print out the message as a description.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:492:
assert_true(false, that.desc + ' ' + msg + '.');
On 2016/08/04 18:07:08, Raymond Toy wrote:
> Why is this different from line 479?

On the other hand, this description message gets printed in this assert when it
fails. Our `msg` usually starts `is..` so it makes sense to have a subject
before the verb. `.desc` is usually the subject of the sentence.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/audio-testing.js:516:
assert_true(false, reason + ' (' + value + ')');
On 2016/08/04 18:07:08, Raymond Toy wrote:
> Hard to tell, but why is this different from just using failureMessage?

When assert_true fails it prints out the message starts with `assert_true`. I
did not want to have duplications in the error message. I might have to make up
some examples to see this happens.

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html
(right):

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html:8:
testharness.js -->
On 2016/08/04 18:07:08, Raymond Toy wrote:
> Is this because audio-testing.js checks for testharness right away?  Wouldn't
it
> be better to check for testharness when we create the Audit object in
> Audit.createTaskRunner()?

Should and Audit both rely on testharness. I think this is okay pattern -
otherwise we have to check for testharness in every instance of Should().

https://codereview.chromium.org/2212023002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/unit-test-audio-testing.html:42:
audit.runTasks();
On 2016/08/04 18:07:08, Raymond Toy wrote:
> This is a breaking change.  We used to have to have a final task that called
> finishJSTest(). It's not clear to me that audit should always automatically do
> this.

Not sure why this is a breaking change. It does not break anything. It just does
one more step when the task runner is finished in the case of testharness.

[hongchan, 2016-08-04 21:13:21]

The CQ bit was checked by hongchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 21:13:39]

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

[Raymond Toy, 2016-08-04 21:27:02]

lgtm

[commit-bot: I haz the power, 2016-08-04 22:28:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-04 22:28:50]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[hongchan, 2016-08-04 22:49:08]

The CQ bit was checked by hongchan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-04 22:49:28]

Dry run: 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-08-04 23:57:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-04 23:57:39]

Dry run: This issue passed the CQ dry run.

[hongchan, 2016-08-05 00:27:25]

The CQ bit was checked by hongchan@chromium.org

[hongchan, 2016-08-05 00:27:26]

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/2212023002/#ps80001
(title: "Changing js-test.js loading order")

[commit-bot: I haz the power, 2016-08-05 00:27:50]

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-08-05 00:33:33]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-08-05 00:35:55]

Description was changed from

==========
Modifying audio-testing.js to support testharness.js.

This CL is to support the new testharness.js within audio-testing.js,
but also keep the backward compatibility for js-test.js.

The summary on changes:

* Should() utility: _testPassed, _testFailed, and _assert now call
  assert_true() method from testharness.js when it's available. The
  success or error messages are printed out properly.

* Audit task runner: now it initializes and finalizes the test suite
  for testharness.js. See the unit test included in the CL to see how
  it works.

TEST=webaudio/unit-test-audio-testing.html
BUG=634359
==========

to

==========
Modifying audio-testing.js to support testharness.js.

This CL is to support the new testharness.js within audio-testing.js,
but also keep the backward compatibility for js-test.js.

The summary on changes:

* Should() utility: _testPassed, _testFailed, and _assert now call
  assert_true() method from testharness.js when it's available. The
  success or error messages are printed out properly.

* Audit task runner: now it initializes and finalizes the test suite
  for testharness.js. See the unit test included in the CL to see how
  it works.

TEST=webaudio/unit-test-audio-testing.html
BUG=634359

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

[commit-bot: I haz the power, 2016-08-05 00:35:56]

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