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 <[email protected]> --- v2: - Added test cases. - Rebased on main. --- tests/checkpatch.at | 65 +++++++++++++++++++++++++++++++++++++++++ utilities/checkpatch.py | 40 +++++++++++++++++++++++++ 2 files changed, 105 insertions(+) diff --git a/tests/checkpatch.at b/tests/checkpatch.at index 6ac0e51f3..b874b1ea6 100755 --- a/tests/checkpatch.at +++ b/tests/checkpatch.at @@ -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 diff --git a/utilities/checkpatch.py b/utilities/checkpatch.py index 35204daa2..a7360e47a 100755 --- a/utilities/checkpatch.py +++ b/utilities/checkpatch.py @@ -203,6 +203,8 @@ __regex_if_macros = re.compile(r'^ +(%s) \([\S]([\s\S]+[\S])*\) { +\\' % __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 @@ -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 @@ -668,6 +685,18 @@ checks = [ 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.")}, ] @@ -899,6 +928,7 @@ 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 @@ -906,6 +936,16 @@ def ovs_checkpatch_parse(text, filename, author=None, committer=None): 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 -- 2.47.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
