Bug 170796 - Add some more WebRTC tests
Summary: Add some more WebRTC tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-04-12 16:45 PDT by youenn fablet
Modified: 2017-04-13 09:54 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.91 KB, patch)
2017-04-12 16:47 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (8.57 KB, patch)
2017-04-13 09:08 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-04-12 16:45:49 PDT
We should add multitrack and AV+data channel tests
Comment 1 youenn fablet 2017-04-12 16:47:25 PDT
Created attachment 306951 [details]
Patch
Comment 2 Eric Carlson 2017-04-12 17:44:41 PDT
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
Comment 3 youenn fablet 2017-04-13 09:08:27 PDT
Created attachment 306990 [details]
Patch for landing
Comment 4 youenn fablet 2017-04-13 09:08:54 PDT
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!
Comment 5 WebKit Commit Bot 2017-04-13 09:54:35 PDT
Comment on attachment 306990 [details]
Patch for landing

Clearing flags on attachment: 306990

Committed r215323: <http://trac.webkit.org/changeset/215323>
Comment 6 WebKit Commit Bot 2017-04-13 09:54:37 PDT
All reviewed patches have been landed.  Closing bug.