Convert constructor/analyser test to new Audit

Convert constructor/analyser test to new Audit

Manually convert analyser.html tests to use new Audit.

Also refactored the tests a bit to move common things to a new
function.  The new script new-audionodeoptions.js is
audionodeoptions.js converted to use new Audit.  This allows us to
update the files one-by-one.  new-audionodeoptions.js will be renamed
once all the files have been converted.

BUG=704967
TEST=constructor/analyser.html

Review-Url: https://codereview.chromium.org/2831953003
Cr-Commit-Position: refs/heads/master@{#466349}
Committed: https://chromium.googlesource.com/chromium/src/+/6e1f3c35d3d0d34ea488a59b3990c5770a701e8e

[Raymond Toy, 2017-04-20 17:00:13]

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

[Raymond Toy, 2017-04-20 17:00:13]

PTAL

[hongchan, 2017-04-20 19:04:19]

I went through the JS file, but it seems there are few spots could use more
refactoring/rearranging. Perhaps we should discuss it offline first?

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js
(right):

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:3:
// channelInterpretation.
Please mention that this method requires |Audit| and |should|.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:4:
function testAudioNodeOptions(should, context, nodeName, nodeOptions) {
Can we name this |expectedNodeOptions| and remove all 'expected' prefix from
other variables?

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:7:
var node;
I see the mixed usage of |var| and |let|. Let's be consistent.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:11:
var testCount = 17;
testChannelCount

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:13:
testCount = nodeOptions.expectedChannelCount.value;
So |expectedChannelCount| is an object, not a value? That seems awkward.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:58:
if (nodeOptions && nodeOptions.expectedChannelCountMode) {
|nodeOptions| cannot be invalid at this point.

[Raymond Toy, 2017-04-20 19:19:39]

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js
(right):

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:3:
// channelInterpretation.
On 2017/04/20 19:04:19, hongchan wrote:
> Please mention that this method requires |Audit| and |should|.

Here and everywhere, I assume.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:13:
testCount = nodeOptions.expectedChannelCount.value;
On 2017/04/20 19:04:19, hongchan wrote:
> So |expectedChannelCount| is an object, not a value? That seems awkward.

Yes. Analyser doesn't show this usage, but, say, Convolver does.

This is needed because, currently, sometimes the channel count is fixed, so we
should throw an error on that, and the error type that is thrown varies.

[Raymond Toy, 2017-04-20 21:00:00]

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js
(right):

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:4:
function testAudioNodeOptions(should, context, nodeName, nodeOptions) {
On 2017/04/20 19:04:19, hongchan wrote:
> Can we name this |expectedNodeOptions| and remove all 'expected' prefix from
> other variables?

Done.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:7:
var node;
On 2017/04/20 19:04:19, hongchan wrote:
> I see the mixed usage of |var| and |let|. Let's be consistent.

Done.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:11:
var testCount = 17;
On 2017/04/20 19:04:19, hongchan wrote:
> testChannelCount

Done.

https://codereview.chromium.org/2831953003/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/webaudio/constructor/new-audionodeoptions.js:58:
if (nodeOptions && nodeOptions.expectedChannelCountMode) {
On 2017/04/20 19:04:19, hongchan wrote:
> |nodeOptions| cannot be invalid at this point.

Done.

[hongchan, 2017-04-20 22:18:29]

lgtm

We discussed offline and decided not to do the functional refactoring.

For restructuring/extending the Audit test framework, I filed an issue:
https://bugs.chromium.org/p/chromium/issues/detail?id=713898

[Raymond Toy, 2017-04-20 22:47:28]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-04-20 22:47:52]

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, 2017-04-20 22:50:50]

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

[commit-bot: I haz the power, 2017-04-20 22:50:51]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Raymond Toy, 2017-04-21 14:39:58]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-04-21 14:40:18]

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, 2017-04-21 15:49:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492785598251780,
"parent_rev": "91d68416a8808790a8d721a5ebe0accdafbcc066", "commit_rev":
"6e1f3c35d3d0d34ea488a59b3990c5770a701e8e"}

[commit-bot: I haz the power, 2017-04-21 15:49:20]

Description was changed from

==========
Convert constructor/analyser test to new Audit

Manually convert analyser.html tests to use new Audit.

Also refactored the tests a bit to move common things to a new
function.  The new script new-audionodeoptions.js is
audionodeoptions.js converted to use new Audit.  This allows us to
update the files one-by-one.  new-audionodeoptions.js will be renamed
once all the files have been converted.

BUG=704967
TEST=constructor/analyser.html
==========

to

==========
Convert constructor/analyser test to new Audit

Manually convert analyser.html tests to use new Audit.

Also refactored the tests a bit to move common things to a new
function.  The new script new-audionodeoptions.js is
audionodeoptions.js converted to use new Audit.  This allows us to
update the files one-by-one.  new-audionodeoptions.js will be renamed
once all the files have been converted.

BUG=704967
TEST=constructor/analyser.html

Review-Url: https://codereview.chromium.org/2831953003
Cr-Commit-Position: refs/heads/master@{#466349}
Committed:
https://chromium.googlesource.com/chromium/src/+/6e1f3c35d3d0d34ea488a59b3990...
==========

[commit-bot: I haz the power, 2017-04-21 15:49:21]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/6e1f3c35d3d0d34ea488a59b3990...