Skip to content

Commit

Permalink
[ews-build.webkit.org] Handle trailing cherry-pick annotation
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=255125
rdar://107732291

Reviewed by Aakash Jain.

* Tools/CISupport/ews-build/events.py:
(CommitClassifier.LineFilter): Renamed from HeaderFilter.
(CommitClassifier.__init__): Set trailers.
(CommitClassifier.matches): Implement processing of trailers. If a
classification defines both headers and trailers, only require either
a matching header or a matching trailer, but not both.
(GitHubEventHandlerNoEdits.classifiy): Pass commit message trailers to
CommitClassifier.matches.
(CommitClassifier.HeaderFilter): Renamed to LineFilter.
* Tools/CISupport/ews-build/events_unittest.py:
(TestCommitClassifier.test_regex_header_filter):
(TestCommitClassifier.test_fuzzy_header_filter):
(TestCommitClassifier.test_commit_class_gardening):
(TestCommitClassifier.test_commit_class_cherry_pick):
(TestCommitClassifier.test_commit_class_tools):

Canonical link: https://commits.webkit.org/262715@main
  • Loading branch information
JonWBedard committed Apr 7, 2023
1 parent 3b1f649 commit 62f3da9
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 35 deletions.
35 changes: 24 additions & 11 deletions Tools/CISupport/ews-build/events.py
Expand Up @@ -284,7 +284,7 @@ def __exit__(self, type, value, tb):
# Based off of webkitscmpy's CommitClassifier, although modified to not raise exceptions
# when provided invalid commit classes.
class CommitClassifier(object):
class HeaderFilter(object):
class LineFilter(object):
DEFAULT_FUZZ_RATIO = 90

@classmethod
Expand All @@ -308,25 +308,30 @@ def __repr__(self):
def __call__(self, string):
return bool(self.do(string)) if self.do else False

def __init__(self, name=None, pickable=True, headers=None, paths=None, **kwargs):
def __init__(self, name=None, pickable=True, headers=None, trailers=None, paths=None, **kwargs):
self.name = name or '?'
self.pickable = pickable
self.headers = [self.HeaderFilter(header) for header in headers or []]
self.headers = [self.LineFilter(header) for header in headers or []]
self.trailers = [self.LineFilter(trailer) for trailer in trailers or []]
self.paths = [re.compile(r'^{}'.format(path)) for path in (paths or [])]

if not self.headers and not self.paths:
if not self.headers and not self.trailers and not self.paths:
log.msg(f'Commit class {self.name} matches all commits')

for argument, _ in kwargs.items():
log.msg(f'{argument} is not a valid member of CommitClassifier')

def matches(self, header, files):
if not self.headers and not self.paths:
def matches(self, header, trailers, files):
if not self.headers and not self.trailers and not self.paths:
return False

if self.headers and not header:
return False
if self.headers and not any([h(header) for h in self.headers]):
matching_header = bool(self.headers and header)
matching_trailer = bool(self.trailers and trailers)

matches_header = self.headers and header and any([f(header) for f in self.headers])
matches_trailers = self.trailers and trailers and any([any([f(trailer) for f in self.trailers]) for trailer in trailers])

if (matching_header or matching_trailer) and not matches_header and not matches_trailers:
return False

if self.paths and not files:
Expand All @@ -346,6 +351,7 @@ class GitHubEventHandlerNoEdits(GitHubEventHandler):
MERGE_QUEUE_LABEL = 'merge-queue'
LABEL_PROCESS_DELAY = 10
ACCOUNTS_TO_IGNORE = ('webkit-early-warning-system', 'webkit-commit-queue')
TRAILER_RE = re.compile(r'^(?P<key>[^:()\t\/*]+): (?P<value>.+)')

_commit_classes = []
_last_commit_classes_refresh = 0
Expand Down Expand Up @@ -391,9 +397,16 @@ def classifiy(cls, message, files):
if not classes:
return defer.returnValue(None)

header = message.splitlines()[0]
lines = message.splitlines()
header = lines[0]
trailers = []
for line in reversed(lines):
if not cls.TRAILER_RE.match(line):
break
trailers.append(line)

for klass in classes:
if klass.matches(header, files):
if klass.matches(header, trailers, files):
return defer.returnValue(klass.name)
return defer.returnValue(None)

Expand Down
50 changes: 26 additions & 24 deletions Tools/CISupport/ews-build/events_unittest.py
Expand Up @@ -28,18 +28,18 @@

class TestCommitClassifier(unittest.TestCase):
def test_regex_header_filter(self):
self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning.'))
self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('versioning.'))
self.assertTrue(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning'))
self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning.'))
self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('versioning.'))
self.assertTrue(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning'))

self.assertFalse(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Bumped versioning'))
self.assertFalse(CommitClassifier.HeaderFilter('^[Vv]ersioning\\.?$')('Versioning bumped.'))
self.assertFalse(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Bumped versioning'))
self.assertFalse(CommitClassifier.LineFilter('^[Vv]ersioning\\.?$')('Versioning bumped.'))

def test_fuzzy_header_filter(self):
self.assertTrue(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[Gardening] Skip n tests'))
self.assertTrue(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[girdening] Skip n tests'))
self.assertTrue(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[Gardening] Skip n tests'))
self.assertTrue(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[girdening] Skip n tests'))

self.assertFalse(CommitClassifier.HeaderFilter({"value": "gardening", "ratio": 85})('[gdening] Skip n tests'))
self.assertFalse(CommitClassifier.LineFilter({"value": "gardening", "ratio": 85})('[gdening] Skip n tests'))

def test_commit_class_gardening(self):
c = CommitClassifier(
Expand All @@ -53,27 +53,29 @@ def test_commit_class_gardening(self):
)
self.assertEqual(c.name, 'Gardening')
self.assertFalse(c.pickable)
self.assertTrue(c.matches('[Gardening] Skip n tests', ['LayoutTests/TestExpectations']))
self.assertTrue(c.matches('[Gardening] Skip n tests', [], ['LayoutTests/TestExpectations']))

self.assertFalse(c.matches('[Gardening] Skip n tests', []))
self.assertFalse(c.matches('[gdening] Skip n tests', ['LayoutTests/TestExpectations']))
self.assertFalse(c.matches('[Gardening] Skip n tests', ['Makefile', 'LayoutTests/TestExpectations']))
self.assertFalse(c.matches('[Gardening] Skip n tests', [], []))
self.assertFalse(c.matches('[gdening] Skip n tests', [], ['LayoutTests/TestExpectations']))
self.assertFalse(c.matches('[Gardening] Skip n tests', [], ['Makefile', 'LayoutTests/TestExpectations']))

def test_commit_class_cherry_pick(self):
c = CommitClassifier(
name='Cherry-pick',
pickable=False,
headers=['^[Cc]herry[- ][Pp]ick'],
trailers=['^[Oo]riginally[- ]landed[- ]as:'],
)
self.assertEqual(c.name, 'Cherry-pick')
self.assertFalse(c.pickable)
self.assertTrue(c.matches('Cherry-pick abcde', ['somefile']))
self.assertTrue(c.matches('cherry-pick abcde', ['otherfile']))
self.assertTrue(c.matches('Cherry-Pick abcde', []))
self.assertTrue(c.matches('cherry pick abcde', []))
self.assertTrue(c.matches('Cherry-pick abcde', [], ['somefile']))
self.assertTrue(c.matches('cherry-pick abcde', [], ['otherfile']))
self.assertTrue(c.matches('Cherry-Pick abcde', [], []))
self.assertTrue(c.matches('cherry pick abcde', [], []))
self.assertTrue(c.matches('[Component] Some change', ['Originally-landed-as: 1234.1@branch (abcde)'], []))

self.assertFalse(c.matches('Partial cherry-pick', []))
self.assertFalse(c.matches('Took cherry-pick', []))
self.assertFalse(c.matches('Partial cherry-pick', [], []))
self.assertFalse(c.matches('Took cherry-pick', [], []))

def test_commit_class_tools(self):
c = CommitClassifier(
Expand All @@ -83,10 +85,10 @@ def test_commit_class_tools(self):
)
self.assertEqual(c.name, 'Tools')
self.assertTrue(c.pickable)
self.assertTrue(c.matches('Some change', ['Tools/Scripts/git-webkit']))
self.assertTrue(c.matches('', ['LayoutTests/some/test.html']))
self.assertTrue(c.matches('', ['metadata/contributors.json']))
self.assertTrue(c.matches('', ['Tools/Scripts/run-webkit-tests', 'metadata/commit_classes.json']))
self.assertTrue(c.matches('Some change', [], ['Tools/Scripts/git-webkit']))
self.assertTrue(c.matches('', [], ['LayoutTests/some/test.html']))
self.assertTrue(c.matches('', [], ['metadata/contributors.json']))
self.assertTrue(c.matches('', [], ['Tools/Scripts/run-webkit-tests', 'metadata/commit_classes.json']))

self.assertFalse(c.matches('', []))
self.assertFalse(c.matches('', ['metadata/contributors.json', 'Makefile']))
self.assertFalse(c.matches('', [], []))
self.assertFalse(c.matches('', [], ['metadata/contributors.json', 'Makefile']))

0 comments on commit 62f3da9

Please sign in to comment.