Be more liberal with Depends-On: parsing
I just spent quite a while debugging only to realise I'd written "Depends-On: https://" (note the two spaces...). This gets passed to urlparse.parse.urlparse() which does not handle leading spaces; thus the return is considered invalid and the Depends-On gets silently ignored. It seems we should just be liberal with leading and trailing space around Depends-On: lines. This strips the results to acheive this, and adds a small unit-test to cover this. Change-Id: Iddd8363580fb2fefd99d4783fc90f68f34f0c949
This commit is contained in:
parent
ac8765eb92
commit
f77fdd9327
47
tests/unit/test_dependson.py
Normal file
47
tests/unit/test_dependson.py
Normal file
@ -0,0 +1,47 @@
|
||||
# Copyright 2021 Red Hat
|
||||
#
|
||||
# Licensed under the Apache License, Version 2.0 (the "License"); you may
|
||||
# not use this file except in compliance with the License. You may obtain
|
||||
# a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing, software
|
||||
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
|
||||
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
|
||||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
# NOTE(ianw): 2021-08-30 we are testing whitespace things that trigger
|
||||
# flake8, for now there is no way to turn of specific tests on a
|
||||
# per-file basis.
|
||||
|
||||
# flake8: noqa
|
||||
|
||||
import textwrap
|
||||
|
||||
from zuul.lib.dependson import find_dependency_headers
|
||||
|
||||
from tests.base import BaseTestCase
|
||||
|
||||
|
||||
class TestDependsOnParsing(BaseTestCase):
|
||||
|
||||
def test_depends_on_parsing(self):
|
||||
msg = textwrap.dedent('''\
|
||||
This is a sample review subject
|
||||
|
||||
Review text
|
||||
|
||||
Depends-On:https://this.is.a.url/1
|
||||
Depends-On: https://this.is.a.url/2
|
||||
Depends-On: https://this.is.a.url/3
|
||||
Depends-On: https://this.is.a.url/4
|
||||
''')
|
||||
r = find_dependency_headers(msg)
|
||||
|
||||
self.assertListEqual(r,
|
||||
['https://this.is.a.url/1',
|
||||
'https://this.is.a.url/2',
|
||||
'https://this.is.a.url/3',
|
||||
'https://this.is.a.url/4'])
|
@ -15,7 +15,7 @@
|
||||
import re
|
||||
|
||||
|
||||
DEPENDS_ON_RE = re.compile(r"^Depends-On: (.*?)\s*$",
|
||||
DEPENDS_ON_RE = re.compile(r"^Depends-On:(.*?)\s*$",
|
||||
re.MULTILINE | re.IGNORECASE)
|
||||
|
||||
|
||||
@ -25,5 +25,5 @@ def find_dependency_headers(message):
|
||||
for match in DEPENDS_ON_RE.findall(message):
|
||||
if match in dependencies:
|
||||
continue
|
||||
dependencies.append(match)
|
||||
dependencies.append(match.strip())
|
||||
return dependencies
|
||||
|
Loading…
x
Reference in New Issue
Block a user