Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Philip Jägenstedt
On Thu, May 24, 2018, 21:40 youenn fablet  wrote:

>
>
>>> Ryosuke, correct me if I am wrong, I think you are pointing out the
>>> following rule:
>>> Changes to LayoutTests/imported/w3c/web-platform-tests tests should land
>>> first in WPT repository, then in WebKit repository.
>>>
>>
>> Oh, that is surprising.
>>
>> https://github.com/w3c/web-platform-tests/pull/10964 is a recent WebKit
>> export, and https://trac.webkit.org/changeset/231788/webkit did modify
>> the test in place. Do you mean that the WPT PR was merged first, or should
>> be in general? Chromium and Gecko do it in the other order, and I'd be
>> interested to understand the trade-offs of flipping the order.
>>
>
> Yep, WPT PR merged first, then WebKit patch landed.
> It makes sure that whenever we are reimporting tests, we are not loosing
> any WebKit specific change.
> Conflict resolution also happens at a time the patch is being committed in
> WebKit.
> So far, I encountered very few conflicts.
>

Interesting. What happens happens if the WebKit patch then fails to land in
WebKit, perhaps because some bot fails the test, a conflict, or anything
else? Is resolving that a manual affair?

When imports are automated and much more frequent, I take it the upside is
that you never need to reapply still-being-exported patches like we do in
Blink. But what about the above case, when the change hasn't been applied
in WebKit yet? Seems like the importer instead needs to *revert* the
in-flight change?

>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Ryosuke Niwa
On Thu, May 24, 2018 at 9:27 PM, Frédéric Wang  wrote:

> Thank you everybody for your replies. So summarizing:
>
> * WebKit devs should be able to choose between:
>   - submitting new WPT tests to the GitHub WPT repo first and then
> importing them to the WebKit repo after they are approved.
>   - OR submitting patches to WebKit Bugzilla first and then exporting
> them to the GitHub WPT repo.
>
> * In any case, you should never land a test into the WebKit repo before
> the corresponding test has successfully landed into the GitHub WPT repo.
>
> * We now have Tools/Scripts to import and export WPT tests. In
> particular the latter script ensures that the previous condition is
> satisfied.
>
> I believe a wiki page to explain the process for adding WPT tests to
> WebKit should be accessible from
> https://trac.webkit.org/wiki#LayoutTests so that with have a centralized
> documentation people can refer to.
>
> Maybe this could be the following one but it was last updated three
> years ago and does not mention the latest tools/agreements/work-in-
> progress:
> https://trac.webkit.org/wiki/WebKitW3CTesting


Yeah, we should probably update this wiki page.

In general, a whole lot of these layout test related wiki pages should be
updated & re-organized. Perhaps we should have a Wiki-updatethon at the
next contributor's meeting for that.

- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Frédéric Wang
On 25/05/2018 00:52, Ryosuke Niwa wrote:
>
> Yes, it's okay for a WebKit commit to merge the change which got
> merged into WPT but it's never okay to first commit the test into
> WebKit and then later upstream it to WPT...
If that's what you meant by "NEVER commit new tests into
LayoutTests/imported/w3c/web-platform-tests" then I agree it's very
sensible. On the other hand, if someone uploads a patch adding a new
test to LayoutTests/imported/w3c/web-platform-tests without the
corresponding GitHub PR I would expect that we only cq- this patch and
point that person to a link explaining clearly the WPT export process
and agreement.

-- 
Frédéric Wang - frederic-wang.fr

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Frédéric Wang
Thank you everybody for your replies. So summarizing:

* WebKit devs should be able to choose between:
  - submitting new WPT tests to the GitHub WPT repo first and then
importing them to the WebKit repo after they are approved.
  - OR submitting patches to WebKit Bugzilla first and then exporting
them to the GitHub WPT repo.

* In any case, you should never land a test into the WebKit repo before
the corresponding test has successfully landed into the GitHub WPT repo.

* We now have Tools/Scripts to import and export WPT tests. In
particular the latter script ensures that the previous condition is
satisfied.

I believe a wiki page to explain the process for adding WPT tests to
WebKit should be accessible from
https://trac.webkit.org/wiki#LayoutTests so that with have a centralized
documentation people can refer to.

Maybe this could be the following one but it was last updated three
years ago and does not mention the latest tools/agreements/work-in-progress:
https://trac.webkit.org/wiki/WebKitW3CTesting

-- 
Frédéric Wang - frederic-wang.fr

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Ryosuke Niwa
On Thu, May 24, 2018 at 3:22 AM, Philip Jägenstedt 
wrote:

> On Wed, May 23, 2018, 23:43 youenn fablet  wrote:
>
>> Le mer. 23 mai 2018 à 14:11, Frédéric Wang  a écrit :
>>
>>> On 23/05/2018 22:50, Ryosuke Niwa wrote:
>>> > As we have preciously discussed, we should NEVER commit new tests into
>>> > LayoutTests/imported/w3c/web-platform-tests.
>>>
>>
>> Ryosuke, correct me if I am wrong, I think you are pointing out the
>> following rule:
>> Changes to LayoutTests/imported/w3c/web-platform-tests tests should land
>> first in WPT repository, then in WebKit repository.
>>
>
> Oh, that is surprising.
>
> https://github.com/w3c/web-platform-tests/pull/10964 is a recent WebKit
> export, and https://trac.webkit.org/changeset/231788/webkit did modify
> the test in place. Do you mean that the WPT PR was merged first, or should
> be in general? Chromium and Gecko do it in the other order, and I'd be
> interested to understand the trade-offs of flipping the order.
>

Yes, it's okay for a WebKit commit to merge the change which got merged
into WPT but it's never okay to first commit the test into WebKit and then
later upstream it to WPT...

Any process like this where changes end up in WebKit trunk via anything
> except a full WPT import will mean that divergence is possible.
>

Precisely to avoid these problems.


> In Chromium, regular imports are the safeguard that will eventually
> resolve any issuesinvolves. In practice, much to my surprise, we've really
> never had something funny happen due to the temporary divergence the
> process involves.
>
>>
- R. Niwa
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread youenn fablet
>
>> Ryosuke, correct me if I am wrong, I think you are pointing out the
>> following rule:
>> Changes to LayoutTests/imported/w3c/web-platform-tests tests should land
>> first in WPT repository, then in WebKit repository.
>>
>
> Oh, that is surprising.
>
> https://github.com/w3c/web-platform-tests/pull/10964 is a recent WebKit
> export, and https://trac.webkit.org/changeset/231788/webkit did modify
> the test in place. Do you mean that the WPT PR was merged first, or should
> be in general? Chromium and Gecko do it in the other order, and I'd be
> interested to understand the trade-offs of flipping the order.
>

Yep, WPT PR merged first, then WebKit patch landed.
It makes sure that whenever we are reimporting tests, we are not loosing
any WebKit specific change.
Conflict resolution also happens at a time the patch is being committed in
WebKit.
So far, I encountered very few conflicts.

Any process like this where changes end up in WebKit trunk via anything
> except a full WPT import will mean that divergence is possible. In
> Chromium, regular imports are the safeguard that will eventually resolve
> any issuesinvolves. In practice, much to my surprise, we've really never
> had something funny happen due to the temporary divergence the process
> involves.
>

Agreed that in practice this probably does not matter much.
There is work to allow easier WPT imports so that it can be done more
regularly in WebKit.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev


Re: [webkit-dev] Standard process for exporting new WPT tests?

2018-05-24 Thread Philip Jägenstedt
On Wed, May 23, 2018, 23:43 youenn fablet  wrote:

> Le mer. 23 mai 2018 à 14:11, Frédéric Wang  a écrit :
>
>> On 23/05/2018 22:50, Ryosuke Niwa wrote:
>> > As we have preciously discussed, we should NEVER commit new tests into
>> > LayoutTests/imported/w3c/web-platform-tests.
>>
>
> Ryosuke, correct me if I am wrong, I think you are pointing out the
> following rule:
> Changes to LayoutTests/imported/w3c/web-platform-tests tests should land
> first in WPT repository, then in WebKit repository.
>

Oh, that is surprising.

https://github.com/w3c/web-platform-tests/pull/10964 is a recent WebKit
export, and https://trac.webkit.org/changeset/231788/webkit did modify the
test in place. Do you mean that the WPT PR was merged first, or should be
in general? Chromium and Gecko do it in the other order, and I'd be
interested to understand the trade-offs of flipping the order.

Any process like this where changes end up in WebKit trunk via anything
except a full WPT import will mean that divergence is possible. In
Chromium, regular imports are the safeguard that will eventually resolve
any issuesinvolves. In practice, much to my surprise, we've really never
had something funny happen due to the temporary divergence the process
involves.

>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev