From 07a9137fd769f0da2eda5936cc3e3db3232e8166 Mon Sep 17 00:00:00 2001 From: Alon Bar-Lev Date: Sat, 4 Nov 2023 21:06:45 +0200 Subject: [PATCH 1/2] change(console): print sorted help console commands may be registered in random order in application, for example each module registers its own commands. the output of help is displayed to human, best to have consistent sorted output so that the implementation ordering will not affect the output and allow the user to see a list in some logic order. Signed-off-by: Alon Bar-Lev --- components/console/commands.c | 13 ++++++++----- .../console/test_apps/console/main/test_console.c | 15 +++++++++++++++ 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/components/console/commands.c b/components/console/commands.c index 5e63db87b4..a95be846b6 100644 --- a/components/console/commands.c +++ b/components/console/commands.c @@ -130,14 +130,17 @@ esp_err_t esp_console_cmd_register(const esp_console_cmd_t *cmd) } item->argtable = cmd->argtable; item->func = cmd->func; - cmd_item_t *last = SLIST_FIRST(&s_cmd_list); + cmd_item_t *last = NULL; + cmd_item_t *it; + SLIST_FOREACH(it, &s_cmd_list, next) { + if (strcmp(it->command, item->command) > 0) { + break; + } + last = it; + } if (last == NULL) { SLIST_INSERT_HEAD(&s_cmd_list, item, next); } else { - cmd_item_t *it; - while ((it = SLIST_NEXT(last, next)) != NULL) { - last = it; - } SLIST_INSERT_AFTER(last, item, next); } return ESP_OK; diff --git a/components/console/test_apps/console/main/test_console.c b/components/console/test_apps/console/main/test_console.c index db7685027a..2e5a28cbd5 100644 --- a/components/console/test_apps/console/main/test_console.c +++ b/components/console/test_apps/console/main/test_console.c @@ -78,6 +78,21 @@ TEST_CASE("esp console help command", "[console][ignore]") TEST_ESP_OK(esp_console_cmd_register(&s_quit_cmd)); TEST_ESP_OK(esp_console_register_help_command()); + const esp_console_cmd_t cmd_a = { + .command = "aaa", + .help = "should appear first in help", + .hint = NULL, + .func = do_hello_cmd, + }; + const esp_console_cmd_t cmd_z = { + .command = "zzz", + .help = "should appear last in help", + .hint = NULL, + .func = do_hello_cmd, + }; + TEST_ESP_OK(esp_console_cmd_register(&cmd_z)); + TEST_ESP_OK(esp_console_cmd_register(&cmd_a)); + TEST_ESP_OK(esp_console_start_repl(s_repl)); vTaskDelay(pdMS_TO_TICKS(5000)); } From dad563cfaf595f82e8ea14a56de9584b00fb5b70 Mon Sep 17 00:00:00 2001 From: Jakob Hasse Date: Fri, 10 Nov 2023 16:53:01 +0800 Subject: [PATCH 2/2] change(console): changed unit tests according to sorted help Merges https://github.com/espressif/esp-idf/pull/12525 --- .../console/test_apps/console/README.md | 6 ++ .../test_apps/console/main/test_console.c | 56 +++++++--- .../test_apps/console/pytest_console.py | 100 +++++++++--------- 3 files changed, 101 insertions(+), 61 deletions(-) diff --git a/components/console/test_apps/console/README.md b/components/console/test_apps/console/README.md index bf47d80ec6..1e72354679 100644 --- a/components/console/test_apps/console/README.md +++ b/components/console/test_apps/console/README.md @@ -1,2 +1,8 @@ | Supported Targets | ESP32 | ESP32-C2 | ESP32-C3 | ESP32-C6 | ESP32-H2 | ESP32-P4 | ESP32-S2 | ESP32-S3 | | ----------------- | ----- | -------- | -------- | -------- | -------- | -------- | -------- | -------- | + +Note: Most of the test cases shouldn't be run manually, but [pytest](https://docs.espressif.com/projects/esp-idf/en/latest/esp32/contribute/esp-idf-tests-with-pytest.html) should be used instead. E.g., to run all test cases on ESP32 using pytest, use: + +``` +pytest --target esp32 -m generic +``` diff --git a/components/console/test_apps/console/main/test_console.c b/components/console/test_apps/console/main/test_console.c index 2e5a28cbd5..e6ae0d92b8 100644 --- a/components/console/test_apps/console/main/test_console.c +++ b/components/console/test_apps/console/main/test_console.c @@ -13,6 +13,17 @@ #include "freertos/FreeRTOS.h" #include "freertos/task.h" +/* + * NOTE: Most of these unit tests DO NOT work standalone. They require pytest to control + * the application and check for correct output. + * E.g., to run the test "esp console help command - reverse registration", type: + * pytest --target esp32 -m "generic" -k test_console_help_reverse_registration + * The pytest test cases are different than the unit test cases here, they can be found + * in the pytest_*.py file in the root directory of this test project. + * For more information on pytest, please refer to + * https://docs.espressif.com/projects/esp-idf/en/latest/esp32/contribute/esp-idf-tests-with-pytest.html. + */ + static int do_hello_cmd(int argc, char **argv) { printf("Hello World\n"); @@ -69,7 +80,20 @@ TEST_CASE("esp console repl test", "[console][ignore]") vTaskDelay(pdMS_TO_TICKS(2000)); } -TEST_CASE("esp console help command", "[console][ignore]") +static const esp_console_cmd_t cmd_a = { + .command = "aaa", + .help = "should appear first in help", + .hint = NULL, + .func = do_hello_cmd, +}; +static const esp_console_cmd_t cmd_z = { + .command = "zzz", + .help = "should appear last in help", + .hint = NULL, + .func = do_hello_cmd, +}; + +TEST_CASE("esp console help command - sorted registration", "[console][ignore]") { esp_console_repl_config_t repl_config = ESP_CONSOLE_REPL_CONFIG_DEFAULT(); esp_console_dev_uart_config_t uart_config = ESP_CONSOLE_DEV_UART_CONFIG_DEFAULT(); @@ -77,21 +101,27 @@ TEST_CASE("esp console help command", "[console][ignore]") TEST_ESP_OK(esp_console_cmd_register(&s_quit_cmd)); TEST_ESP_OK(esp_console_register_help_command()); + TEST_ESP_OK(esp_console_cmd_register(&cmd_a)); + TEST_ESP_OK(esp_console_cmd_register(&cmd_z)); + + TEST_ESP_OK(esp_console_start_repl(s_repl)); + vTaskDelay(pdMS_TO_TICKS(5000)); +} + +/** + * The commands in the 'help'-command's output should be alphabetically sorted, + * regardless of their registration order. + */ +TEST_CASE("esp console help command - reverse registration", "[console][ignore]") +{ + esp_console_repl_config_t repl_config = ESP_CONSOLE_REPL_CONFIG_DEFAULT(); + esp_console_dev_uart_config_t uart_config = ESP_CONSOLE_DEV_UART_CONFIG_DEFAULT(); + TEST_ESP_OK(esp_console_new_repl_uart(&uart_config, &repl_config, &s_repl)); - const esp_console_cmd_t cmd_a = { - .command = "aaa", - .help = "should appear first in help", - .hint = NULL, - .func = do_hello_cmd, - }; - const esp_console_cmd_t cmd_z = { - .command = "zzz", - .help = "should appear last in help", - .hint = NULL, - .func = do_hello_cmd, - }; TEST_ESP_OK(esp_console_cmd_register(&cmd_z)); TEST_ESP_OK(esp_console_cmd_register(&cmd_a)); + TEST_ESP_OK(esp_console_register_help_command()); + TEST_ESP_OK(esp_console_cmd_register(&s_quit_cmd)); TEST_ESP_OK(esp_console_start_repl(s_repl)); vTaskDelay(pdMS_TO_TICKS(5000)); diff --git a/components/console/test_apps/console/pytest_console.py b/components/console/test_apps/console/pytest_console.py index 0dfdd13526..1cbaffea2a 100644 --- a/components/console/test_apps/console/pytest_console.py +++ b/components/console/test_apps/console/pytest_console.py @@ -15,27 +15,34 @@ def do_test_quit(dut: Dut) -> None: dut.expect_exact('ByeBye', timeout=5) -def do_test_help_generic(dut: Dut) -> None: +def do_test_help_generic(dut: Dut, registration_order: str) -> None: dut.expect_exact('Press ENTER to see the list of tests') - dut.write('"esp console help command"') + dut.write('"esp console help command - {} registration"'.format(registration_order)) dut.expect_exact('esp>', timeout=5) dut.write('help') - dut.expect_exact('quit', timeout=5) - dut.expect_exact('Quit REPL environment', timeout=5) + dut.expect_exact('aaa', timeout=5) + dut.expect_exact('should appear first in help', timeout=5) dut.expect(r'help\s+\[\]', timeout=5) # Note: repl seems to do the line breaks by itself, this needs to be adjusted if repl changes its line width dut.expect_exact('Print the summary of all registered commands if no arguments are given,', timeout=5) dut.expect_exact('otherwise print summary of given command.', timeout=5) - dut.expect(r'\s+Name of command\s+esp>', timeout=5) + dut.expect(r'\s+Name of command', timeout=5) + + dut.expect_exact('quit', timeout=5) + dut.expect_exact('Quit REPL environment', timeout=5) + + dut.expect_exact('zzz', timeout=5) + dut.expect_exact('should appear last in help', timeout=5) + dut.expect_exact('esp>', timeout=5) def do_test_help_quit(dut: Dut) -> None: dut.expect_exact('Press ENTER to see the list of tests') - dut.write('"esp console help command"') + dut.write('"esp console help command - sorted registration"') dut.expect_exact('esp>', timeout=5) dut.write('help quit') @@ -43,54 +50,51 @@ def do_test_help_quit(dut: Dut) -> None: dut.expect(r'quit\s+Quit REPL environment\s+esp>', timeout=5) -@pytest.mark.generic -@pytest.mark.supported_targets -def test_console(dut: Dut) -> None: +@pytest.mark.parametrize( + 'test_on', [ + pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]), + pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]), + ] +) +def test_console(dut: Dut, test_on: str) -> None: dut.run_all_single_board_cases() -@pytest.mark.generic -@pytest.mark.supported_targets -def test_console_repl(dut: Dut) -> None: +@pytest.mark.parametrize( + 'test_on', [ + pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]), + pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]), + ] +) +def test_console_repl(dut: Dut, test_on: str) -> None: do_test_quit(dut) -@pytest.mark.generic -@pytest.mark.supported_targets -def test_console_help_generic(dut: Dut) -> None: - do_test_help_generic(dut) +@pytest.mark.parametrize( + 'test_on', [ + pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]), + pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]), + ] +) +def test_console_help_sorted_registration(dut: Dut, test_on: str) -> None: + do_test_help_generic(dut, 'sorted') -@pytest.mark.generic -@pytest.mark.supported_targets -def test_console_help_quit(dut: Dut) -> None: - do_test_help_quit(dut) - - -@pytest.mark.host_test -@pytest.mark.qemu -@pytest.mark.esp32 -@pytest.mark.esp32c3 -def test_console_qemu(dut: Dut) -> None: - dut.run_all_single_board_cases() - - -@pytest.mark.host_test -@pytest.mark.qemu -@pytest.mark.esp32 -def test_console_repl_qemu(dut: Dut) -> None: - do_test_quit(dut) - - -@pytest.mark.host_test -@pytest.mark.qemu -@pytest.mark.esp32 -def test_console_help_generic_qemu(dut: Dut) -> None: - do_test_help_generic(dut) - - -@pytest.mark.host_test -@pytest.mark.qemu -@pytest.mark.esp32 -def test_console_help_quit_qemu(dut: Dut) -> None: +@pytest.mark.parametrize( + 'test_on', [ + pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]), + pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]), + ] +) +def test_console_help_reverse_registration(dut: Dut, test_on: str) -> None: + do_test_help_generic(dut, 'reverse') + + +@pytest.mark.parametrize( + 'test_on', [ + pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]), + pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]), + ] +) +def test_console_help_quit(dut: Dut, test_on: str) -> None: do_test_help_quit(dut)