Bug 223890

Summary: [ews] Add build step to find list of layout tests modified by a patch
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: RESOLVED FIXED    
Severity: Normal CC: aakash_jain, ap, dewei_zhu, jbedard, ryanhaddad, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=246323
Bug Depends on:    
Bug Blocks: 223938    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Aakash Jain 2021-03-29 11:44:19 PDT
Add build step to find list of layout tests modified by a patch. This list would be used by Stress test EWS in subsequent patches, to run those layout-tests large number of times.
Comment 1 Aakash Jain 2021-03-29 11:48:44 PDT
Created attachment 424556 [details]
Patch
Comment 2 Jonathan Bedard 2021-03-29 13:20:51 PDT
Comment on attachment 424556 [details]
Patch

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

If we want EWS to own this task without running run-webkit-tests (which is a reasonable requirement), look through layout_test_finder.py to make sure we duplicate all of the required logic.

> Tools/CISupport/ews-build/steps.py:463
> +    RE_LAYOUT_TEST = b'^(\+\+\+).*(LayoutTests.*.html)'

The logic for determining what is and is not a layout test are more complicated than this. Layout tests can be the following extensions (as per layout_test_finder.py):
    '.html', '.shtml', '.xml', '.xhtml', '.pl', '.py', '.htm', '.php', '.svg', '.mht', '.xht'
We also need exclude the following directories:
    'resources', 'support', 'script-tests', 'tools', 'reference', 'reftest'

> Tools/CISupport/ews-build/steps.py:469
> +            if match and (b'-expected.html' not in line):

Suffix exclusions include:
    '-expected', '-expected-mismatch', '-ref', '-notref'
Comment 3 Aakash Jain 2021-03-29 14:18:52 PDT
Created attachment 424578 [details]
Patch
Comment 4 Jonathan Bedard 2021-03-29 14:54:10 PDT
Comment on attachment 424578 [details]
Patch

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

Aakash wants to get this running in production and add additional extensions in a later patch

> Tools/CISupport/ews-build/steps.py:463
> +    RE_LAYOUT_TEST = b'^(\+\+\+).*(LayoutTests.*.html)'

I think this might should be:
    b'^(\+\+\+).* LayoutTests/(.*.html)'
although we might want to save that change for later, since based on some of the staging instance behavior, that may end up changing the behavior of skipped tests.

> Tools/CISupport/ews-build/steps.py:472
> +                if any((suffix + '.html').encode('utf-8') in line for suffix in self.SUFFIXES_TO_IGNORE):

We probably don't actually need to add .html here, although that will be more relevant when more extensions are added.
Comment 5 Aakash Jain 2021-03-30 09:29:43 PDT
Comment on attachment 424578 [details]
Patch

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

>> Tools/CISupport/ews-build/steps.py:472
>> +                if any((suffix + '.html').encode('utf-8') in line for suffix in self.SUFFIXES_TO_IGNORE):
> 
> We probably don't actually need to add .html here, although that will be more relevant when more extensions are added.

This would be more relevant when more extensions are added. Currently it's better to have .html as well here, so that we don't match in case these suffixes are in between, not at the end of the file name/path.
Comment 6 Aakash Jain 2021-03-30 09:31:46 PDT
Created attachment 424651 [details]
Patch
Comment 7 Aakash Jain 2021-03-30 09:36:57 PDT
Comment on attachment 424651 [details]
Patch

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

> Tools/CISupport/ews-build/steps.py:463
> +    RE_LAYOUT_TEST = b'^(\+\+\+).*(LayoutTests.*\.html)'

Modified this regular expression little bit to avoid matching any file when the file path contains a directory named html. 
For e.g. in https://ews-build.webkit-uat.org/#/builders/71/builds/229, this RE matched couple of directories (named html) instead of specific layout-tests.
Tested the change in https://ews-build.webkit-uat.org/#/builders/71/builds/241
Also added unit-test for this scenario (test_ignore_non_layout_test_in_html_directory).
Comment 8 EWS 2021-03-30 10:14:03 PDT
Committed r275211: <https://commits.webkit.org/r275211>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 424651 [details].
Comment 9 Radar WebKit Bug Importer 2021-03-30 10:15:18 PDT
<rdar://problem/76008872>