Bug 179769

Summary: Import 2dcontext/the-canvas-state/ from wpt.
Product: WebKit Reporter: Ms2ger (he/him; ⌚ UTC+1/+2) <Ms2ger>
Component: Tools / TestsAssignee: Ms2ger (he/him; ⌚ UTC+1/+2) <Ms2ger>
Status: REOPENED ---    
Severity: Normal CC: ajuma, ap, commit-queue, ddkilzer, fred.wang, gsnedders, lforschler, mcatanzaro, simon.fraser, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=193042
https://bugs.webkit.org/show_bug.cgi?id=193579
Bug Depends on: 179813    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-16 08:08:34 PST
.
Comment 1 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-16 08:34:20 PST
Created attachment 327064 [details]
Patch
Comment 2 youenn fablet 2017-11-16 08:59:39 PST
Comment on attachment 327064 [details]
Patch

Nice!
Comment 3 WebKit Commit Bot 2017-11-16 09:30:08 PST
Comment on attachment 327064 [details]
Patch

Clearing flags on attachment: 327064

Committed r224921: <https://trac.webkit.org/changeset/224921>
Comment 4 WebKit Commit Bot 2017-11-16 09:30:09 PST
All reviewed patches have been landed.  Closing bug.
Comment 5 Radar WebKit Bug Importer 2017-11-16 09:31:29 PST
<rdar://problem/35590236>
Comment 6 Alexey Proskuryakov 2017-11-16 22:08:29 PST
We shouldn't be doing this. These tests are primarily useful when shared with other teams, so requiring a complicated harness to run them is unacceptable.
Comment 7 WebKit Commit Bot 2017-11-16 22:09:22 PST
Re-opened since this is blocked by bug 179813
Comment 8 youenn fablet 2017-11-16 22:15:46 PST
Reopening for further discussion.
Comment 9 Ms2ger (he/him; ⌚ UTC+1/+2) 2017-11-17 01:18:30 PST
I'm afraid I don't really understand your objection; could you elaborate?
Comment 10 youenn fablet 2017-11-17 09:21:07 PST
Given the discussion in webkit-dev, I think we can proceed with this patch:
- removed tests are clearly conformance tests and not regression tests
- added tests are slightly better than removed tests and will be kept up to date
Comment 11 Alexey Proskuryakov 2017-11-17 09:40:44 PST
> I'm afraid I don't really understand your objection; could you elaborate?

When there is a problem in graphics frameworks below WebKit, I can easily put the test in a zip archive, and ask the team working on the framework to debug what's going on. They can edit the test locally as they see fit to simplify it as part of their investigation.

Requiring a full WebKit checkout and complicated scripts that may or may not even work in their OS build will make getting fixes for such problems substantially trickier.
Comment 12 youenn fablet 2017-11-17 09:58:29 PST
Creating a zip archive is not straightforward either.
It still requires to get all the necessary resources available which might be tricky.
WPT tests are available at https://w3c-test.org.
Comment 13 Alexey Proskuryakov 2017-11-17 10:27:54 PST
One cannot edit the w3c-test.org copy, which is important for debugging.
Comment 14 youenn fablet 2017-11-21 08:37:12 PST
(In reply to Alexey Proskuryakov from comment #13)
> One cannot edit the w3c-test.org copy, which is important for debugging.

Is a webarchive somehow hackable?
If so, it should be fairly easy and less error-prone than a zip archive to create a webarchive from an existing webpage, send that webarchive and edit it.
Comment 15 youenn fablet 2017-11-21 08:37:39 PST
Given the discussion in webkit-dev, we should probably reland this patch.
Please let me know if there is any objection.
Comment 16 Frédéric Wang (:fredw) 2017-11-24 06:31:57 PST
(In reply to youenn fablet from comment #15)
> Given the discussion in webkit-dev, we should probably reland this patch.
> Please let me know if there is any objection.

@Youenn:

I'm not sure which part of the conversation makes you think we should, but as I understand the situation is stalled until we improve runtime performance of WPT tests on WebKit.

From my point of view, sharing the tests with the rest of the web platform community (to keep it up-to-date, being able to compare results, etc) is more important than making it convenient for Apple to work internally on proprietary code outside WebKit. I wonder, if maybe we should still import the new tests and let each port maintainers decide which version of the tests they want to skip in TestExpectations?
Comment 17 youenn fablet 2017-11-24 08:32:23 PST
> @Youenn:
> 
> I'm not sure which part of the conversation makes you think we should, but
> as I understand the situation is stalled until we improve runtime
> performance of WPT tests on WebKit.

From webkit-dev, it seems fine to continue importing WPT tests, including these 2d tests.

The main discussion point is whether we should remove the corresponding WebKit tests.
Since the existing WebKit 2dcontext tests are not regression tests but conformance tests, I think it is fine to remove them as well.
Comment 18 Alexey Proskuryakov 2017-11-27 11:50:05 PST
> Is a webarchive somehow hackable?

It's not. Definitely not within easy reach of someone who works on lower level frameworks.

> From webkit-dev, it seems fine to continue importing WPT tests, including these 2d tests.

Let's give everyone who just got back from the Thanksgiving holiday (including myself) catch up on the discussion.

My feeling is that test stability and performance is more important for WebKit right now than adding more tests. This is obviously a very general statement, we can have lots of special cases.
Comment 19 Frédéric Wang (:fredw) 2019-01-21 06:45:56 PST
@David @Simon: Instead of having 3 separate bugs for this issue, we should probably merge all of them into one. Can we mark bug 193579 and bug 193042 as duplicate of this one?