Bug 223890 - [ews] Add build step to find list of layout tests modified by a patch
Summary: [ews] Add build step to find list of layout tests modified by a patch
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords: InRadar
Depends on:
Blocks: 223938
  Show dependency treegraph
 
Reported: 2021-03-29 11:44 PDT by Aakash Jain
Modified: 2021-03-30 10:23 PDT (History)
6 users (show)

See Also:


Attachments
Patch (8.03 KB, patch)
2021-03-29 11:48 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Patch (9.54 KB, patch)
2021-03-29 14:18 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff
Patch (9.96 KB, patch)
2021-03-30 09:31 PDT, Aakash Jain
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>