Bug 90937 – Improve BiquadFilterNode test case

• Component: Web Audio
• Status: RESOLVED FIXED

[Li Yin]

Improve BiquadFilterNode test to cover constant value and number of inputs and outputs.

[Li Yin]

Created attachment 151592 [details]
Patch

[Raymond Toy]

Comment on attachment 151592 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=151592&action=review

Except for the couple of nits, this looks fine.

> LayoutTests/webaudio/biquadfilternode-basic.html:31
> +    shouldBeTrue("filter.numberOfOutputs === 1");

I think it reads better if the text said something like

"Number of inputs to Biquad filter = 1"
"Number of outputs of Biquad filter = 1"

> LayoutTests/webaudio/biquadfilternode-basic.html:38
> +    var typeArray = [filter.LOWPASS,

Rename typeArray to filterTypeArray?

[Li Yin]

Created attachment 153639 [details]
Patch

[Li Yin]

(In reply to comment #2)
> (From update of attachment 151592 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=151592&action=review
> 
> Except for the couple of nits, this looks fine.
> 
> > LayoutTests/webaudio/biquadfilternode-basic.html:31
> > +    shouldBeTrue("filter.numberOfOutputs === 1");
> 
> I think it reads better if the text said something like
> 
> "Number of inputs to Biquad filter = 1"
> "Number of outputs of Biquad filter = 1"
> 

Most of other models like using shouleBe(), such as FileAPI, it looks more simple, and also decreases the code lines.
Anyway, fixed.

> > LayoutTests/webaudio/biquadfilternode-basic.html:38
> > +    var typeArray = [filter.LOWPASS,
> 
> Rename typeArray to filterTypeArray?

Done.

Thanks for your comments.

[Kentaro Hara]

Comment on attachment 153639 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=153639&action=review

LGTM

> LayoutTests/webaudio/biquadfilternode-basic.html:-38
> -    // FIXME: use last defined filter constant (ALLPASS) instead of hard-coded 7 once it's defined in the IDL.

Let's add the link to the spec (http://www.w3.org/TR/webaudio/#BiquadFilterNode) to ChangeLog.

[Li Yin]

Created attachment 153714 [details]
Patch

[Kentaro Hara]

Comment on attachment 153714 [details]
Patch

LGTM. rtoy@ is LGTMing too. Let me r+ it.

[WebKit Review Bot]

Comment on attachment 153714 [details]
Patch

Clearing flags on attachment: 153714

Committed r123306: <http://trac.webkit.org/changeset/123306>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.