- Revision
- 262565
- Author
- clo...@igalia.com
- Date
- 2020-06-04 13:16:05 -0700 (Thu, 04 Jun 2020)
Log Message
Improve watchlist logic for comments on patches touching imported WPT tests.
https://bugs.webkit.org/show_bug.cgi?id=212597
Reviewed by Youenn Fablet.
On r262295 I added a watchlist comment for patches touching the imported WPT tests.
However, this is commenting on patches that are importing WPT tests.
To avoid this situations, this patch adds a new rule to detect if the changes modify
any of the w3c-import.log files, and then changes the logic to make the comment only
for patches that modify the WPT imported tests but not the w3c-import.log files.
In order to support this new logic, watchlist rule parsing is improved to support
the "and" and "not" operators. Previously it only supported the "or" operator.
* Scripts/webkitpy/common/config/watchlist:
* Scripts/webkitpy/common/watchlist/watchlistparser.py:
(WatchListParser._rule_definitions_as_set):
* Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py:
(WatchListParserTest.test_cc_rule_with_undefined_defintion_with_suggestion):
(WatchListParserTest):
(WatchListParserTest.test_cc_rule_with_complex_logic):
* Scripts/webkitpy/common/watchlist/watchlistrule.py:
(WatchListRule.__init__):
(WatchListRule._match_test_definitions):
(WatchListRule.match):
* Scripts/webkitpy/common/watchlist/watchlistrule_unittest.py:
(WatchListRuleTest.test_complex_definition_or):
(WatchListRuleTest):
(WatchListRuleTest.test_complex_definition_and):
(WatchListRuleTest.test_complex_definition_not):
(WatchListRuleTest.test_complex_definition_combined):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (262564 => 262565)
--- trunk/Tools/ChangeLog 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/ChangeLog 2020-06-04 20:16:05 UTC (rev 262565)
@@ -1,5 +1,40 @@
2020-06-04 Carlos Alberto Lopez Perez <clo...@igalia.com>
+ Improve watchlist logic for comments on patches touching imported WPT tests.
+ https://bugs.webkit.org/show_bug.cgi?id=212597
+
+ Reviewed by Youenn Fablet.
+
+ On r262295 I added a watchlist comment for patches touching the imported WPT tests.
+ However, this is commenting on patches that are importing WPT tests.
+
+ To avoid this situations, this patch adds a new rule to detect if the changes modify
+ any of the w3c-import.log files, and then changes the logic to make the comment only
+ for patches that modify the WPT imported tests but not the w3c-import.log files.
+
+ In order to support this new logic, watchlist rule parsing is improved to support
+ the "and" and "not" operators. Previously it only supported the "or" operator.
+
+ * Scripts/webkitpy/common/config/watchlist:
+ * Scripts/webkitpy/common/watchlist/watchlistparser.py:
+ (WatchListParser._rule_definitions_as_set):
+ * Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py:
+ (WatchListParserTest.test_cc_rule_with_undefined_defintion_with_suggestion):
+ (WatchListParserTest):
+ (WatchListParserTest.test_cc_rule_with_complex_logic):
+ * Scripts/webkitpy/common/watchlist/watchlistrule.py:
+ (WatchListRule.__init__):
+ (WatchListRule._match_test_definitions):
+ (WatchListRule.match):
+ * Scripts/webkitpy/common/watchlist/watchlistrule_unittest.py:
+ (WatchListRuleTest.test_complex_definition_or):
+ (WatchListRuleTest):
+ (WatchListRuleTest.test_complex_definition_and):
+ (WatchListRuleTest.test_complex_definition_not):
+ (WatchListRuleTest.test_complex_definition_combined):
+
+2020-06-04 Carlos Alberto Lopez Perez <clo...@igalia.com>
+
svn-apply command is too slow with big patches
https://bugs.webkit.org/show_bug.cgi?id=212766
Modified: trunk/Tools/Scripts/webkitpy/common/config/watchlist (262564 => 262565)
--- trunk/Tools/Scripts/webkitpy/common/config/watchlist 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/Scripts/webkitpy/common/config/watchlist 2020-06-04 20:16:05 UTC (rev 262565)
@@ -380,6 +380,9 @@
"WebPlatformTests": {
"filename": r"LayoutTests/imported/w3c/web-platform-tests/((?!-expected\.txt).)*$",
},
+ "WebPlatformTestsImport": {
+ "filename": r"LayoutTests/imported/w3c/web-platform-tests/.*w3c-import.log$",
+ },
"WebRTC": {
"filename": r"Source/ThirdParty/libwebrtc/"
r"|Source/WebKit/Shared/*RTC*"
@@ -478,7 +481,7 @@
"See http://trac.webkit.org/wiki/UpdatingANGLE", ],
"WebInspectorProtocol": [ "This patch modifies the inspector protocol. Please ensure that any frontend changes appropriately use feature checks for new protocol features." ],
"WebInspectorGenerator": [ "This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)" ],
- "WebPlatformTests": [ "This patch modifies the imported WPT tests. Please ensure that any changes on the tests are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess" ],
+ "WebPlatformTests&!WebPlatformTestsImport": [ "This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess" ],
"JSBuiltinsGenerator": [ "This patch modifies the JS builtins code generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-builtins-generator-tests --reset-results`)" ],
"WasmJSON": [ "This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at \"Source/_javascript_Core/wasm/wasm.json\" and \"JSTests/wasm/wasm.json\"." ],
},
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py (262564 => 262565)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser.py 2020-06-04 20:16:05 UTC (rev 262565)
@@ -177,5 +177,6 @@
def _rule_definitions_as_set(self, rules):
definition_set = set()
for rule in rules:
- definition_set = definition_set.union(rule.definitions_to_match)
+ definitions_inside_rule = re.split('&|\|', rule.definitions_to_match.replace('!', ''))
+ definition_set = definition_set.union(definitions_inside_rule)
return definition_set
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py (262564 => 262565)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistparser_unittest.py 2020-06-04 20:16:05 UTC (rev 262565)
@@ -280,3 +280,31 @@
OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
expected_logs='In section "CC_RULES", the following definitions are not used and should be removed: WatchList'
+ '\n\nPerhaps it should be WatchList1.\n')
+
+ def test_cc_rule_with_complex_logic(self):
+ watch_list = (
+ '{'
+ ' "DEFINITIONS": {'
+ ' "WatchList1": {'
+ ' "filename": r".*MyFileName\\.cpp",'
+ ' },'
+ ' "WatchList2": {'
+ ' "filename": r".*MyFileName\\.h",'
+ ' },'
+ ' "WatchList3": {'
+ ' "filename": r".*MyFileName\\.o",'
+ ' },'
+ ' },'
+ ' "CC_RULES": {'
+ ' "!WatchList1&!WatchList2|!WatchList3&!WatchListUndefined": ["clo...@igalia.com"]'
+ ' },'
+ ' "MESSAGE_RULES": {'
+ ' "!WatchList1|WatchList2&!WatchList3|!WatchListUndefined": ["clo...@igalia.com"]'
+ ' },'
+ '}')
+
+ OutputCapture().assert_outputs(self, self._watch_list_parser.parse, args=[watch_list],
+ expected_logs='In section "CC_RULES", the following definitions are not used and should be removed: WatchListUndefined\n\n' +
+ 'Perhaps it should be WatchList3 or WatchList2 or WatchList1.\n' +
+ 'In section "MESSAGE_RULES", the following definitions are not used and should be removed: WatchListUndefined\n\n' +
+ 'Perhaps it should be WatchList3 or WatchList2 or WatchList1.\n')
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule.py (262564 => 262565)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule.py 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule.py 2020-06-04 20:16:05 UTC (rev 262565)
@@ -30,12 +30,24 @@
class WatchListRule:
'''A rule with instructions to do when the rule is satisified.'''
def __init__(self, complex_definition, instructions):
- self.definitions_to_match = complex_definition.split('|')
+ self.definitions_to_match = complex_definition
self._instructions = instructions
+ def _match_test_definitions(self, test, matching_definitions):
+ if test.startswith('!'):
+ return test.split('!', 1)[1] not in matching_definitions
+ return test in matching_definitions
+
def match(self, matching_definitions):
- for test_definition in self.definitions_to_match:
- if test_definition in matching_definitions:
+ for test_definition in self.definitions_to_match.split('|'):
+ if '&' in test_definition:
+ match_all_subgroup = True
+ for test_definition_suball in test_definition.split('&'):
+ if not self._match_test_definitions(test_definition_suball, matching_definitions):
+ match_all_subgroup = False
+ if match_all_subgroup:
+ return True
+ elif self._match_test_definitions(test_definition, matching_definitions):
return True
return False
Modified: trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule_unittest.py (262564 => 262565)
--- trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule_unittest.py 2020-06-04 20:02:29 UTC (rev 262564)
+++ trunk/Tools/Scripts/webkitpy/common/watchlist/watchlistrule_unittest.py 2020-06-04 20:16:05 UTC (rev 262565)
@@ -50,7 +50,7 @@
self.assertTrue(rule.match([definition_name]))
self.assertFalse(rule.match([definition_name + '1']))
- def test_complex_definition(self):
+ def test_complex_definition_or(self):
definition_name1 = 'definition1'
definition_name2 = 'definition2'
definition_name3 = 'definition3'
@@ -61,3 +61,57 @@
self.assertFalse(rule.match([definition_name1 + '1']))
self.assertFalse(rule.match([definition_name2 + '1']))
self.assertFalse(rule.match([definition_name3 + '1']))
+
+ def test_complex_definition_and(self):
+ definition_name1 = 'definition1'
+ definition_name2 = 'definition2'
+ definition_name3 = 'definition3'
+ rule = WatchListRule(definition_name1 + '&' + definition_name2 + '&' + definition_name3, [])
+ self.assertFalse(rule.match([definition_name1]))
+ self.assertFalse(rule.match([definition_name1, definition_name2]))
+ self.assertFalse(rule.match([definition_name1, definition_name3]))
+ self.assertFalse(rule.match([definition_name2]))
+ self.assertFalse(rule.match([definition_name2, definition_name3]))
+ self.assertFalse(rule.match([definition_name3]))
+ self.assertTrue(rule.match([definition_name1, definition_name2, definition_name3]))
+
+ def test_complex_definition_not(self):
+ definition_name1 = 'definition1'
+ definition_name2 = 'definition2'
+ rule = WatchListRule('!' + definition_name1, [])
+ self.assertFalse(rule.match([definition_name1]))
+ self.assertTrue(rule.match([definition_name2]))
+
+ def test_complex_definition_combined(self):
+ definition_name1 = 'definition1'
+ definition_name2 = 'definition2'
+ definition_name3 = 'definition3'
+ definition_name4 = 'definition4'
+ rule1 = WatchListRule(definition_name1 + '&!' + definition_name2, [])
+ self.assertFalse(rule1.match([definition_name1, definition_name2]))
+ self.assertFalse(rule1.match([definition_name2]))
+ self.assertTrue(rule1.match([definition_name1]))
+ rule2 = WatchListRule(definition_name1 + '|' + definition_name2 + '&!' + definition_name3, [])
+ self.assertFalse(rule2.match([definition_name2, definition_name3]))
+ self.assertFalse(rule2.match([definition_name3]))
+ self.assertTrue(rule2.match([definition_name1]))
+ self.assertTrue(rule2.match([definition_name1, definition_name2]))
+ self.assertTrue(rule2.match([definition_name1, definition_name2, definition_name3]))
+ self.assertTrue(rule2.match([definition_name1, definition_name3]))
+ self.assertTrue(rule2.match([definition_name2]))
+ rule3 = WatchListRule(definition_name1 + '|' + definition_name2 + '&!' + definition_name3 + '|!' + definition_name4, [])
+ self.assertFalse(rule3.match([definition_name2, definition_name3, definition_name4]))
+ self.assertFalse(rule3.match([definition_name2, definition_name3, definition_name4]))
+ self.assertFalse(rule3.match([definition_name3, definition_name4]))
+ self.assertFalse(rule3.match([definition_name3, definition_name4]))
+ self.assertFalse(rule3.match([definition_name4]))
+ self.assertTrue(rule3.match([definition_name1]))
+ self.assertTrue(rule3.match([definition_name1, definition_name2, definition_name3, definition_name4]))
+ self.assertTrue(rule3.match([definition_name1, definition_name2, definition_name4]))
+ self.assertTrue(rule3.match([definition_name1, definition_name3, definition_name4]))
+ self.assertTrue(rule3.match([definition_name1, definition_name4]))
+ self.assertTrue(rule3.match([definition_name2]))
+ self.assertTrue(rule3.match([definition_name2, definition_name4]))
+ self.assertTrue(rule3.match([definition_name2, definition_name4]))
+ self.assertTrue(rule3.match([definition_name2, definition_name4]))
+ self.assertTrue(rule3.match([definition_name3]))
