Merge branch 'contrib/github_pr_12525' into 'master'

change(console): print sorted help (GitHub PR)

Closes IDFGH-11382

See merge request espressif/esp-idf!27022

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;

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
+```

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,6 +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));
+
+    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));

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('aaa', timeout=5)
-    dut.expect_exact('Quit REPL environment', timeout=5)
+    dut.expect_exact('should appear first in help', timeout=5)
 
     dut.expect(r'help\s+\[<string>\]', 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'<string>\s+Name of command\s+esp>', timeout=5)
+    dut.expect(r'<string>\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.parametrize(
-@pytest.mark.supported_targets
+    'test_on', [
-def test_console(dut: Dut) -> None:
+        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.parametrize(
-@pytest.mark.supported_targets
+    'test_on', [
-def test_console_repl(dut: Dut) -> None:
+        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.parametrize(
-@pytest.mark.supported_targets
+    'test_on', [
-def test_console_help_generic(dut: Dut) -> None:
+        pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]),
-    do_test_help_generic(dut)
+        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.parametrize(
-@pytest.mark.supported_targets
+    'test_on', [
-def test_console_help_quit(dut: Dut) -> None:
+        pytest.param('target', marks=[pytest.mark.supported_targets, pytest.mark.generic]),
-    do_test_help_quit(dut)
+        pytest.param('qemu', marks=[pytest.mark.esp32, pytest.mark.host_test, pytest.mark.qemu]),
-
+    ]
-
+)
-@pytest.mark.host_test
+def test_console_help_reverse_registration(dut: Dut, test_on: str) -> None:
-@pytest.mark.qemu
+    do_test_help_generic(dut, 'reverse')
-@pytest.mark.esp32
+
-@pytest.mark.esp32c3
+
-def test_console_qemu(dut: Dut) -> None:
+@pytest.mark.parametrize(
-    dut.run_all_single_board_cases()
+    '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]),
-@pytest.mark.host_test
+    ]
-@pytest.mark.qemu
+)
-@pytest.mark.esp32
+def test_console_help_quit(dut: Dut, test_on: str) -> None:
-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:
     do_test_help_quit(dut)