Skip to content

Commit

Permalink
checkpatch: Ensure ovn-nbctl/sbctl commands have a check.
Browse files Browse the repository at this point in the history
ovn-sbctl and ovn-nbctl commands should have a check in tests, except the
commands "find", "show", "list", "get" and "dump-flows".

Commands displaying an uuid, such as "create" should be prepended by check_uuid.
This will ensure that those commands succeeds and tests behave as expected.

Multiline commands (ending with '\') in tests are now checked as single lines
to be able to detect potential "list", "find", ... commands.

Signed-off-by: Xavier Simonart <xsimonar@redhat.com>
Signed-off-by: 0-day Robot <robot@bytheb.org>
  • Loading branch information
simonartxavier authored and ovsrobot committed Jan 8, 2025
1 parent 19a39a2 commit 7519585
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 0 deletions.
65 changes: 65 additions & 0 deletions tests/checkpatch.at
Original file line number Diff line number Diff line change
Expand Up @@ -644,3 +644,68 @@ try_checkpatch \
"

AT_CLEANUP

AT_SETUP([checkpatch - check and check_uuid])
# Verify that missing check is covered.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+ovn-nbctl lsp-add ls
" \
"WARNING: ovn-nbctl return should be checked Consider adding check or check_uuid in front.
#8 FILE: tests/something.at:1:
ovn-nbctl lsp-add ls
"

# Verify that existing check makes checkpatch happy.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+check ovn-nbctl lsp-add ls
"

# Verify that missing check_uuid is covered.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+ovn-sbctl -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1
" \
"WARNING: ovn-sbctl return should be checked Consider adding check or check_uuid in front.
#8 FILE: tests/something.at:1:
ovn-sbctl -- create FDB mac='00:00:00:00:00:03' dp_key=1 port_key=1
"

# Verify that existing check_uuid makes checkpatch happy.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+check_uuid ovn-sbctl -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1
"

# Verify that missing check for a get command does not cause checkpatch failures.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+ovn-nbctl get Logical-Switch-Port p1 dynamic_addresses
"

# Verify that missing check for a create command causes checkpatch failure - multiline.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+ovn-sbctl \
+ -- create FDB mac='"00\:00\:00\:00\:00\:03"' dp_key=1 port_key=1
" \
"WARNING: ovn-sbctl return should be checked Consider adding check or check_uuid in front.
#8 FILE: tests/something.at:1:
ovn-sbctl + -- create FDB mac='00:00:00:00:00:03' dp_key=1 port_key=1
"

# Verify that missing check for a get command does not cause checkpatch failures - multiline.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+ovn-nbctl \
+ -- get Logical-Switch-Port p1 dynamic_addresses
"

# Verify that we can still store outout of ovn-nbctl w/o check.
try_checkpatch \
"COMMON_PATCH_HEADER([tests/something.at])
+lsp1_uuid=$(ovn-nbctl --bare --columns _uuid find Logical_Switch_Port name=lsp1)
"

AT_CLEANUP
40 changes: 40 additions & 0 deletions utilities/checkpatch.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ def reset_counters():
__regex_nonascii_characters = re.compile("[^\u0000-\u007f]")
__regex_efgrep = re.compile(r'.*[ef]grep.*$')
__regex_hardcoded_table = re.compile(r'.*(table=[0-9]+)|.*(resubmit\(,[0-9]+\))')
__regex_ovn_nbctl = re.compile(r'^\s*ovn-nbctl ')
__regex_ovn_sbctl = re.compile(r'^\s*ovn-sbctl ')

skip_leading_whitespace_check = False
skip_trailing_whitespace_check = False
Expand Down Expand Up @@ -377,6 +379,21 @@ def has_hardcoded_table(line):
resubmit(,<NUMBER>)"""
return __regex_hardcoded_table.match(line) is not None

def has_nbctl_non_checked_commands(line):
"""Return TRUE if the current line is missing check in front of ovn-nbctl,
except for commands containing 'find', 'show', 'list' and 'get'."""
return (__regex_ovn_nbctl.match(line) is not None and
"find" not in line and "show" not in line and
"list" not in line and "get" not in line)

def has_sbctl_non_checked_commands(line):
"""Return TRUE if the current line is missing check in front of ovn-sbctl.
except for commands containing 'find', 'show', 'list' and 'get'."""
return (__regex_ovn_sbctl.match(line) is not None and
"dump-flows" not in line and
"find" not in line and "show" not in line and
"list" not in line and "get" not in line)

def filter_comments(current_line, keep=False):
"""remove all of the c-style comments in a line"""
STATE_NORMAL = 0
Expand Down Expand Up @@ -668,6 +685,18 @@ def empty_return(line):
lambda: print_warning("Use of hardcoded table=<NUMBER> or"
" resubmit=(,<NUMBER>) is discouraged in tests."
" Consider using MACRO instead.")},

{'regex': r'\.at$', 'match_name': None,
'check': lambda x: has_nbctl_non_checked_commands(x),
'print':
lambda: print_warning("ovn-nbctl return should be checked"
" Consider adding check or check_uuid in front.")},

{'regex': r'\.at$', 'match_name': None,
'check': lambda x: has_sbctl_non_checked_commands(x),
'print':
lambda: print_warning("ovn-sbctl return should be checked"
" Consider adding check or check_uuid in front.")},
]


Expand Down Expand Up @@ -899,13 +928,24 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None):

reset_counters()

current_line = ""
for line in text.split("\n"):
if current_file != previous_file:
previous_file = current_file

lineno = lineno + 1
total_line = total_line + 1

if current_file.endswith(".at"):
if line.endswith("\\"):
current_line += line[:-1]
continue
else:
current_line += line

line = current_line
current_line = ""

if line == "\f":
# Form feed
continue
Expand Down

0 comments on commit 7519585

Please sign in to comment.