Closed Bug 878875 Opened 7 years ago Closed 7 years ago

Add tests for PannerNode

Categories

(Core :: Web Audio, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla24

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(2 files)

Ehsan said in bug 878765:

> Blink has a number of tests for the panner node.  Can you please look into porting them 
> over, or at least writing some of our own?  Thanks!

We only have tests for the DOM part, we need to actually look at the audio the node outputs in various spatial configuration.
Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.
Component: Video/Audio → Web Audio
This imports the files from the Blink repo.
Attachment #758624 - Flags: review?(ehsan)
This wires up the tests to the build system, make the glue between LayoutTest
functions and SimpleTest functions, and make the only two adjustments needed to
make the test pass.

Surprisingly easy.
Attachment #758627 - Flags: review?(ehsan)
Comment on attachment 758624 [details] [diff] [review]
Import PannerNode tests from Blink. r=

Review of attachment 758624 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/webaudio/test/blink/README
@@ +3,5 @@
> +
> +The process of borrowing tests from Blink is as follows:
> +
> +* Import the pristine file from the Blink repo, noting the revision in the
> +  commit message.

You broke your own rule.  ;-)
Attachment #758624 - Flags: review?(ehsan) → review+
Comment on attachment 758627 [details] [diff] [review]
Port Blink's LayoutTest for PannerNode to mochitest-plain. r=

Review of attachment 758627 [details] [diff] [review]:
-----------------------------------------------------------------

Sweet!

::: content/media/webaudio/test/blink/panner-model-testing.js
@@ +108,5 @@
>      }
>  }
>  
> +function checkResult(e) {
> +    renderedBuffer = e.renderedBuffer;

Technically this should not be needed, but it's ok if you want to keep this.  :-)
Attachment #758627 - Flags: review?(ehsan) → review+
(In reply to :Ehsan Akhgari (needinfo? me!) from comment #4)
> Comment on attachment 758624 [details] [diff] [review]
> Import PannerNode tests from Blink. r=
> 
> Review of attachment 758624 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/webaudio/test/blink/README
> @@ +3,5 @@
> > +
> > +The process of borrowing tests from Blink is as follows:
> > +
> > +* Import the pristine file from the Blink repo, noting the revision in the
> > +  commit message.
> 
> You broke your own rule.  ;-)

The revision is in the second line of the commit message.
(In reply to Paul Adenot (:padenot) from comment #6)
> The revision is in the second line of the commit message.

Ah, you're right.  Splinter hides it pretty well.

Still, can you please use the SVN revision number instead?  There has been talks of them rewriting the history of their git repo...
Ha, was not aware of that, I'll do that before landing.
https://hg.mozilla.org/mozilla-central/rev/b61e0a52d29c
https://hg.mozilla.org/mozilla-central/rev/fcdcf78fa4e3
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.