Convert note-grain-on tests to testharness

Convert note-grain-on tests to testharness

Manually convert test to use testharness

BUG=676394
TEST=note-grain-on-play.html, note-grain-on-timing.html

Review-Url: https://codereview.chromium.org/2594183003
Cr-Commit-Position: refs/heads/master@{#445225}
Committed: https://chromium.googlesource.com/chromium/src/+/af202730923e6c2d878e2fe9fd5b5a4c4bfd6a80

[Raymond Toy, 2017-01-04 22:53:26]

Description was changed from

==========
Convert AudioBufferSource tests to testharness

Manually convert test to use testharness

BUG=676394
TEST=note-grain-on-play.html, note-grain-on-timing.html
==========

to

==========
Convert note-grain-on tests to testharness

Manually convert test to use testharness

BUG=676394
TEST=note-grain-on-play.html, note-grain-on-timing.html
==========

[Raymond Toy, 2017-01-13 19:28:47]

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

[Raymond Toy, 2017-01-13 19:28:47]

PTAL.

The output is a lot more verbose now.  Something like

This is a testharness.js-based test.
PASS # AUDIT TASK RUNNER STARTED. 
PASS > [note-grain-on-play] Test noteGrainOn offset rendering 
PASS   Found all grain starts and ends is true. 
PASS   Number of start frames is equal to 100. 
PASS   Number of end frames is equal to 100. 
PASS   Pulse 0 boundary is identical to the array [0,953]. 
PASS   Pulse 1 boundary is identical to the array [1174,2127]. 
PASS   Pulse 2 boundary is identical to the array [2347,3300]. 
PASS   Pulse 3 boundary is identical to the array [3521,4474]. 
...
PASS   Number of grains that started at the correct time is equal to 100. 
PASS   Number of grains that ended at the correct time is equal to 100. 
PASS   Pulse 0 contained the expected data is true. 
PASS   Pulse 1 contained the expected data is true. 
...
PASS   Pulse 99 contained the expected data is true. 
PASS   Number of grains that did not contain the expected data is equal to 0. 
PASS < [note-grain-on-play] All assertions passed. (total 206 assertions) 
PASS # AUDIT TASK RUNNER FINISHED: 1 tasks ran successfully. 
Harness: the test ran to completion.

[hongchan, 2017-01-18 18:19:04]

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html
(right):

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:25:
success = success && verifyStartAndEndFrames(startEndFrames, should);
Hmm. Why do we need 'success'? The new Audit task keeps this inside of a Task
object. Can we utilize it? See the comment at the end.

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js
(right):

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:123:
&& success;
Please see the comment at the end.

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:128:
&& success;
Ditto.

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:161: 
Remove this empty line.

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:173:
return success;
Just do all the |should| assertions, then get the |success| from the task
object. We really don't need to keep track of |success|.

[Raymond Toy, 2017-01-18 19:33:30]

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html
(right):

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:25:
success = success && verifyStartAndEndFrames(startEndFrames, should);
On 2017/01/18 18:19:03, hongchan (OOO 1.17-1.19) wrote:
> Hmm. Why do we need 'success'? The new Audit task keeps this inside of a Task
> object. Can we utilize it? See the comment at the end.

Not needed anymore.  It was previously used to print a summary message, but
audit takes care of that for us (in a different way).

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js
(right):

https://codereview.chromium.org/2594183003/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:173:
return success;
On 2017/01/18 18:19:04, hongchan (OOO 1.17-1.19) wrote:
> Just do all the |should| assertions, then get the |success| from the task
> object. We really don't need to keep track of |success|.

I don't think we even need to return success anymore.  Nothing really uses the
result now.

[hongchan, 2017-01-20 17:46:10]

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:61:
renderedData = buffer.getChannelData(0);
This is bad. We need |let| here. I know it happened in the past, but let's fix
it.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:81:
should(result.verified, "Pulse " + k + " contained the expected data")
Should be wrapped.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:111:
checkResult(audioBuffer, should);
This does not return a promise and it's synchronous. We can call task.done()
right after this and don't need to chain it with .then().

WDYT?

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:27:
context = new OfflineAudioContext(2, sampleRate * renderTime, sampleRate);
Wrap at 80.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:29:
squarePulseBuffer = createSignalBuffer(context, function (k) { return 1 });
Wrap at 80.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:35:
checkResult(audioBuffer, should);
We can call task.done() right after this.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:1:
var sampleRate = 44100.0;
var => let (all instances in this file)

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:122:
should(startFrames.length, "Number of start frames").beEqualTo(numberOfTests);
Wrap at 80.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:131:
var expectedEnd = extraFramesHRTF + expectedStart + grainLengthInSampleFrames(k
* grainOffsetStep, duration, sampleRate);
Let's wrap at 80.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:149:
"Number of grains out of " + numberOfTests + "that started at the wrong time")
This line needs to be wrapped.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:159:
"Number of grains out of " + numberOfTests + " that ended at the wrong time")
Ditto.

[Raymond Toy, 2017-01-20 19:37:11]

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:61:
renderedData = buffer.getChannelData(0);
On 2017/01/20 17:46:09, hongchan wrote:
> This is bad. We need |let| here. I know it happened in the past, but let's fix
> it.

renderedData is defined in note-grain-on-testing.js.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:81:
should(result.verified, "Pulse " + k + " contained the expected data")
On 2017/01/20 17:46:09, hongchan wrote:
> Should be wrapped.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-play.html:111:
checkResult(audioBuffer, should);
On 2017/01/20 17:46:09, hongchan wrote:
> This does not return a promise and it's synchronous. We can call task.done()
> right after this and don't need to chain it with .then().
> 
> WDYT?

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:27:
context = new OfflineAudioContext(2, sampleRate * renderTime, sampleRate);
On 2017/01/20 17:46:09, hongchan wrote:
> Wrap at 80.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:29:
squarePulseBuffer = createSignalBuffer(context, function (k) { return 1 });
On 2017/01/20 17:46:09, hongchan wrote:
> Wrap at 80.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/AudioBufferSource/note-grain-on-timing.html:35:
checkResult(audioBuffer, should);
On 2017/01/20 17:46:09, hongchan wrote:
> We can call task.done() right after this.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js
(right):

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:1:
var sampleRate = 44100.0;
On 2017/01/20 17:46:10, hongchan wrote:
> var => let (all instances in this file)

I was avoiding that to make the diff smaller, but ok.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:122:
should(startFrames.length, "Number of start frames").beEqualTo(numberOfTests);
On 2017/01/20 17:46:10, hongchan wrote:
> Wrap at 80.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:131:
var expectedEnd = extraFramesHRTF + expectedStart + grainLengthInSampleFrames(k
* grainOffsetStep, duration, sampleRate);
On 2017/01/20 17:46:09, hongchan wrote:
> Let's wrap at 80.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:149:
"Number of grains out of " + numberOfTests + "that started at the wrong time")
On 2017/01/20 17:46:10, hongchan wrote:
> This line needs to be wrapped.

Done.

https://codereview.chromium.org/2594183003/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/webaudio/resources/note-grain-on-testing.js:159:
"Number of grains out of " + numberOfTests + " that ended at the wrong time")
On 2017/01/20 17:46:10, hongchan wrote:
> Ditto.

Done.

[hongchan, 2017-01-20 23:21:27]

lgtm

[Raymond Toy, 2017-01-20 23:24:49]

The CQ bit was checked by rtoy@chromium.org

[commit-bot: I haz the power, 2017-01-20 23:25:25]

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-01-21 00:26:25]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484954689099390,
"parent_rev": "3ae4ce708f660411f8a26da7a4382c6842c75346", "commit_rev":
"af202730923e6c2d878e2fe9fd5b5a4c4bfd6a80"}

[commit-bot: I haz the power, 2017-01-21 00:26:57]

Description was changed from

==========
Convert note-grain-on tests to testharness

Manually convert test to use testharness

BUG=676394
TEST=note-grain-on-play.html, note-grain-on-timing.html
==========

to

==========
Convert note-grain-on tests to testharness

Manually convert test to use testharness

BUG=676394
TEST=note-grain-on-play.html, note-grain-on-timing.html

Review-Url: https://codereview.chromium.org/2594183003
Cr-Commit-Position: refs/heads/master@{#445225}
Committed:
https://chromium.googlesource.com/chromium/src/+/af202730923e6c2d878e2fe9fd5b...
==========

[commit-bot: I haz the power, 2017-01-21 00:26:58]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/af202730923e6c2d878e2fe9fd5b...