Summary: | [ews] Add build step to find list of layout tests modified by a patch | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Aakash Jain <aakash_jain> | ||||||||
Component: | Tools / Tests | Assignee: | 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
Aakash Jain
2021-03-29 11:44:19 PDT
Created attachment 424556 [details]
Patch
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' Created attachment 424578 [details]
Patch
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 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. Created attachment 424651 [details]
Patch
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). Committed r275211: <https://commits.webkit.org/r275211> All reviewed patches have been landed. Closing bug and clearing flags on attachment 424651 [details]. |