Bug 223890 – [ews] Add build step to find list of layout tests modified by a patch

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Aakash Jain]

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.

[Aakash Jain]

Created attachment 424556 [details]
Patch

[Jonathan Bedard]

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'

[Aakash Jain]

Created attachment 424578 [details]
Patch

[Jonathan Bedard]

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.

[Aakash Jain]

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.

[Aakash Jain]

Created attachment 424651 [details]
Patch

[Aakash Jain]

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).

[EWS]

Committed r275211: <https://commits.webkit.org/r275211>

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

[Radar WebKit Bug Importer]

<rdar://problem/76008872>