Merge pull request #206 from david-cermak/bugfix/mdns_unit_tests

fix(mdns): Make unit test executable with pytest

.github/workflows/target-test.yml

@@ -62,48 +62,30 @@ jobs:
       matrix:
         idf_ver: ["latest"]
         idf_target: ["esp32", "esp32s2", "esp32c3"]
-        config: ["eth_custom_netif", "eth_def", "eth_no_ipv6", "eth_socket"]
+        test: [ { app: example, path: "components/mdns/examples" }, { app: unit_test, path: "components/mdns/tests/unit_test" } ]
     runs-on: ubuntu-20.04
     container: espressif/idf:${{ matrix.idf_ver }}
-    env:
-      TEST_DIR: components/mdns/examples
     steps:
       - name: Checkout esp-protocols
         uses: actions/checkout@v3
         with:
           submodules: recursive
-      - name: Build ${{ matrix.example }} with IDF-${{ matrix.idf_ver }} for ${{ matrix.idf_target }} for ${{ matrix.config }}
+      - name: Build ${{ matrix.test.app }} with IDF-${{ matrix.idf_ver }} for ${{ matrix.idf_target }}
-        env:
-          IDF_TARGET: ${{ matrix.idf_target }}
         shell: bash
-        working-directory: ${{ env.TEST_DIR }}
+        working-directory: ${{ matrix.test.path }}
         run: |
+          ${IDF_PATH}/install.sh --enable-pytest
           . ${IDF_PATH}/export.sh
-          rm -rf sdkconfig sdkconfig.defaults build build_${{ matrix.config }}
+          python $IDF_PATH/tools/ci/ci_build_apps.py . --target ${{ matrix.idf_target }} -vv --preserve-all --pytest-app
-          cat sdkconfig.ci.${{ matrix.config }} >> sdkconfig.defaults
+          for dir in `ls -d build_*`; do
-          idf.py set-target ${{ matrix.idf_target }}
+          $GITHUB_WORKSPACE/ci/clean_build_artifacts.sh `pwd`/$dir
-          idf.py build
+          zip -qur artifacts.zip $dir
-          mv build build_${{ matrix.config }}
+          done
-      - name: Merge binaries with IDF-${{ matrix.idf_ver }} for ${{ matrix.config }}
-        working-directory: ${{ env.TEST_DIR }}
-        env:
-          IDF_TARGET: ${{ matrix.idf_target }}
-        shell: bash
-        run: |
-          . ${IDF_PATH}/export.sh
-          cd build_${{ matrix.config }}
-          esptool.py --chip ${{ matrix.idf_target }} merge_bin --fill-flash-size 4MB -o flash_image.bin @flash_args
       - uses: actions/upload-artifact@v3
+        if: ${{ matrix.idf_target }} == "esp32"
         with:
-          name: examples_app_bin_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.config }}
+          name: mdns_bin_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.test.app }}
-          path: |
+          path: ${{ matrix.test.path }}/artifacts.zip
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/bootloader/bootloader.bin
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/partition_table/partition-table.bin
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/*.bin
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/*.elf
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/flasher_args.json
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/config/sdkconfig.h
-            ${{ env.TEST_DIR }}/build_${{ matrix.config }}/config/sdkconfig.json
           if-no-files-found: error
 
   build_asio:
@@ -240,14 +222,12 @@ jobs:
       matrix:
         idf_ver: ["latest"]
         idf_target: ["esp32"]
-        config: ["eth_custom_netif", "eth_def", "eth_no_ipv6", "eth_socket"]
+        test: [ { app: example, path: "components/mdns/examples" }, { app: unit_test, path: "components/mdns/tests/unit_test" } ]
-    name: Run mDNS Example Test on target
+    name: Run mDNS target tests
     needs: build_mdns
     runs-on:
       - self-hosted
       - ESP32-ETHERNET-KIT
-    env:
-      TEST_DIR: components/mdns/examples
     # Skip running on forks since it won't have access to secrets
     if: github.repository == 'espressif/esp-protocols'
     steps:
@@ -256,28 +236,27 @@ jobs:
       - uses: actions/checkout@v3
       - uses: actions/download-artifact@v3
         with:
-          name: examples_app_bin_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.config }}
+          name: mdns_bin_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.test.app }}
-          path: ${{ env.TEST_DIR }}/build_${{ matrix.config }}
+          path: ${{ matrix.test.path }}/ci/
       - name: Install Python packages
         env:
           PIP_EXTRA_INDEX_URL: "https://www.piwheels.org/simple"
         run: |
             sudo apt-get install -y dnsutils
-      - name: Download Example Test to target ${{ matrix.config }}
+      - name: Run ${{ matrix.test.app }} application on ${{ matrix.idf_target }}
-        run: |
+        working-directory: ${{ matrix.test.path }}
-            python -m esptool --chip ${{ matrix.idf_target }} write_flash 0x0 components/mdns/examples/build_${{ matrix.config }}/flash_image.bin
-      - name: Run Example Test on target ${{ matrix.config }}
-        working-directory: components/mdns/examples
         run: |
+            unzip ci/artifacts.zip -d ci
+            for dir in `ls -d ci/build_*`; do
             rm -rf build sdkconfig.defaults
-            mv build_${{ matrix.config }} build
+            mv $dir build
-            cat sdkconfig.ci.${{ matrix.config }} >> sdkconfig.defaults
+            python -m pytest --log-cli-level DEBUG --junit-xml=./results_${{ matrix.test.app }}_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${dir#"ci/build_"}.xml --target=${{ matrix.idf_target }}
-            python -m pytest --log-cli-level DEBUG --junit-xml=./examples_results_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.config }}.xml --target=${{ matrix.idf_target }}
+            done
       - uses: actions/upload-artifact@v3
         if: always()
         with:
-          name: examples_results_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.config }}
+          name: results_${{ matrix.test.app }}_${{ matrix.idf_target }}_${{ matrix.idf_ver }}.xml
-          path: ${{ env.TEST_DIR }}/*.xml
+          path: ${{ matrix.test.path }}/*.xml
 
   run-target-asio:
     strategy:
@@ -348,7 +327,6 @@ jobs:
           idf.py set-target ${{ matrix.idf_target }}
           idf.py build
           $GITHUB_WORKSPACE/ci/clean_build_artifacts.sh ${GITHUB_WORKSPACE}/${TEST_DIR}/build
-          ls build
       - uses: actions/upload-artifact@v3
         with:
           name: modem_target_bin_${{ matrix.idf_target }}_${{ matrix.idf_ver }}_${{ matrix.test.app }}

ci/clean_build_artifacts.sh

@@ -4,4 +4,4 @@
 # - flasher args
 # - sdkconfigs (header and json)
 # (Ignoring the command failure as it refuses to delete nonempty dirs)
-find $1 ! -regex ".*/build/[^/]+.\(bin\|elf\)" -a ! -regex ".*\(bootloader\|partition-table\).bin" -a ! -name "flasher_args.json" -a ! -regex ".*/build/config/sdkconfig.\(h\|json\)" -delete || true
+find $1 ! -regex ".*/build[^/]*/[^/]+.\(bin\|elf\)" -a ! -regex ".*\(bootloader\|partition-table\).bin" -a ! -name "flasher_args.json" -a ! -regex ".*/build[^/]*/config/sdkconfig.\(h\|json\)" -delete || true

components/mdns/Kconfig

@@ -64,17 +64,6 @@ menu "mDNS"
             Configures timeout for adding a new mDNS service. Adding a service
             fails if could not be completed within this time.
 
-    config MDNS_STRICT_MODE
-        bool "mDNS strict mode"
-        default "n"
-        help
-            Configures strict mode. Set this to 1 for the mDNS library to strictly follow the RFC6762:
-            Currently the only strict feature: Do not repeat original questions in response packets
-            (defined in RFC6762 sec. 6).
-            Default configuration is 0, i.e. non-strict mode, since some implementations,
-            such as lwIP mDNS resolver (used by standard POSIX API like getaddrinfo, gethostbyname)
-            could not correctly resolve advertised names.
-
     config MDNS_TIMER_PERIOD_MS
         int "mDNS timer period (ms)"
         range 10 10000

components/mdns/examples/pytest_mdns.py

@@ -1,4 +1,4 @@
-# SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: Unlicense OR CC0-1.0
 import re
 import select
@@ -8,8 +8,11 @@ import subprocess
 import time
 from threading import Event, Thread
 
-import dpkt
+try:
-import dpkt.dns
+    import dpkt
+    import dpkt.dns
+except ImportError:
+    pass
 
 
 def get_dns_query_for_esp(esp_host):

components/mdns/mdns.c

@@ -1728,7 +1728,6 @@ static void _mdns_create_answer_from_parsed_packet(mdns_parsed_packet_t *parsed_
             return;
         }
 
-#ifdef MDNS_REPEAT_QUERY_IN_RESPONSE
         if (parsed_packet->src_port != MDNS_SERVICE_PORT &&  // Repeat the queries only for "One-Shot mDNS queries"
                 (q->type == MDNS_TYPE_ANY || q->type == MDNS_TYPE_A || q->type == MDNS_TYPE_AAAA)) {
             mdns_out_question_t *out_question = malloc(sizeof(mdns_out_question_t));
@@ -1751,7 +1750,6 @@ static void _mdns_create_answer_from_parsed_packet(mdns_parsed_packet_t *parsed_
             out_question->own_dynamic_memory = true;
             queueToEnd(mdns_out_question_t, packet->questions, out_question);
         }
-#endif // MDNS_REPEAT_QUERY_IN_RESPONSE
         if (q->unicast) {
             unicast = true;
         }
@@ -5398,7 +5396,7 @@ esp_err_t mdns_instance_name_set(const char *instance)
     if (!_mdns_server) {
         return ESP_ERR_INVALID_STATE;
     }
-    if (_str_null_or_empty(instance) || strlen(instance) > (MDNS_NAME_BUF_LEN - 1)) {
+    if (_str_null_or_empty(instance) || _mdns_server->hostname == NULL || strlen(instance) > (MDNS_NAME_BUF_LEN - 1)) {
         return ESP_ERR_INVALID_ARG;
     }
     char *new_instance = strndup(instance, MDNS_NAME_BUF_LEN - 1);

components/mdns/private_include/mdns_private.h

@@ -21,27 +21,6 @@
 #define _mdns_dbg_printf(...) printf(__VA_ARGS__)
 #endif
 
-/** mDNS strict mode: Set this to 1 for the mDNS library to strictly follow the RFC6762:
- * Strict features:
- *   - to do not set original questions in response packets per RFC6762, sec 6
- *
- * The actual configuration is 0, i.e. non-strict mode, since some implementations,
- * such as lwIP mdns resolver (used by standard POSIX API like getaddrinfo, gethostbyname)
- * could not correctly resolve advertised names.
- */
-#ifndef CONFIG_MDNS_STRICT_MODE
-#define MDNS_STRICT_MODE 0
-#else
-#define MDNS_STRICT_MODE 1
-#endif
-
-#if !MDNS_STRICT_MODE
-/* mDNS responders sometimes repeat queries in responses
- * but according to RFC6762, sec 6: Responses MUST NOT contain
- * any item in question field */
-#define  MDNS_REPEAT_QUERY_IN_RESPONSE 1
-#endif
-
 /** Number of predefined interfaces */
 #ifndef CONFIG_MDNS_PREDEF_NETIF_STA
 #define CONFIG_MDNS_PREDEF_NETIF_STA 0

components/mdns/tests/unit_test/CMakeLists.txt

@@ -1,2 +1,7 @@
-idf_component_register(SRC_DIRS "."
+# This is the project CMakeLists.txt file for the test subproject
-                    PRIV_REQUIRES cmock test_utils mdns)
+cmake_minimum_required(VERSION 3.16)
+
+set(EXTRA_COMPONENT_DIRS ../.. "$ENV{IDF_PATH}/tools/unit-test-app/components")
+
+include($ENV{IDF_PATH}/tools/cmake/project.cmake)
+project(mdns_test)

components/mdns/tests/unit_test/main/CMakeLists.txt

@@ -0,0 +1,5 @@
+
+idf_component_register(SRCS "test_mdns.c"
+                       REQUIRES test_utils
+                       INCLUDE_DIRS "."
+                       PRIV_REQUIRES unity mdns)

components/mdns/tests/unit_test/main/test_mdns.c

@@ -1,13 +1,16 @@
 /*
- * SPDX-FileCopyrightText: 2022 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Unlicense OR CC0-1.0
  */
-#include "test_utils.h"
+
 #include "mdns.h"
 #include "esp_event.h"
 #include "unity.h"
+#include "test_utils.h"
 
+#include "unity_fixture.h"
+#include "memory_checks.h"
 
 #define MDNS_HOSTNAME "test-hostname"
 #define MDNS_DELEGATE_HOSTNAME "delegate-hostname"
@@ -16,6 +19,18 @@
 #define MDNS_SERVICE_PROTO "_tcp"
 #define MDNS_SERVICE_PORT   80
 
+TEST_GROUP(mdns);
+
+TEST_SETUP(mdns)
+{
+    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(mdns)
+{
+    test_utils_finish_and_evaluate_leaks(32, 64);
+}
 
 static void yield_to_all_priorities(void)
 {
@@ -27,7 +42,7 @@ static void yield_to_all_priorities(void)
 }
 
 
-TEST_CASE("mdns api to fail in invalid state", "[mdns][leaks=64]")
+TEST(mdns, api_fails_with_invalid_state)
 {
     TEST_ASSERT_NOT_EQUAL(ESP_OK, mdns_init() );
     TEST_ASSERT_NOT_EQUAL(ESP_OK, mdns_hostname_set(MDNS_HOSTNAME) );
@@ -35,7 +50,7 @@ TEST_CASE("mdns api to fail in invalid state", "[mdns][leaks=64]")
     TEST_ASSERT_NOT_EQUAL(ESP_OK, mdns_service_add(MDNS_INSTANCE, MDNS_SERVICE_NAME, MDNS_SERVICE_PROTO, MDNS_SERVICE_PORT, NULL, 0) );
 }
 
-TEST_CASE("mdns init and deinit", "[mdns][leaks=64]")
+TEST(mdns, init_deinit)
 {
     test_case_uses_tcpip();
     TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_create_default());
@@ -45,7 +60,7 @@ TEST_CASE("mdns init and deinit", "[mdns][leaks=64]")
     esp_event_loop_delete_default();
 }
 
-TEST_CASE("mdns api return expected err-code and do not leak memory", "[mdns][leaks=64]")
+TEST(mdns, api_fails_with_expected_err)
 {
     mdns_txt_item_t serviceTxtData[CONFIG_MDNS_MAX_SERVICES] = { {NULL, NULL},
     };
@@ -91,7 +106,7 @@ TEST_CASE("mdns api return expected err-code and do not leak memory", "[mdns][le
     esp_event_loop_delete_default();
 }
 
-TEST_CASE("mdns query api return expected err-code and do not leak memory", "[leaks=64]")
+TEST(mdns, query_api_fails_with_expected_err)
 {
     mdns_result_t *results = NULL;
     esp_ip6_addr_t addr6;
@@ -100,6 +115,14 @@ TEST_CASE("mdns query api return expected err-code and do not leak memory", "[le
     TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_create_default());
 
     TEST_ASSERT_EQUAL(ESP_OK, mdns_init() );
+    // check it is not possible to register a service or set an instance without configuring the hostname
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, mdns_service_add(MDNS_INSTANCE, MDNS_SERVICE_NAME, MDNS_SERVICE_PROTO, MDNS_SERVICE_PORT, NULL, 0));
+    TEST_ASSERT_EQUAL(ESP_ERR_INVALID_ARG, mdns_instance_name_set(MDNS_INSTANCE));
+    TEST_ASSERT_EQUAL(ESP_OK, mdns_hostname_set(MDNS_HOSTNAME));
+    // hostname is set, now adding a service and instance should succeed
+    TEST_ASSERT_EQUAL(ESP_OK, mdns_service_add(MDNS_INSTANCE, MDNS_SERVICE_NAME, MDNS_SERVICE_PROTO, MDNS_SERVICE_PORT, NULL, 0));
+    TEST_ASSERT_EQUAL(ESP_OK, mdns_instance_name_set(MDNS_INSTANCE));
+
     TEST_ASSERT_EQUAL(ESP_OK, mdns_query_ptr(MDNS_SERVICE_NAME, MDNS_SERVICE_PROTO, 10, CONFIG_MDNS_MAX_SERVICES,  &results) );
     mdns_query_results_free(results);
 
@@ -118,3 +141,16 @@ TEST_CASE("mdns query api return expected err-code and do not leak memory", "[le
     mdns_free();
     esp_event_loop_delete_default();
 }
+
+TEST_GROUP_RUNNER(mdns)
+{
+    RUN_TEST_CASE(mdns, api_fails_with_invalid_state)
+    RUN_TEST_CASE(mdns, api_fails_with_expected_err)
+    RUN_TEST_CASE(mdns, query_api_fails_with_expected_err)
+    RUN_TEST_CASE(mdns, init_deinit)
+}
+
+void app_main(void)
+{
+    UNITY_MAIN(mdns);
+}

components/mdns/tests/unit_test/pytest_mdns.py

@@ -0,0 +1,8 @@
+# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: Apache-2.0
+
+from pytest_embedded import Dut
+
+
+def test_lwip(dut: Dut) -> None:
+    dut.expect_unity_test_output()

components/mdns/tests/unit_test/sdkconfig.defaults

@@ -0,0 +1,2 @@
+CONFIG_UNITY_ENABLE_FIXTURE=y
+CONFIG_UNITY_ENABLE_IDF_TEST_RUNNER=n