Bug 181589 – [GTK][WPE] Add support for unit test expectations

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

We currently have a way to skip tests by annotating them in the api test runner script. The main problem of this approach is that we skip test when they fail in the bots and we never notice if they stop failing, keeping the tests skipped forever. This is indeed the case of several WebKit2 C API tests. Annotating skipped tests in the script itself is not a good idea either.

[Carlos Garcia Campos]

Created attachment 331195 [details]
Patch

This will also allow us to handle flaky tests, by re-running the unexpected failures.

[Michael Catanzaro]

Comment on attachment 331195 [details]
Patch

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

I haven't looked in detail at the python code, but it looks sane. Bonus points for including unit tests.

I don't really like json, but this is clearly superior to what we had before. Thanks.

The next step would be to update the expectations for WPE, since the WPE API tests are in real bad shape.

I would name the file TestExpectations.json, instead of expectations.json, since it will be more familiar to WebKit developers. The purpose of the file should then be immediately obvious.

> Tools/ChangeLog:10
> +        We currently have a way to skip tests by annotating them in the api test runner script. The main problem of this
> +        approach is that we skip test when they fail in the bots and we never notice if they stop failing, keeping the
> +        tests skipped forever. This is indeed the case of several WebKit2 C API tests. Annotating skipped tests in the

I think it's understood that skipped tests are broken and need to be fixed. We just need to actually start fixing them instead of ignoring them....

> Tools/Scripts/webkitpy/common/test_expectations_unittest.py:33
> +class MockTestExpectations(TestExpectations):

Are these tests run automatically, as part of testwebkitpy? If not, please find a way to make sure they are. Nobody (except maybe you) is ever going to run this manually: having tests is not worthwhile unless they're running on the bots.

IMO it's fine to run this test on all platforms.

[Carlos Garcia Campos]

(In reply to Michael Catanzaro from comment #2)
> Comment on attachment 331195 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=331195&action=review
> 
> I haven't looked in detail at the python code, but it looks sane. Bonus
> points for including unit tests.
> 
> I don't really like json, but this is clearly superior to what we had
> before. Thanks.
> 
> The next step would be to update the expectations for WPE, since the WPE API
> tests are in real bad shape.

Yes, that's my idea.

> I would name the file TestExpectations.json, instead of expectations.json,
> since it will be more familiar to WebKit developers. The purpose of the file
> should then be immediately obvious.

Ok.

> > Tools/ChangeLog:10
> > +        We currently have a way to skip tests by annotating them in the api test runner script. The main problem of this
> > +        approach is that we skip test when they fail in the bots and we never notice if they stop failing, keeping the
> > +        tests skipped forever. This is indeed the case of several WebKit2 C API tests. Annotating skipped tests in the
> 
> I think it's understood that skipped tests are broken and need to be fixed.
> We just need to actually start fixing them instead of ignoring them....

That's not how things work unfortunately. Very often we skip unit tests that stop passing for a unknown reason only to keep the bots green, sometimes it even only fails on the bots and work locally. Then, the test suddenly start passing again for another unknown reason, but since it's skipped we never notice it. It has happened so many times, all WebKit2 C API tests currently skipped pass for me locally, and I bet they also pass in the bots, we'll see when this patch lands.

> > Tools/Scripts/webkitpy/common/test_expectations_unittest.py:33
> > +class MockTestExpectations(TestExpectations):
> 
> Are these tests run automatically, as part of testwebkitpy? If not, please
> find a way to make sure they are. Nobody (except maybe you) is ever going to
> run this manually: having tests is not worthwhile unless they're running on
> the bots.
> 
> IMO it's fine to run this test on all platforms.

Yes, they are run automatically for all platforms.

[Carlos Garcia Campos]

Committed r226944: <https://trac.webkit.org/changeset/226944>

[Radar WebKit Bug Importer]

<rdar://problem/36514890>