From 5a342e8a4b3e1fa4c0af14a40da2c4ebfcb85d1a Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Mon, 25 Nov 2024 11:12:32 +0100 Subject: [PATCH 1/2] tests/sys/shell: fix invalid escaping in test This fixes: /home/marian.buschsieweke@ml-pa.loc/Repos/software/RIOT/master/tests/sys/shell/tests/01-run.py:86: SyntaxWarning: invalid escape sequence '\e' Python still assumes `\\` if `\` is not valid, but started to warn. Chances are good that this will be elevated to an error in future python releases. --- tests/sys/shell/tests/01-run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sys/shell/tests/01-run.py b/tests/sys/shell/tests/01-run.py index faa79a3859..b0f21091d0 100755 --- a/tests/sys/shell/tests/01-run.py +++ b/tests/sys/shell/tests/01-run.py @@ -83,7 +83,7 @@ CMDS = ( ('echo escaped\\ space', '"echo""escaped space"'), ('echo escape within \'\\s\\i\\n\\g\\l\\e\\q\\u\\o\\t\\e\'', '"echo""escape""within""singlequote"'), ('echo escape within "\\d\\o\\u\\b\\l\\e\\q\\u\\o\\t\\e"', '"echo""escape""within""doublequote"'), - ("""echo "t\e st" "\\"" '\\'' a\ b""", '"echo""te st"""""\'""a b"'), # noqa: W605 + ("""echo "t\\e st" "\\"" '\\'' a\ b""", '"echo""te st"""""\'""a b"'), # noqa: W605 # test correct quoting ('echo "hello"world', '"echo""helloworld"'), From b858614fecb1805bf9d2446b66bf3daa9092363d Mon Sep 17 00:00:00 2001 From: Marian Buschsieweke Date: Mon, 25 Nov 2024 11:43:47 +0100 Subject: [PATCH 2/2] tests/sys/shell: make test more robust This changes the behavior of the test script for verifying the `help` command: It no longer assumes a specific order for the list of commands. Making the test robust is a bit tricky, as the module `shell_cmds_default` that is used here may add commands specific to a set of board. We use `help_json` to get the list of commands actually provided, so that we know how many rows the command table printed by `help` need to be parsed. A minimum set of commands that *must* be present for all boards is hard-coded in the test script and the actually provided commands are tested against this. Otherwise e.g. an empty list of commands presented by `help` and `help_json` would also pass. Co-authored-by: benpicco --- tests/sys/shell/Makefile | 3 + tests/sys/shell/tests/01-run.py | 103 ++++++++++++++++++++++++++------ 2 files changed, 87 insertions(+), 19 deletions(-) diff --git a/tests/sys/shell/Makefile b/tests/sys/shell/Makefile index 9258c81784..eb0c8ad315 100644 --- a/tests/sys/shell/Makefile +++ b/tests/sys/shell/Makefile @@ -6,6 +6,9 @@ USEMODULE += shell_cmds_default USEMODULE += ps USEMODULE += ztimer_msec +# JSON help is needed by test script +USEMODULE += shell_builtin_cmd_help_json + # Use a terminal that does not introduce extra characters into the stream. RIOT_TERMINAL ?= socat diff --git a/tests/sys/shell/tests/01-run.py b/tests/sys/shell/tests/01-run.py index b0f21091d0..a817fce269 100755 --- a/tests/sys/shell/tests/01-run.py +++ b/tests/sys/shell/tests/01-run.py @@ -6,28 +6,30 @@ # General Public License v2.1. See the file LICENSE in the top level # directory for more details. -import sys +import json import os +import sys from testrunner import run -EXPECTED_HELP = ( - 'Command Description', - '---------------------------------------', - 'bufsize Get the shell\'s buffer size', - 'start_test starts a test', - 'end_test ends a test', - 'echo prints the input command', - 'empty print nothing on command', - 'periodic periodically print command', - 'app_metadata Returns application metadata', - 'pm interact with layered PM subsystem', - 'ps Prints information about running threads.', - 'reboot Reboot the node', - 'version Prints current RIOT_VERSION', - 'xfa_test1 xfa test command 1', - 'xfa_test2 xfa test command 2' -) +# This is the minimum subset of commands expected to be available on all +# boards. The test will still pass if additional commands are present, as +# `shell_cmds_default` may pull in board specific commands. +EXPECTED_CMDS = { + 'bufsize': 'Get the shell\'s buffer size', + 'start_test': 'starts a test', + 'end_test': 'ends a test', + 'echo': 'prints the input command', + 'empty': 'print nothing on command', + 'periodic': 'periodically print command', + 'app_metadata': 'Returns application metadata', + 'pm': 'interact with layered PM subsystem', + 'ps': 'Prints information about running threads.', + 'reboot': 'Reboot the node', + 'version': 'Prints current RIOT_VERSION', + 'xfa_test1': 'xfa test command 1', + 'xfa_test2': 'xfa test command 2', +} EXPECTED_PS = ( '\tpid | state Q | pri', @@ -103,7 +105,6 @@ CMDS = ( # test default commands ('ps', EXPECTED_PS), - ('help', EXPECTED_HELP), # test commands added to shell_commands_xfa ('xfa_test1', '[XFA TEST 1 OK]'), @@ -151,6 +152,68 @@ def check_cmd(child, cmd, expected): child.expect_exact(line) +def check_help(child): + """ + Runs the `help_json` and `help` command to check if the list of commands + and descriptions match and contain a list of expected commands. + """ + + # Run help_json to get the list of commands present + child.expect(PROMPT) + child.sendline('help_json') + # expect JSON object as response the covers the whole line + child.expect(r"(\{[^\n\r]*\})\r\n") + + # use a set to track which expected commands were already covered + cmds_expected = set(EXPECTED_CMDS) + + # record actually present commands (which may be more than the expected + # ones) and their descriptions in here + cmds_actual = set() + desc_actual = {} + + # parse the commands and iterate over the list + cmdlist = json.loads(child.match.group(1))["cmds"] + for item in cmdlist: + # for expected commands, validate the description and ensure they + # are listed exactly once + if item['cmd'] in EXPECTED_CMDS: + assert item['cmd'] in cmds_expected, f"command {item['cmd']} listed twice" + assert item['desc'] == EXPECTED_CMDS[item['cmd']], f"description of {item['cmd']} not expected" + cmds_expected.remove(item['cmd']) + + # populate the list of actually present commands and their description + cmds_actual.add(item['cmd']) + desc_actual[item['cmd']] = item['desc'] + + assert len(cmds_expected) == 0, f"commands {cmds_expected} missing" + + # Now: Run regular help and expect the same commands as help_json + child.expect(PROMPT) + child.sendline('help') + + # expect the header first + child.expect_exact('Command Description\r\n') + child.expect_exact('---------------------------------------\r\n') + + # loop until the set of actually present commands assembled from the JSON + # is empty. We remove each command from the set when we process it, so that + # we can detect duplicates + while len(cmds_actual) > 0: + # parse line into command and description + child.expect(r"([a-z0-9_-]*)[ \t]*(.*)\r\n") + cmd = child.match.group(1) + desc = child.match.group(2) + + # expect the command to be in the set got from the JSON. Then remove + # it, so that a duplicated line would trigger the assert + assert cmd in cmds_actual, f"Command \"{cmd}\" unexpected or listed twice in help" + cmds_actual.remove(cmd) + + # description should match the one got from JSON + assert desc == desc_actual[cmd], f"Description for \"{cmd}\" not matching" + + def check_startup(child): child.sendline(CONTROL_C) child.expect_exact(PROMPT) @@ -247,6 +310,8 @@ def testfunc(child): check_cmd(child, cmd, expected) + check_help(child) + if RIOT_TERMINAL in CLEANTERMS: for cmd, expected in CMDS_CLEANTERM: check_cmd(child, cmd, expected)