EWS should skip mac-wk1 and mac-debug-wk1 tests for patches that only change WebKit2 sources

• Component: Tools / Tests
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

EWS should skip mac-wk1 and mac-debug-wk1 tests for patches that only change WebKit2 sources.

The patches attached to Bug 210094 only changed files in the Source/WebKit folder.

Running mac-wk1 and mac-debug-wk1 tests doesn't seem useful in that case.

[David Kilzer (:ddkilzer)]

(In reply to David Kilzer (:ddkilzer) from comment #0)
> Running mac-wk1 and mac-debug-wk1 tests doesn't seem useful in that case.

The only thing it would provide is more evidence of flakey WK1 tests on those bots, I guess.  I'm not sure if folks use EWS test results such a specific way, though.  (I suspect there are plenty of patches that would run the WK1 tests to discover flakey tests, though.)

[Radar WebKit Bug Importer]

<rdar://problem/61395869>

[Aakash Jain]

> I'm not sure if folks use EWS test results such a specific way, though.  
We don't currently use the flakiness information from EWS in any meaningful manner, apart from checking manually when a test becomes too much flaky.

[Aakash Jain]

(In reply to David Kilzer (:ddkilzer) from comment #0)
> EWS should skip mac-wk1 and mac-debug-wk1 tests for patches that only change WebKit2 sources.
I like the idea.
Can we come up with a list of folders for which wk1 tests should be run? e.g.: 'Source/WebKitLegacy', 'LayoutTests'

The tests will be skipped if the patch doesn't touch any of those. We have similar logic in ews for various queues like jsc/webkitpy/service/bindings in https://trac.webkit.org/browser/webkit/trunk/Tools/BuildSlaveSupport/ews-build/steps.py#L250

[David Kilzer (:ddkilzer)]

(In reply to Aakash Jain from comment #4)
> (In reply to David Kilzer (:ddkilzer) from comment #0)
> > EWS should skip mac-wk1 and mac-debug-wk1 tests for patches that only change WebKit2 sources.
> I like the idea.
> Can we come up with a list of folders for which wk1 tests should be run?
> e.g.: 'Source/WebKitLegacy', 'LayoutTests'

Basically anything in WebKitLegacy or "above", including tests:

Source/bmalloc
Source/WTF
Source/JavaScriptCore
Source/ThirdParty/{ANGLE,libwebrtc,gtest}
Source/WebCore
Source/WebDriver (?)
Source/WebInspectorUI (?)
Source/WebKitLegacy
LayoutTests
Tools

I think it would be easier to say "if the patch only changes files under Source/WebKit, then skip wk1 EWS tests"; rather than "if the patch changes these directories, run wk1 EWS tests".

Maybe it's easier to do the latter, though?

[Aakash Jain]

Created attachment 395807 [details]
Patch

[Aakash Jain]

(In reply to David Kilzer (:ddkilzer) from comment #5)
> I think it would be easier to say "if the patch only changes files under Source/WebKit, then skip wk1 EWS tests"; rather than "if the patch changes these directories, run wk1 EWS tests".
> 
> Maybe it's easier to do the latter, though?
Yes, we already have the code for that. I can look into adding support for the former if we realize that current approach isn't working well.

[Aakash Jain]

Sample runs:
Patch without relevant change: https://ews-build.webkit-uat.org/#/builders/32/builds/4
Patch with relevant change: https://ews-build.webkit-uat.org/#/builders/32/builds/3

[EWS]

Committed r259718: <https://trac.webkit.org/changeset/259718>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 395807 [details].

[Aakash Jain]

Deployed on the server. Seems to be working fine. e.g.: https://bugs.webkit.org/show_bug.cgi?id=210229

Please let me know if you notice wk1 tests being skipped on any patch on which it shouldn't be skipped, or if you have further ideas to skip build/test on other queues.

[David Kilzer (:ddkilzer)]

(In reply to Aakash Jain from comment #10)
> Deployed on the server. Seems to be working fine. e.g.:
> https://bugs.webkit.org/show_bug.cgi?id=210229
> 
> Please let me know if you notice wk1 tests being skipped on any patch on
> which it shouldn't be skipped, or if you have further ideas to skip
> build/test on other queues.

Awesome!  Will do.