Bug 227744 - [GTK] Add a new GTK layout tester bot to build.webkit.org that runs with --skip-failing-tests switch
Summary: [GTK] Add a new GTK layout tester bot to build.webkit.org that runs with --sk...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Carlos Alberto Lopez Perez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-07 07:46 PDT by Carlos Alberto Lopez Perez
Modified: 2021-08-18 18:48 PDT (History)
5 users (show)

See Also:


Attachments
Patch (4.19 KB, patch)
2021-07-07 07:50 PDT, Carlos Alberto Lopez Perez
no flags Details | Formatted Diff | Diff
Patch (4.21 KB, patch)
2021-07-12 11:11 PDT, Carlos Alberto Lopez Perez
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2021-07-07 07:46:53 PDT
We are trying to have a layout tests runner for the GTK port in the EWS, but we are experimenting there random flakiness and failures that are not reproducible on the GTK layout tester of build.webkit.org

I think this is related with the different way of running the layout tests:
- On build.webkit.org all known tests are scheduled for running (even those that are expected to crash or timeout).
- On the EWS, the switch --skip-failing-tests is passed and that changes the order in which the tests are run because the test that are expected to fail are not scheduled for running.

This different order of scheduling the test in the EWS I think it may cause different results sometimes or can be a source of undetected flakiness.

So I think is a good idea to add a new tester for the GTK port for build.webkit.org that runs with this flag like in the EWS.

I believe that will help us to easily discover and make easier doing gardening of this tests that fail or are flaky due to this different order of scheduling the tests because thanks to this bot we will get access to info about this runs in https://results.webkit.org and wktesthunter <https://github.com/clopez/webkit-testhunter>
Comment 1 Carlos Alberto Lopez Perez 2021-07-07 07:50:12 PDT
Created attachment 433028 [details]
Patch
Comment 2 Jonathan Bedard 2021-07-12 08:28:55 PDT
If I'm understanding the thought process here: You are adding a new queue so that the failures caused by test ordering are surfaced somewhere and can be investigated so that the tree can be kept clean enough for EWS to be effective.

That generally seems reasonable, just a few thoughts here because this is a problem Apple's port have encountered too:
- Such failures are usually indicative of a bug in either the test harness or the engine itself, tests *shouldn't* be able to effect the outcome of tests running after them, but this happens commonly enough that's it's something we explicitly check for any time we encounter a flakey test
- When we've noticed this happening with many tests, it's often just a few bugs and you can make a huge difference by focusing on a single random case of test interdependence and fixing it
- Test order is never consistent, so while this strategy is probably a good idea as a temporary measure to get EWS working, it's probably not going to be a great permanent solution.
Comment 3 Aakash Jain 2021-07-12 08:36:00 PDT
Comment on attachment 433028 [details]
Patch

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

> Tools/CISupport/build-webkit-org/config.json:438
> +                      "name": "GTK-Linux-64-bit-Release-Skip-Failing-EWS-Tests", "factory": "TestAllButJSCFactory", "builddir": "gtk-linux-64-release-skip-failing-ews-tests",

Nit: I don't think builder name should have EWS in it. All it does is pass an additional argument which is already in the name.
Comment 4 Carlos Alberto Lopez Perez 2021-07-12 11:09:31 PDT
(In reply to Jonathan Bedard from comment #2)
> If I'm understanding the thought process here: You are adding a new queue so
> that the failures caused by test ordering are surfaced somewhere and can be
> investigated so that the tree can be kept clean enough for EWS to be
> effective.
> 
> That generally seems reasonable, just a few thoughts here because this is a
> problem Apple's port have encountered too:
> - Such failures are usually indicative of a bug in either the test harness
> or the engine itself, tests *shouldn't* be able to effect the outcome of
> tests running after them, but this happens commonly enough that's it's
> something we explicitly check for any time we encounter a flakey test
> - When we've noticed this happening with many tests, it's often just a few
> bugs and you can make a huge difference by focusing on a single random case
> of test interdependence and fixing it
> - Test order is never consistent, so while this strategy is probably a good
> idea as a temporary measure to get EWS working, it's probably not going to
> be a great permanent solution.

Thanks for the thoughts. I agree with them. The main idea here is to use this bot as a temporary measure to get the EWS working with the pass rate that is expected. I think it can be also helpful to identify better those cases of test interdependences and help fixing them.
Comment 5 Carlos Alberto Lopez Perez 2021-07-12 11:11:00 PDT
Created attachment 433332 [details]
Patch

Remove EWS from the bot queue name
Comment 6 Carlos Alberto Lopez Perez 2021-07-12 11:56:25 PDT
Comment on attachment 433332 [details]
Patch

Clearing flags on attachment: 433332

Committed r279844 (239600@main): <https://commits.webkit.org/239600@main>
Comment 7 Carlos Alberto Lopez Perez 2021-07-12 11:56:30 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Radar WebKit Bug Importer 2021-07-12 11:57:15 PDT
<rdar://problem/80474599>
Comment 9 Aakash Jain 2021-07-12 13:59:48 PDT
Restarted buildbot few hours back to pick up this change. Queue is live at: https://build.webkit.org/#/builders/GTK-Linux-64-bit-Release-Skip-Failing-Tests