From 921b2a6331c157a0084299e38eecd449bc56aae7 Mon Sep 17 00:00:00 2001
From: David Cermak <cermak@espressif.com>
Date: Thu, 20 Jun 2024 09:27:18 +0200
Subject: [PATCH] fix(esp_netif): Add unit test for loopback configuration

Split tests into common and lwip/loopback specific and adds loopback
tests as separate configuration.
---
 .../esp_netif/loopback/esp_netif_loopback.c   |  4 +
 .../test_app_esp_netif/main/CMakeLists.txt    | 16 ++--
 .../test_app_esp_netif/main/esp_netif_test.c  | 72 ++++++++++++++++++
 .../test_app_esp_netif/main/esp_netif_test.h  | 14 ++++
 .../main/esp_netif_test_loopback.c            | 49 ++++++++++++
 .../main/esp_netif_test_lwip.c                | 75 ++++++++-----------
 .../test_app_esp_netif/pytest_esp_netif.py    |  1 +
 .../sdkconfig.ci.dns_per_netif                |  2 +
 .../sdkconfig.ci.global_dns                   |  2 +
 .../test_app_esp_netif/sdkconfig.ci.loopback  |  2 +
 10 files changed, 190 insertions(+), 47 deletions(-)
 create mode 100644 components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.c
 create mode 100644 components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.h
 create mode 100644 components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_loopback.c
 create mode 100644 components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.loopback

diff --git a/components/esp_netif/loopback/esp_netif_loopback.c b/components/esp_netif/loopback/esp_netif_loopback.c
index 49a400567e..28fd7cabab 100644
--- a/components/esp_netif/loopback/esp_netif_loopback.c
+++ b/components/esp_netif/loopback/esp_netif_loopback.c
@@ -480,4 +480,8 @@ esp_err_t esp_netif_tcpip_exec(esp_netif_callback_fn fn, void*ctx)
     return fn(ctx);
 }
 
+esp_netif_t *esp_netif_get_handle_from_ifkey(const char *if_key)
+{
+    return esp_netif_get_handle_from_ifkey_unsafe(if_key);
+}
 #endif /* CONFIG_ESP_NETIF_LOOPBACK */
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/main/CMakeLists.txt b/components/esp_netif/test_apps/test_app_esp_netif/main/CMakeLists.txt
index cd1df185c2..e2354e125e 100644
--- a/components/esp_netif/test_apps/test_app_esp_netif/main/CMakeLists.txt
+++ b/components/esp_netif/test_apps/test_app_esp_netif/main/CMakeLists.txt
@@ -1,5 +1,11 @@
-idf_component_register(SRCS "esp_netif_test.c"
-                       REQUIRES test_utils
-                       INCLUDE_DIRS "."
-                       PRIV_INCLUDE_DIRS "$ENV{IDF_PATH}/components/esp_netif/private_include" "."
-                       PRIV_REQUIRES unity esp_netif nvs_flash esp_wifi)
+if(CONFIG_ESP_NETIF_TCPIP_LWIP)
+    set(srcs_test_stack esp_netif_test_lwip.c)
+elseif(CONFIG_ESP_NETIF_LOOPBACK)
+    set(srcs_test_stack esp_netif_test_loopback.c)
+endif()
+
+idf_component_register(SRCS esp_netif_test.c ${srcs_test_stack}
+                   REQUIRES test_utils
+                   INCLUDE_DIRS "."
+                   PRIV_INCLUDE_DIRS "$ENV{IDF_PATH}/components/esp_netif/private_include" "."
+                   PRIV_REQUIRES unity esp_netif nvs_flash esp_wifi)
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.c b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.c
new file mode 100644
index 0000000000..c8ba023ab4
--- /dev/null
+++ b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.c
@@ -0,0 +1,72 @@
+/*
+ * SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+#include <stdio.h>
+#include <string.h>
+#include "unity.h"
+#include "unity_fixture.h"
+#include "esp_netif.h"
+#include "esp_netif_sntp.h"
+#include "esp_netif_net_stack.h"
+#include "esp_wifi.h"
+#include "nvs_flash.h"
+#include "esp_wifi_netif.h"
+#include "sdkconfig.h"
+#include "test_utils.h"
+#include "memory_checks.h"
+#include "lwip/netif.h"
+
+//// This is a private esp-netif API, but include here to test it
+bool esp_netif_is_netif_listed(esp_netif_t *esp_netif);
+
+void create_delete_multiple_netifs(void)
+{
+    // interface key has to be a unique identifier
+    const char* if_keys[] = { "if1", "if2", "if3", "if4", "if5", "if6", "if7", "if8", "if9" };
+    const int nr_of_netifs = sizeof(if_keys)/sizeof(char*);
+    esp_netif_t *netifs[nr_of_netifs];
+
+    // create 10 wifi stations
+    for (int i=0; i<nr_of_netifs; ++i) {
+        esp_netif_inherent_config_t base_netif_config = { .if_key = if_keys[i]};
+        esp_netif_config_t cfg = { .base = &base_netif_config, .stack = ESP_NETIF_NETSTACK_DEFAULT_WIFI_STA };
+        netifs[i] = esp_netif_new(&cfg);
+        TEST_ASSERT_NOT_NULL(netifs[i]);
+    }
+
+    // there's no AP within created stations
+    TEST_ASSERT_EQUAL(NULL, esp_netif_get_handle_from_ifkey("WIFI_AP_DEF"));
+
+    // check that the created netifs are correctly found by their interface keys and globally listed
+    for (int i=0; i<nr_of_netifs; ++i) {
+        TEST_ASSERT_EQUAL(netifs[i], esp_netif_get_handle_from_ifkey(if_keys[i]));
+        TEST_ASSERT_TRUE(esp_netif_is_netif_listed(netifs[i]));
+    }
+
+    // destroy one by one and check it's been removed
+    for (int i=0; i<nr_of_netifs; ++i) {
+        esp_netif_destroy(netifs[i]);
+        TEST_ASSERT_FALSE(esp_netif_is_netif_listed(netifs[i]));
+    }
+}
+
+
+void get_from_if_key(void)
+{
+    // init default netif
+    esp_netif_config_t cfg = ESP_NETIF_DEFAULT_WIFI_STA();
+    esp_netif_t *esp_netif = esp_netif_new(&cfg);
+    TEST_ASSERT_NOT_NULL(esp_netif);
+
+    // check it's accessible by key
+    TEST_ASSERT_EQUAL(esp_netif, esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+
+    // destroy it
+    esp_netif_destroy(esp_netif);
+
+    // check it's also destroyed in list
+    TEST_ASSERT_EQUAL(NULL, esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
+
+}
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.h b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.h
new file mode 100644
index 0000000000..716e88188b
--- /dev/null
+++ b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test.h
@@ -0,0 +1,14 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+#pragma once
+
+#include "unity.h"
+#include "unity_fixture.h"
+#include "esp_netif.h"
+
+// List of tests that are common for both configurations
+void create_delete_multiple_netifs(void);
+void get_from_if_key(void);
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_loopback.c b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_loopback.c
new file mode 100644
index 0000000000..1465b4423a
--- /dev/null
+++ b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_loopback.c
@@ -0,0 +1,49 @@
+/*
+ * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Unlicense OR CC0-1.0
+ */
+#include <stdio.h>
+#include <string.h>
+#include "unity.h"
+#include "unity_fixture.h"
+#include "esp_netif.h"
+#include "memory_checks.h"
+#include "esp_netif_test.h"
+
+TEST_GROUP(esp_netif);
+
+TEST_SETUP(esp_netif)
+{
+    test_utils_record_free_mem();
+    TEST_ESP_OK(test_utils_set_leak_level(0, ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_GENERAL));
+}
+
+TEST_TEAR_DOWN(esp_netif)
+{
+    test_utils_finish_and_evaluate_leaks(test_utils_get_leak_level(ESP_LEAK_TYPE_WARNING, ESP_COMP_LEAK_ALL),
+                                         test_utils_get_leak_level(ESP_LEAK_TYPE_CRITICAL, ESP_COMP_LEAK_ALL));
+}
+
+TEST(esp_netif, create_delete_multiple_netifs)
+{
+    create_delete_multiple_netifs();
+}
+
+TEST(esp_netif, get_from_if_key)
+{
+    get_from_if_key();
+}
+
+TEST_GROUP_RUNNER(esp_netif)
+{
+    RUN_TEST_CASE(esp_netif, create_delete_multiple_netifs)
+    RUN_TEST_CASE(esp_netif, get_from_if_key)
+}
+
+void app_main(void)
+{
+    UNITY_MAIN(esp_netif);
+}
+
+const esp_netif_netstack_config_t *_g_esp_netif_netstack_default_wifi_sta = (esp_netif_netstack_config_t*)1;
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_lwip.c b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_lwip.c
index 080f565ec5..6fe84750b4 100644
--- a/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_lwip.c
+++ b/components/esp_netif/test_apps/test_app_esp_netif/main/esp_netif_test_lwip.c
@@ -17,6 +17,7 @@
 #include "test_utils.h"
 #include "memory_checks.h"
 #include "lwip/netif.h"
+#include "esp_netif_test.h"
 
 TEST_GROUP(esp_netif);
 
@@ -91,20 +92,7 @@ TEST(esp_netif, convert_ip_addresses)
 
 TEST(esp_netif, get_from_if_key)
 {
-    // init default netif
-    esp_netif_config_t cfg = ESP_NETIF_DEFAULT_WIFI_STA();
-    esp_netif_t *esp_netif = esp_netif_new(&cfg);
-    TEST_ASSERT_NOT_NULL(esp_netif);
-
-    // check it's accessible by key
-    TEST_ASSERT_EQUAL(esp_netif, esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
-
-    // destroy it
-    esp_netif_destroy(esp_netif);
-
-    // check it's also destroyed in list
-    TEST_ASSERT_EQUAL(NULL, esp_netif_get_handle_from_ifkey("WIFI_STA_DEF"));
-
+    get_from_if_key();
 }
 
 // This is a private esp-netif API, but include here to test it
@@ -112,34 +100,7 @@ bool esp_netif_is_netif_listed(esp_netif_t *esp_netif);
 
 TEST(esp_netif, create_delete_multiple_netifs)
 {
-    // interface key has to be a unique identifier
-    const char* if_keys[] = { "if1", "if2", "if3", "if4", "if5", "if6", "if7", "if8", "if9" };
-    const int nr_of_netifs = sizeof(if_keys)/sizeof(char*);
-    esp_netif_t *netifs[nr_of_netifs];
-
-    // create 10 wifi stations
-    for (int i=0; i<nr_of_netifs; ++i) {
-        esp_netif_inherent_config_t base_netif_config = { .if_key = if_keys[i]};
-        esp_netif_config_t cfg = { .base = &base_netif_config, .stack = ESP_NETIF_NETSTACK_DEFAULT_WIFI_STA };
-        netifs[i] = esp_netif_new(&cfg);
-        TEST_ASSERT_NOT_NULL(netifs[i]);
-    }
-
-    // there's no AP within created stations
-    TEST_ASSERT_EQUAL(NULL, esp_netif_get_handle_from_ifkey("WIFI_AP_DEF"));
-
-    // check that the created netifs are correctly found by their interface keys and globally listed
-    for (int i=0; i<nr_of_netifs; ++i) {
-        TEST_ASSERT_EQUAL(netifs[i], esp_netif_get_handle_from_ifkey(if_keys[i]));
-        TEST_ASSERT_TRUE(esp_netif_is_netif_listed(netifs[i]));
-    }
-
-    // destroy one by one and check it's been removed
-    for (int i=0; i<nr_of_netifs; ++i) {
-        esp_netif_destroy(netifs[i]);
-        TEST_ASSERT_FALSE(esp_netif_is_netif_listed(netifs[i]));
-    }
-
+    create_delete_multiple_netifs();
 }
 
 static bool desc_matches_with(esp_netif_t *netif, void *ctx)
@@ -147,6 +108,12 @@ static bool desc_matches_with(esp_netif_t *netif, void *ctx)
     return strcmp(ctx, esp_netif_get_desc(netif)) == 0;
 }
 
+/*
+ * This test validates esp_netif_find_if() API by searching in the list of netifs
+ * by their description using the predicate function desc_matches_with() above.
+ * These netifs have the same key and description, so we can use esp_netif_get_handle_from_ifkey()
+ * to validate the test.
+ */
 TEST(esp_netif, find_netifs)
 {
     // Create some interfaces
@@ -181,6 +148,10 @@ TEST(esp_netif, find_netifs)
 }
 
 #ifdef CONFIG_ESP_WIFI_ENABLED
+/*
+ * This test creates a default WiFi station and checks all possible transitions
+ * of the DHCP client used by the station.
+ */
 TEST(esp_netif, dhcp_client_state_transitions_wifi_sta)
 {
     // init default wifi netif
@@ -230,6 +201,10 @@ TEST(esp_netif, dhcp_client_state_transitions_wifi_sta)
 #endif // CONFIG_ESP_WIFI_ENABLED
 
 #if defined(CONFIG_ESP_WIFI_ENABLED) && defined(CONFIG_ESP_WIFI_SOFTAP_SUPPORT)
+/*
+ * This test creates a default WiFi AP and checks all possible transitions
+ * of the DHCP server used by the soft AP.
+ */
 TEST(esp_netif, dhcp_server_state_transitions_wifi_ap)
 {
     // init default wifi netif
@@ -272,6 +247,10 @@ TEST(esp_netif, dhcp_server_state_transitions_wifi_ap)
     nvs_flash_deinit();
 }
 
+/*
+ * This test creates a default mesh interfaces and checks all possible transitions
+ * of the DHCP client and server used by these netifs.
+ */
 TEST(esp_netif, dhcp_server_state_transitions_mesh)
 {
     esp_netif_t *ap = NULL;
@@ -336,6 +315,10 @@ TEST(esp_netif, dhcp_server_state_transitions_mesh)
 #endif // CONFIG_ESP_WIFI_ENABLED && CONFIG_ESP_WIFI_SOFTAP_SUPPORT
 
 #ifdef CONFIG_ESP_WIFI_ENABLED
+/*
+ * This test validates convenience API esp_netif_create_wifi() which creates WiFi station
+ * or API with the specified inherent network config.
+ */
 TEST(esp_netif, create_custom_wifi_interfaces)
 {
     esp_netif_t *ap = NULL;
@@ -449,6 +432,14 @@ static esp_err_t dummy_transmit(void* hd, void *buf, size_t length)
     return ESP_OK;
 }
 
+/*
+ * This test validates the route priority of multiple netifs. It checks that the default route (default netif)
+ * is set correctly for the netifs according to their `route_prio` value and `link_up` state.
+ * - We create 10 netifs with prios: 0, 1, 2, 3, 4, 0, 0, ...., 0 (netifs[nr_of_netifs/2] has max_prio)
+ * - We check the default netif is correct after bringing it down/up, overriding it
+ * - We destroy the default netif and check again
+ * - We destroy the remaining netifs
+ */
 TEST(esp_netif, route_priority)
 {
     test_case_uses_tcpip();
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/pytest_esp_netif.py b/components/esp_netif/test_apps/test_app_esp_netif/pytest_esp_netif.py
index 43c0125637..746316674f 100644
--- a/components/esp_netif/test_apps/test_app_esp_netif/pytest_esp_netif.py
+++ b/components/esp_netif/test_apps/test_app_esp_netif/pytest_esp_netif.py
@@ -10,6 +10,7 @@ from pytest_embedded import Dut
 @pytest.mark.parametrize('config', [
     'global_dns',
     'dns_per_netif',
+    'loopback',             # test config without LWIP
 ], indirect=True)
 def test_esp_netif(dut: Dut) -> None:
     dut.expect_unity_test_output()
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.dns_per_netif b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.dns_per_netif
index 3abbf3a3bc..b7cd513a8e 100644
--- a/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.dns_per_netif
+++ b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.dns_per_netif
@@ -1 +1,3 @@
+CONFIG_ESP_NETIF_TCPIP_LWIP=y
+CONFIG_ESP_NETIF_LOOPBACK=n
 CONFIG_ESP_NETIF_SET_DNS_PER_DEFAULT_NETIF=y
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.global_dns b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.global_dns
index ba627cbf81..61733abdeb 100644
--- a/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.global_dns
+++ b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.global_dns
@@ -1 +1,3 @@
+CONFIG_ESP_NETIF_TCPIP_LWIP=y
+CONFIG_ESP_NETIF_LOOPBACK=n
 CONFIG_ESP_NETIF_SET_DNS_PER_DEFAULT_NETIF=n
diff --git a/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.loopback b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.loopback
new file mode 100644
index 0000000000..ddeaa1295f
--- /dev/null
+++ b/components/esp_netif/test_apps/test_app_esp_netif/sdkconfig.ci.loopback
@@ -0,0 +1,2 @@
+CONFIG_ESP_NETIF_LOOPBACK=y
+CONFIG_ESP_NETIF_TCPIP_LWIP=n