Bug 233241

Summary: Convert error to string before passing to testFailed() in FileSystemAccess layout tests
Product: WebKit Reporter: Sihui Liu <sihui_liu>
Component: Tools / TestsAssignee: Sihui Liu <sihui_liu>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Sihui Liu 2021-11-16 23:04:13 PST
...
Comment 1 Sihui Liu 2021-11-16 23:10:24 PST
Created attachment 444483 [details]
Patch
Comment 2 youenn fablet 2021-11-18 05:41:00 PST
Comment on attachment 444483 [details]
Patch

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

> LayoutTests/ChangeLog:8
> +        testFailed() in js-test.js expects string, but finishTest() may receive Error object as parameter.

Proposed approach is fine, but why not updating testFailed to pretty print Error itself?

> LayoutTests/storage/filesystemaccess/resources/shared.js:8
> +        }

We usually do not need { } for one liner if.
Comment 3 Radar WebKit Bug Importer 2021-11-23 23:05:22 PST
<rdar://problem/85718910>
Comment 4 Sihui Liu 2021-11-28 11:42:20 PST
Comment on attachment 444483 [details]
Patch

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

>> LayoutTests/ChangeLog:8
>> +        testFailed() in js-test.js expects string, but finishTest() may receive Error object as parameter.
> 
> Proposed approach is fine, but why not updating testFailed to pretty print Error itself?

testFailed() has always received string as parameter, and it's symmetric to testPassed(), so I didn't change it in js-test.js. 
But it may be worthwhile to make the change if we see other tests have the same issue.

>> LayoutTests/storage/filesystemaccess/resources/shared.js:8
>> +        }
> 
> We usually do not need { } for one liner if.

Will remove.
Comment 5 Sihui Liu 2021-11-28 11:42:48 PST
Created attachment 445239 [details]
Patch for landing
Comment 6 EWS 2021-11-28 12:34:42 PST
Committed r286198 (244580@main): <https://commits.webkit.org/244580@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445239 [details].