Bug 170796 – Add some more WebRTC tests

• Component: Tools / Tests
• Status: RESOLVED FIXED

[youenn fablet]

We should add multitrack and AV+data channel tests

[youenn fablet]

Created attachment 306951 [details]
Patch

[Eric Carlson]

Comment on attachment 306951 [details]
Patch

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

> LayoutTests/webrtc/multi-video.html:25
> +video1 = document.getElementById("video1");
> +video2 = document.getElementById("video2");
> +video3 = document.getElementById("video3");
> +video4 = document.getElementById("video4");
> +video5 = document.getElementById("video5");
> +video6 = document.getElementById("video6");
> +canvas = document.getElementById("canvas");

Nit: technically none of these are necessary since the elements all have ids with the same names.

> LayoutTests/webrtc/multi-video.html:50
> +    assert_true(data[index] < 100, "test 1 for " + id);
> +    assert_true(data[index + 1] < 100, "test 2 for " + id);
> +    assert_true(data[index + 2] < 100, "test 3 for " + id);
> +
> +    index = 80;
> +    assert_true(data[index] > 200, "test 4 for " + id);
> +    assert_true(data[index + 1] > 200, "test 5 for " + id);
> +    assert_true(data[index + 2] > 200, "test 6 for " + id);
> +
> +    index += 80;
> +    assert_true(data[index] > 200, "test 7 for " + id);
> +    assert_true(data[index + 1] > 200, "test 8 for " + id);
> +    assert_true(data[index + 2] < 100, "test 9 for " + id);

Nit: you could use a template literals for these, eg. `test 1 for ${id}`, etc

[youenn fablet]

Created attachment 306990 [details]
Patch for landing

[youenn fablet]

Thanks for the review.

(In reply to Eric Carlson from comment #2)
> Comment on attachment 306951 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=306951&action=review
> 
> > LayoutTests/webrtc/multi-video.html:25
> > +video1 = document.getElementById("video1");
> > +video2 = document.getElementById("video2");
> > +video3 = document.getElementById("video3");
> > +video4 = document.getElementById("video4");
> > +video5 = document.getElementById("video5");
> > +video6 = document.getElementById("video6");
> > +canvas = document.getElementById("canvas");
> 
> Nit: technically none of these are necessary since the elements all have ids
> with the same names.

Done

> > LayoutTests/webrtc/multi-video.html:50
> > +    assert_true(data[index] < 100, "test 1 for " + id);
> > +    assert_true(data[index + 1] < 100, "test 2 for " + id);
> > +    assert_true(data[index + 2] < 100, "test 3 for " + id);
> > +
> > +    index = 80;
> > +    assert_true(data[index] > 200, "test 4 for " + id);
> > +    assert_true(data[index + 1] > 200, "test 5 for " + id);
> > +    assert_true(data[index + 2] > 200, "test 6 for " + id);
> > +
> > +    index += 80;
> > +    assert_true(data[index] > 200, "test 7 for " + id);
> > +    assert_true(data[index + 1] > 200, "test 8 for " + id);
> > +    assert_true(data[index + 2] < 100, "test 9 for " + id);
> 
> Nit: you could use a template literals for these, eg. `test 1 for ${id}`, etc

Will do in future patches!

[WebKit Commit Bot]

Comment on attachment 306990 [details]
Patch for landing

Clearing flags on attachment: 306990

Committed r215323: <http://trac.webkit.org/changeset/215323>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.