From 159ff6e08e41de7744b03c70db5151083ead7102 Mon Sep 17 00:00:00 2001 From: Angus Gratton Date: Wed, 26 Sep 2018 10:17:46 +1000 Subject: [PATCH] unit tests: Only initialise tcpip_adapter() when needed by the test Prevents unexpected memory allocations when running tests which don't require tcpip_adapter. --- components/driver/test/test_adc2.c | 5 +++- .../esp32/test/{test_esp32.c => test_wifi.c} | 5 +++- .../http_server/test/test_http_server.c | 27 ++++++++++++++--- components/vfs/test/test_vfs_select.c | 6 +++- .../components/unity/include/test_utils.h | 26 ++++++++++++++++ .../components/unity/test_utils.c | 30 +++++++++++++++++++ .../components/unity/unity_platform.c | 17 +++++++---- tools/unit-test-app/main/app_main.c | 5 ---- 8 files changed, 103 insertions(+), 18 deletions(-) rename components/esp32/test/{test_esp32.c => test_wifi.c} (98%) diff --git a/components/driver/test/test_adc2.c b/components/driver/test/test_adc2.c index ec6e2ff4f..40549428e 100644 --- a/components/driver/test/test_adc2.c +++ b/components/driver/test/test_adc2.c @@ -10,6 +10,7 @@ #include "esp_wifi.h" #include "esp_log.h" #include "nvs_flash.h" +#include "test_utils.h" static const char* TAG = "test_adc2"; @@ -44,7 +45,9 @@ TEST_CASE("adc2 work with wifi","[adc]") { int read_raw; int target_value; - + + test_case_uses_tcpip(); + //adc and dac init TEST_ESP_OK( dac_output_enable( DAC_CHANNEL_1 )); TEST_ESP_OK( dac_output_enable( DAC_CHANNEL_2 )); diff --git a/components/esp32/test/test_esp32.c b/components/esp32/test/test_wifi.c similarity index 98% rename from components/esp32/test/test_esp32.c rename to components/esp32/test/test_wifi.c index 82e24b12c..ec35ce5ad 100644 --- a/components/esp32/test/test_esp32.c +++ b/components/esp32/test/test_wifi.c @@ -9,6 +9,7 @@ #include "esp_wifi_types.h" #include "esp_log.h" #include "nvs_flash.h" +#include "test_utils.h" static const char* TAG = "test_wifi"; @@ -79,7 +80,9 @@ static void test_wifi_start_stop(wifi_init_config_t *cfg, wifi_config_t* wifi_co } TEST_CASE("wifi stop and deinit","[wifi]") -{ +{ + test_case_uses_tcpip(); + wifi_init_config_t cfg = WIFI_INIT_CONFIG_DEFAULT(); wifi_config_t wifi_config = { .sta = { diff --git a/components/http_server/test/test_http_server.c b/components/http_server/test/test_http_server.c index 7c7eb6a05..07c91add5 100644 --- a/components/http_server/test/test_http_server.c +++ b/components/http_server/test/test_http_server.c @@ -18,6 +18,7 @@ #include #include "unity.h" +#include "test_utils.h" int pre_start_mem, post_stop_mem, post_stop_min_mem; bool basic_sanity = true; @@ -115,24 +116,39 @@ httpd_handle_t test_httpd_start(uint16_t id) TEST_CASE("Leak Test", "[HTTP SERVER]") { httpd_handle_t hd[SERVER_INSTANCES]; - unsigned task_count = uxTaskGetNumberOfTasks(); - pre_start_mem = esp_get_free_heap_size(); + unsigned task_count; bool res = true; + test_case_uses_tcpip(); + + task_count = uxTaskGetNumberOfTasks(); + printf("Initial task count: %d\n", task_count); + + pre_start_mem = esp_get_free_heap_size(); + for (int i = 0; i < SERVER_INSTANCES; i++) { hd[i] = test_httpd_start(i); vTaskDelay(10); - if (uxTaskGetNumberOfTasks() != ++task_count) { + unsigned num_tasks = uxTaskGetNumberOfTasks(); + task_count++; + if (num_tasks != task_count) { + printf("Incorrect task count (starting): %d expected %d\n", + num_tasks, task_count); res = false; } } for (int i = 0; i < SERVER_INSTANCES; i++) { if (httpd_stop(hd[i]) != ESP_OK) { + printf("Failed to stop httpd task %d\n", i); res = false; } vTaskDelay(10); - if (uxTaskGetNumberOfTasks() != --task_count) { + unsigned num_tasks = uxTaskGetNumberOfTasks(); + task_count--; + if (num_tasks != task_count) { + printf("Incorrect task count (stopping): %d expected %d\n", + num_tasks, task_count); res = false; } } @@ -144,6 +160,9 @@ TEST_CASE("Basic Functionality Tests", "[HTTP SERVER]") { httpd_handle_t hd; httpd_config_t config = HTTPD_DEFAULT_CONFIG(); + + test_case_uses_tcpip(); + TEST_ASSERT(httpd_start(&hd, &config) == ESP_OK); test_handler_limit(hd); TEST_ASSERT(httpd_stop(hd) == ESP_OK); diff --git a/components/vfs/test/test_vfs_select.c b/components/vfs/test/test_vfs_select.c index f9927a54f..8ec6794f4 100644 --- a/components/vfs/test/test_vfs_select.c +++ b/components/vfs/test/test_vfs_select.c @@ -24,6 +24,7 @@ #include "esp_vfs_dev.h" #include "lwip/sockets.h" #include "lwip/netdb.h" +#include "test_utils.h" typedef struct { int fd; @@ -114,6 +115,8 @@ static inline void start_task(const test_task_param_t *test_task_param) static void init(int *uart_fd, int *socket_fd) { + test_case_uses_tcpip(); + uart1_init(); UART1.conf0.loopback = 1; @@ -296,9 +299,10 @@ TEST_CASE("concurent selects work", "[vfs]") }; int uart_fd, socket_fd; - const int dummy_socket_fd = open_dummy_socket(); init(&uart_fd, &socket_fd); + const int dummy_socket_fd = open_dummy_socket(); + fd_set rfds; FD_ZERO(&rfds); FD_SET(uart_fd, &rfds); diff --git a/tools/unit-test-app/components/unity/include/test_utils.h b/tools/unit-test-app/components/unity/include/test_utils.h index 746d94a08..68e8e81d7 100644 --- a/tools/unit-test-app/components/unity/include/test_utils.h +++ b/tools/unit-test-app/components/unity/include/test_utils.h @@ -43,6 +43,32 @@ void ref_clock_deinit(); */ uint64_t ref_clock_get(); + +/** + * @brief Reset automatic leak checking which happens in unit tests. + * + * Updates recorded "before" free memory values to the free memory values + * at time of calling. Resets leak checker if tracing is enabled in + * config. + * + * This can be called if a test case does something which allocates + * memory on first use, for example. + * + * @note Use with care as this can mask real memory leak problems. + */ +void unity_reset_leak_checks(void); + + +/** + * @brief Call this function from a test case which requires TCP/IP or + * LWIP functionality. + * + * @note This should be the first function the test case calls, as it will + * allocate memory on first use (and also reset the test case leak checker). + */ +void test_case_uses_tcpip(void); + + /** * @brief wait for signals. * diff --git a/tools/unit-test-app/components/unity/test_utils.c b/tools/unit-test-app/components/unity/test_utils.c index 01176a816..36aae4c29 100644 --- a/tools/unit-test-app/components/unity/test_utils.c +++ b/tools/unit-test-app/components/unity/test_utils.c @@ -17,6 +17,10 @@ #include "test_utils.h" #include "rom/ets_sys.h" #include "rom/uart.h" +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" +#include "tcpip_adapter.h" +#include "lwip/sockets.h" const esp_partition_t *get_test_data_partition() { @@ -41,6 +45,32 @@ static void wait_user_control() } } +void test_case_uses_tcpip() +{ + // Can be called more than once, does nothing on subsequent calls + tcpip_adapter_init(); + + // Allocate all sockets then free them + // (First time each socket is allocated some one-time allocations happen.) + int sockets[CONFIG_LWIP_MAX_SOCKETS]; + for (int i = 0; i < CONFIG_LWIP_MAX_SOCKETS; i++) { + int type = (i % 2 == 0) ? SOCK_DGRAM : SOCK_STREAM; + int family = (i % 3 == 0) ? PF_INET6 : PF_INET; + sockets[i] = socket(family, type, IPPROTO_IP); + } + for (int i = 0; i < CONFIG_LWIP_MAX_SOCKETS; i++) { + close(sockets[i]); + } + + // Allow LWIP tasks to finish initialising themselves + vTaskDelay(25 / portTICK_RATE_MS); + + printf("Note: tcpip_adapter_init() has been called. Until next reset, TCP/IP task will periodicially allocate memory and consume CPU time.\n"); + + // Reset the leak checker as LWIP allocates a lot of memory on first run + unity_reset_leak_checks(); +} + // signal functions, used for sync between unity DUTs for multiple devices cases void unity_wait_for_signal(const char* signal_name) { diff --git a/tools/unit-test-app/components/unity/unity_platform.c b/tools/unit-test-app/components/unity/unity_platform.c index 6beb85cb9..02fcd887b 100644 --- a/tools/unit-test-app/components/unity/unity_platform.c +++ b/tools/unit-test-app/components/unity/unity_platform.c @@ -37,6 +37,16 @@ static size_t before_free_32bit; const size_t WARN_LEAK_THRESHOLD = 256; const size_t CRITICAL_LEAK_THRESHOLD = 4096; +void unity_reset_leak_checks(void) +{ + before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT); + before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT); + +#ifdef CONFIG_HEAP_TRACING + heap_trace_start(HEAP_TRACE_LEAKS); +#endif +} + /* setUp runs before every test */ void setUp(void) { @@ -54,12 +64,7 @@ void setUp(void) printf("%s", ""); /* sneakily lazy-allocate the reent structure for this test task */ get_test_data_partition(); /* allocate persistent partition table structures */ - before_free_8bit = heap_caps_get_free_size(MALLOC_CAP_8BIT); - before_free_32bit = heap_caps_get_free_size(MALLOC_CAP_32BIT); - -#ifdef CONFIG_HEAP_TRACING - heap_trace_start(HEAP_TRACE_LEAKS); -#endif + unity_reset_leak_checks(); } static void check_leak(size_t before_free, size_t after_free, const char *type) diff --git a/tools/unit-test-app/main/app_main.c b/tools/unit-test-app/main/app_main.c index 1dbcdd3b0..73a8201ff 100644 --- a/tools/unit-test-app/main/app_main.c +++ b/tools/unit-test-app/main/app_main.c @@ -3,7 +3,6 @@ #include "freertos/task.h" #include "unity.h" #include "unity_config.h" -#include "tcpip_adapter.h" void unityTask(void *pvParameters) { @@ -13,10 +12,6 @@ void unityTask(void *pvParameters) void app_main() { - // TCP/IP adapter is initialized here because it leaks memory so the - // initialization in test cases would make the test fail because of leak. - tcpip_adapter_init(); - // Note: if unpinning this task, change the way run times are calculated in // unity_platform xTaskCreatePinnedToCore(unityTask, "unityTask", UNITY_FREERTOS_STACK_SIZE, NULL,