Re: [webkit-dev] WPT first test policy proposal

[Ryosuke Niwa via webkit-dev]

On Fri, Nov 19, 2021 at 1:06 PM Tim Nguyen via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

> Hello everyone,
>
> I would like to start a discussion on a policy to enforce WPT usage as a
> first choice, that would be enforced via check-webkit-style on Changelog
> files.
>
> *Why use WPT?*
>
> Contributing to WPT has many benefits:
>
>- interoperability/compatibility issues with WPT we write may be
>detected by other browser vendors and we would get faster feedback and
>turnaround to fix them
>- creates/encourages discussion in case of disagreement with other
>browser vendors
>- WebKit greatly benefits from WPT coverage, it is time to provide our
>own coverage to other browsers
>- Improves WebKit’s score generally for WPT (which has tests mostly
>contributed by Chromium, Firefox at the moment)
>
>
> *Are there reasons to not use WPT?*
>
> Common reasons for not writing WPT that have been mentioned are:
>
>- "WPTs are less pleasant to write.”
>
> This is not true imo, the WPT harness is documented (
> https://web-platform-tests.org/writing-tests/index.html), unlike WebKit
> internals, making it easier for new contributors to figure out things.
>

This is absolutely still the case for me. WPT harness require so much
boilerplate even today.

I absolutely hate having to describe what I'm testing when the test code
speaks for itself. This is akin to our WebKit coding style of not adding
comments that repeat the code.

I also absolutely hate having to start a HTTP server just to test some DOM
Node API. There is absolutely no reason why we need to do that.

Also, why do we need to call setup or done just to avoid a single test()
function call? The harness should be automatically detecting that.

I can go on and on about all other issues I have with WPT.

>
>- “When you actually move a regression test to WPT proper, commit
>history is lost, and you don't know what kind of user facing problem it's
>preventing any more.”
>
> Use `` with a reference to the webkit bug.
> Ensure you actually export the WPT and merge the WPT PR as well!
>

This is yet another thing we have to do. We shouldn't be making writing
WebKit tests harder.

*Here is my current proposal:*
>
> Every LayoutTests/ changelog adding new test files would contain:
>

I'm opposed to introducing a policy like this in general. WebKit project
avoids adding new policies wherever possible.

To me, we make the experience of writing WPT tests so much better & so much
more pleasant that people would prefer writing them over js-test.js tests
on their own, not forced upon by a policy.

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

Re: [webkit-dev] WPT first test policy proposal

[Fujii Hironori via webkit-dev]

It sounds a good idea. It'd be nice to be documented in the web site for
new contributors.
https://webkit.org/testing-contributions/

Do we need another WPT-exemption-reason 'non-standards' for WebKit specific
features?

I think WPT-exemption-reason should be in the test case rather than
ChangeLog.
It's easy to know what WPT-exemption-reason is the file you are watching.
It's easy to know how many WPT-exemption-reason:urgency test cases are
remaining by grepping.

On Sat, Nov 20, 2021 at 6:06 AM Tim Nguyen via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

>
>
In addition:
>
> WPT-PR: https://github.com/web-platform-tests/wpt/pulls/XX must be
> present in LayoutTests/imported/w3c/Changelog, this is to ensure tests are
> not lost at the next import.
>

Who will do the next import? Will someone update all test cases linked by
the WPT-PR line after the test case is committed to WPT repo? Or, will
someone update all test cases periodically?

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

[webkit-dev] WPT first test policy proposal

[Tim Nguyen via webkit-dev]

Hello everyone,

I would like to start a discussion on a policy to enforce WPT usage as a first 
choice, that would be enforced via check-webkit-style on Changelog files.

Why use WPT?

Contributing to WPT has many benefits:
interoperability/compatibility issues with WPT we write may be detected by 
other browser vendors and we would get faster feedback and turnaround to fix 
them
creates/encourages discussion in case of disagreement with other browser vendors
WebKit greatly benefits from WPT coverage, it is time to provide our own 
coverage to other browsers
Improves WebKit’s score generally for WPT (which has tests mostly contributed 
by Chromium, Firefox at the moment)

Are there reasons to not use WPT?

Common reasons for not writing WPT that have been mentioned are:
"WPTs are less pleasant to write.”
This is not true imo, the WPT harness is documented 
(https://web-platform-tests.org/writing-tests/index.html 
), unlike WebKit 
internals, making it easier for new contributors to figure out things.

“When you actually move a regression test to WPT proper, commit history is 
lost, and you don't know what kind of user facing problem it's preventing any 
more.”
Use `` with a reference to the webkit bug. Ensure you 
actually export the WPT and merge the WPT PR as well!

There are things WPT can’t test
True (e.g. tree dumps). My proposal below covers that.

Here is my current proposal:

Every LayoutTests/ changelog adding new test files would contain:

WPT-exemption-reason:  NONE (OOPS!)

If that line is removed or no valid reason is provided, webkit-style would 
throw out an error. Valid reasons could be:

WPT-exemption-reason: urgency (webkit.org/b/XXX )
I’m personally not in favour of this reason existing, but some have expressed 
interest in it, this may be used in case of urgency (security fixes etc.), 
where a bug must be filed to port it to WPT (alternatively the test path could 
be added to a new allowlist file). Eventually this reason would be removed if 
two way sync with the WPT repo is available.

WPT-exemption-reason: test-harness-deficiency (webkit.org/b/XXX 
)
If the WebKit implementation of the WPT harness lacks a certain feature that 
should be implemented in the WPT harness, a new bug may be filed, or an 
existing bug may be referenced.
Maintainers of the WPT harness may watch this reason to prioritize work needed 
on the harness.

WPT-exemption-reason: requires-internals: NONE (OOPS!)
requires-internals should have a valid explanation of which internals are 
needed. (Tree dumps would be a valid reason, although usually reftests can 
cover those)

In addition: 

WPT-PR: https://github.com/web-platform-tests/wpt/pulls/XX 
 must be present in 
LayoutTests/imported/w3c/Changelog, this is to ensure tests are not lost at the 
next import.

This would also be enforced in webkit-style, although eventually this might be 
replaced by a fully automated process in the future similar to Chrome/Firefox.

Any feedback is absolutely welcome!

Cheers,
Tim___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev