RESOLVED FIXED 170796
Add some more WebRTC tests
https://bugs.webkit.org/show_bug.cgi?id=170796
Summary Add some more WebRTC tests
youenn fablet
Reported 2017-04-12 16:45:49 PDT
We should add multitrack and AV+data channel tests
Attachments
Patch (8.91 KB, patch)
2017-04-12 16:47 PDT, youenn fablet
no flags
Patch for landing (8.57 KB, patch)
2017-04-13 09:08 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2017-04-12 16:47:25 PDT
Eric Carlson
Comment 2 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
youenn fablet
Comment 3 2017-04-13 09:08:27 PDT
Created attachment 306990 [details] Patch for landing
youenn fablet
Comment 4 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!
WebKit Commit Bot
Comment 5 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>
WebKit Commit Bot
Comment 6 2017-04-13 09:54:37 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.