[REF] forwardport: update re_matches to not use a regex

Using a regex as the pattern is quite frustrating due to all the
escaping necessary, which in this refactoring I found out I'd missed,
multiple times.

Convert the pattern to something bespoke but not too complicated, we
may want to add anchoring support and a bit more finesse and the
future but for now straightforward "holes" seem to work well. I've
added support for capturing and even named groups even if this as yet
unnecessary and unused.

Fixes #861

[^1]: https://docs.pytest.org/en/stable/reference.html#pytest.hookspec.pytest_assertrepr_compare
This commit is contained in:
Xavier Morel 2024-06-04 12:15:29 +02:00
parent 98aaa9107f
commit c1e2e5a2e0
6 changed files with 69 additions and 57 deletions

View File

@ -1,6 +1,6 @@
import re import re
from utils import Commit, make_basic, to_pr, seen, re_matches from utils import Commit, make_basic, to_pr, seen, matches
def test_single_updated(env, config, make_repo): def test_single_updated(env, config, make_repo):
@ -381,26 +381,26 @@ def test_add_to_forward_port_conflict(env, config, make_repo, users):
assert pr2_c.comments == [ assert pr2_c.comments == [
seen(env, pr2_c, users), seen(env, pr2_c, users),
# should have conflicts # should have conflicts
(users['user'], re_matches(r"""@{user} cherrypicking of pull request {previous.display_name} failed\. (users['user'], matches("""@{user} cherrypicking of pull request {previous.display_name} failed.
stdout: stdout:
``` ```
Auto-merging b Auto-merging b
CONFLICT \(add/add\): Merge conflict in b CONFLICT (add/add): Merge conflict in b
``` ```
stderr: stderr:
``` ```
.* $$
``` ```
Either perform the forward-port manually \(and push to this branch, proceeding as usual\) or close this PR \(maybe\?\)\. Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
In the former case, you may want to edit this PR message as well\. In the former case, you may want to edit this PR message as well.
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}\. :warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}.
More info at https://github\.com/odoo/odoo/wiki/Mergebot#forward-port More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
""".format(project=project, previous=pr2_b_id, **users), re.DOTALL)) """.format(project=project, previous=pr2_b_id, **users)))
] ]

View File

@ -2,7 +2,7 @@ import re
import time import time
from operator import itemgetter from operator import itemgetter
from utils import make_basic, Commit, validate_all, re_matches, seen, REF_PATTERN, to_pr from utils import make_basic, Commit, validate_all, matches, seen, REF_PATTERN, to_pr
def test_conflict(env, config, make_repo, users): def test_conflict(env, config, make_repo, users):
@ -59,39 +59,39 @@ def test_conflict(env, config, make_repo, users):
assert prod.read_tree(c) == { assert prod.read_tree(c) == {
'f': 'c', 'f': 'c',
'g': 'a', 'g': 'a',
'h': re_matches(r'''<<<\x3c<<< HEAD 'h': matches('''<<<\x3c<<< HEAD
a a
|||||||| parent of [\da-f]{7,}.* ||||||| parent of $$ (temp)
======= =======
xxx xxx
>>>\x3e>>> [\da-f]{7,}.* >>>\x3e>>> $$ (temp)
'''), '''),
} }
assert prc.comments == [ assert prc.comments == [
seen(env, prc, users), seen(env, prc, users),
(users['user'], re_matches( (users['user'], matches(
fr'''@{users['user']} @{users['reviewer']} cherrypicking of pull request {pra_id.display_name} failed\. f'''@{users['user']} @{users['reviewer']} cherrypicking of pull request {pra_id.display_name} failed.
stdout: stdout:
``` ```
Auto-merging h Auto-merging h
CONFLICT \(add/add\): Merge conflict in h CONFLICT (add/add): Merge conflict in h
``` ```
stderr: stderr:
``` ```
.* $$
``` ```
Either perform the forward-port manually \(and push to this branch, proceeding as usual\) or close this PR \(maybe\?\)\. Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
In the former case, you may want to edit this PR message as well\. In the former case, you may want to edit this PR message as well.
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}\. :warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}.
More info at https://github\.com/odoo/odoo/wiki/Mergebot#forward-port More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
''', re.DOTALL)) '''))
] ]
prb = prod.get_pr(prb_id.number) prb = prod.get_pr(prb_id.number)
@ -372,7 +372,7 @@ b
assert pr2.comments == [ assert pr2.comments == [
seen(env, pr2, users), seen(env, pr2, users),
(users['user'], re_matches(r'@%s @%s .*CONFLICT' % (users['user'], users['reviewer']), re.DOTALL)), (users['user'], matches('@%s @%s $$CONFLICT' % (users['user'], users['reviewer']))),
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+'),
(users['user'], f"@{users['user']} @{users['reviewer']} unable to stage: " (users['user'], f"@{users['user']} @{users['reviewer']} unable to stage: "
"All commits must have author and committer email, " "All commits must have author and committer email, "

View File

@ -6,7 +6,7 @@ from datetime import datetime, timedelta
import pytest import pytest
from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr, re_matches from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr, matches
FMT = '%Y-%m-%d %H:%M:%S' FMT = '%Y-%m-%d %H:%M:%S'
FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT) FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT)
@ -346,27 +346,27 @@ def test_empty(env, config, make_repo, users):
pr1_id.display_name pr1_id.display_name
) )
) )
conflict = (users['user'], re_matches( conflict = (users['user'], matches(
fr"""@{users['user']} @{users['reviewer']} cherrypicking of pull request {pr1_id.display_name} failed\. f"""@{users['user']} @{users['reviewer']} cherrypicking of pull request {pr1_id.display_name} failed.
stdout: stdout:
``` ```
.* $$
``` ```
stderr: stderr:
``` ```
.* $$
``` ```
Either perform the forward-port manually \(and push to this branch, proceeding as usual\) or close this PR \(maybe\?\)\. Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).
In the former case, you may want to edit this PR message as well\. In the former case, you may want to edit this PR message as well.
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}\. :warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}.
More info at https://github\.com/odoo/odoo/wiki/Mergebot#forward-port More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
""", re.DOTALL)) """))
assert pr1.comments == [ assert pr1.comments == [
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+'),
seen(env, pr1, users), seen(env, pr1, users),

View File

@ -6,7 +6,7 @@ import re
import pytest import pytest
from utils import seen, re_matches, Commit, make_basic, to_pr from utils import seen, matches, Commit, make_basic, to_pr
@pytest.mark.parametrize("merge_parent", [False, True]) @pytest.mark.parametrize("merge_parent", [False, True])
@ -433,12 +433,12 @@ def test_subsequent_conflict(env, make_repo, config, users):
assert repo.read_tree(repo.commit(pr3_id.head)) == { assert repo.read_tree(repo.commit(pr3_id.head)) == {
'f': 'c', 'f': 'c',
'g': 'a', 'g': 'a',
'h': re_matches(r'''<<<\x3c<<< HEAD 'h': matches('''<<<\x3c<<< HEAD
a a
|||||||| parent of [\da-f]{7,}.* ||||||| parent of $$ (temp)
======= =======
conflict! conflict!
>>>\x3e>>> [\da-f]{7,}.* >>>\x3e>>> $$ (temp)
'''), '''),
'x': '0', 'x': '0',
} }
@ -458,18 +458,22 @@ conflict!
# 1. link to status page # 1. link to status page
# 2. forward-port chain thing # 2. forward-port chain thing
assert repo.get_pr(pr3_id.number).comments[2:] == [ assert repo.get_pr(pr3_id.number).comments[2:] == [
(users['user'], re_matches(f'''\ (users['user'], matches(f'''\
@{users['user']} @{users['reviewer']} WARNING: the update of {pr2_id.display_name} to {pr2_id.head} has caused a \ @{users['user']} @{users['reviewer']} WARNING: the update of {pr2_id.display_name} to {pr2_id.head} has caused a \
conflict in this pull request, data may have been lost. conflict in this pull request, data may have been lost.
stdout: stdout:
```.*? ```
CONFLICT \(add/add\): Merge conflict in h.*? Auto-merging h
CONFLICT (add/add): Merge conflict in h
``` ```
stderr: stderr:
``` ```
\\d{{2}}:\\d{{2}}:\\d{{2}}.\\d+ .* {pr2_id.head} $$:$$:$$.$$ {pr2_id.head}
error: could not apply [0-9a-f]+\\.\\.\\. newfiles error: could not apply $$... newfiles
''', re.DOTALL)) hint: $$
----------
status:
'''))
] ]

View File

@ -42,18 +42,26 @@ def _simple_init(repo):
prx = repo.make_pr(title='title', body='body', target='master', head=c2) prx = repo.make_pr(title='title', body='body', target='master', head=c2)
return prx return prx
class re_matches: class matches(str):
# necessary so str.__new__ does not freak out on `flags`
def __new__(cls, pattern, flags=0):
return super().__new__(cls, pattern)
def __init__(self, pattern, flags=0): def __init__(self, pattern, flags=0):
self._r = re.compile(pattern, flags) p, n = re.subn(
# `re.escape` will escape the `$`, so we need to handle that...
# maybe it should not be $?
r'\\\$(\w*?)\\\$',
lambda m: f'(?P<{m[1]}>.*?)' if m[1] else '(.*?)',
re.escape(self),
)
assert n, f"matches' pattern should have at least one placeholder, found none in\n{pattern}"
self._r = re.compile(p, flags | re.DOTALL)
def __eq__(self, text): def __eq__(self, text):
return self._r.match(text) if not isinstance(text, str):
return NotImplemented
def __str__(self): return self._r.search(text)
return re.sub(r'\\(.)', r'\1', self._r.pattern)
def __repr__(self):
return repr(str(self))
def seen(env, pr, users): def seen(env, pr, users):
url = to_pr(env, pr).url url = to_pr(env, pr).url

View File

@ -10,7 +10,7 @@ import requests
from lxml import html from lxml import html
import odoo import odoo
from utils import _simple_init, seen, re_matches, get_partner, Commit, pr_page, to_pr, part_of, ensure_one from utils import _simple_init, seen, matches, get_partner, Commit, pr_page, to_pr, part_of, ensure_one
@pytest.fixture @pytest.fixture
@ -174,7 +174,7 @@ def test_trivial_flow(env, repo, page, users, config):
('', 'Reviewer'), ('', 'Reviewer'),
]), ]),
# staging succeeded # staging succeeded
(re_matches(r'.*'), f'<p>staging {st.id} succeeded</p>', [ (matches('$$'), f'<p>staging {st.id} succeeded</p>', [
# set merge date # set merge date
(False, pr_id.merge_date + 'Z'), (False, pr_id.merge_date + 'Z'),
# updated state # updated state
@ -676,7 +676,7 @@ def test_ff_failure(env, repo, config, page):
_new, prev = doc.cssselect('li.staging') _new, prev = doc.cssselect('li.staging')
assert 'bg-gray-lighter' in prev.classes, "ff failure is ~ cancelling" assert 'bg-gray-lighter' in prev.classes, "ff failure is ~ cancelling"
assert prev.get('title') == re_matches('fast forward failed \(update is not a fast forward\)') assert 'fast forward failed (update is not a fast forward)' in prev.get('title')
assert env['runbot_merge.pull_requests'].search([ assert env['runbot_merge.pull_requests'].search([
('repository.name', '=', repo.name), ('repository.name', '=', repo.name),
@ -1029,7 +1029,7 @@ def test_rebase_failure(env, repo, users, config):
assert pr_a.comments == [ assert pr_a.comments == [
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+'),
seen(env, pr_a, users), seen(env, pr_a, users),
(users['user'], re_matches(r'^Unable to stage PR')), (users['user'], matches('Unable to stage PR')),
] ]
assert pr_b.comments == [ assert pr_b.comments == [
(users['reviewer'], 'hansen r+'), (users['reviewer'], 'hansen r+'),
@ -3968,7 +3968,7 @@ class TestInfrastructure:
assert repo.get_ref('heads/master') == m1 assert repo.get_ref('heads/master') == m1
def node(name, *children): def node(name, *children):
assert type(name) in (str, re_matches) assert type(name) in (str, matches)
return name, frozenset(children) return name, frozenset(children)
def log_to_node(log): def log_to_node(log):
log = list(log) log = list(log)