Bug 231645

Summary: add test for bug 231353
Product: WebKit Reporter: Cameron McCormack (:heycam) <heycam>
Component: WebRTCAssignee: Cameron McCormack (:heycam) <heycam>
Status: NEW ---    
Severity: Normal CC: eric.carlson, ews-watchlist, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 229025, 231353    
Bug Blocks:    
Attachments:
Description Flags
Patch
youennf: review+, ews-feeder: commit-queue-
Patch for landing
ews-feeder: commit-queue-
Patch for EWS
ews-feeder: commit-queue-
Patch for EWS ews-feeder: commit-queue-

Description Cameron McCormack (:heycam) 2021-10-12 16:39:03 PDT
.
Comment 1 Radar WebKit Bug Importer 2021-10-12 16:40:04 PDT
<rdar://problem/84172363>
Comment 2 Cameron McCormack (:heycam) 2021-10-12 16:41:37 PDT
Created attachment 441018 [details]
Patch
Comment 3 Cameron McCormack (:heycam) 2021-10-12 16:41:55 PDT
(That'll fail EWS since the dependencies haven't landed yet.)
Comment 4 youenn fablet 2021-10-13 11:06:23 PDT
Comment on attachment 441018 [details]
Patch

r=me once bots are happy

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

> LayoutTests/webrtc/captureStream-color-space.html:10
> +    let srcContext = src.getContext("2d");

Could use const I guess here and below.

> LayoutTests/webrtc/captureStream-color-space.html:28
> +    assert_array_approx_equals(actual, expected, 2, `actual = ${actual}, expected = ${expected}`);

If this fails, we probably never hit t.done().
Maybe 2 is too small.

> LayoutTests/webrtc/captureStream-color-space.html:32
> +var t = async_test(function(t) {

Could use async_test(async function...

> LayoutTests/webrtc/captureStream-color-space.html:45
> +    local.createOffer().then(function(desc) {

You could write it with await in a more readable way
Comment 5 Cameron McCormack (:heycam) 2021-10-13 21:11:04 PDT
(In reply to youenn fablet from comment #4)
> > LayoutTests/webrtc/captureStream-color-space.html:28
> > +    assert_array_approx_equals(actual, expected, 2, `actual = ${actual}, expected = ${expected}`);
> 
> If this fails, we probably never hit t.done().

I think the test harness considers a test done when it catches an exception, and we are in a step_func() here.

> Maybe 2 is too small.

I want it to be just small enough that it passes.  We'll see if EWS is OK with it.
Comment 6 Cameron McCormack (:heycam) 2021-10-19 01:42:19 PDT
Created attachment 441701 [details]
Patch for landing
Comment 7 Cameron McCormack (:heycam) 2021-10-22 18:22:41 PDT
Created attachment 442238 [details]
Patch for EWS
Comment 8 Cameron McCormack (:heycam) 2021-10-22 23:26:13 PDT
Created attachment 442254 [details]
Patch for EWS