From 34680833b88b37bbcffca0b31dffe45f29e9d35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Fri, 14 Aug 2026 00:51:40 +0000 Subject: [PATCH] fix(test): make the native-windows test suite build and run (#11482) * fix(test): make the native-windows test suite build and run pio test -e native-windows failed every suite at the build stage. Five independent causes, all Windows-only: - TestUtil.cpp called lstat(), which MinGW-w64 does not provide. The state-checkpoint walk added in #11322 is fenced with ARCH_PORTDUINO, which native-windows also satisfies, so all 53 suites failed to compile. Route it through a stat() shim on _WIN32. - test_default, test_http_content_handler, test_meshpacket_serializer and test_serial define no setUp/tearDown and relied on the weak defaults PlatformIO emits in unity_config.c. GCC lowers a weak definition on PE-COFF to a weak external, leaving the symbol undefined, so it does not satisfy unity.c's reference and the link fails. Define them explicitly, as the other 49 suites already do. - test_mqtt included , absent on MinGW, for htonl(). Use winsock2.h there. - test_gps_update_scheduling uses TEST_ASSERT_DOUBLE_WITHIN. Unity omits double support unless UNITY_INCLUDE_DOUBLE is defined, so the assertion compiled to an unconditional failure. Define it for the env. - test_getfiles_rejects_overlong_path is excluded on _WIN32. Overrunning the 228-byte file_name needs at least 229 bytes below the portduino root, and that root is already ~34 bytes, so every qualifying path passes the 260-byte MAX_PATH: the nested mkdir() fails, the file is never created, and getFiles() has nothing to drop. No component layout satisfies both limits. Each of the seven suites that failed on Windows was verified individually after the change. test_fscommon_getfiles still fails in a full run, for a cause outside this change: rmDir() does not remove directories on Windows, so empty dirs left by an earlier run survive setUp() and make getFiles() report a depth truncation. That is a pre-existing FSCommon bug, reported separately. No Linux or macOS behaviour changes: every guard is _WIN32-only except UNITY_INCLUDE_DOUBLE, which is scoped to env:native-windows. * fix(test): define UNITY_INCLUDE_DOUBLE for every native env The flag was scoped to env:native-windows, but the gap is not Windows-specific. Verified on Debian with gcc against the Linux env's own Unity 2.6.1 and PlatformIO's generated native unity_config: UNITY_INCLUDE_DOUBLE : NOT defined UNITY_EXCLUDE_DOUBLE : defined test_double_within:FAIL: Unity Double Precision Disabled UNITY_INCLUDE_DOUBLE appears nowhere in the repo, the ini files, the workflow, or PlatformIO's unity runner, which adds only UNITY_INCLUDE_CONFIG_H. So TEST_ASSERT_DOUBLE_* is an always-failing stub on Linux and macOS too, not only on Windows. Moved to portduino_base.build_flags_common, which every native env resolves: native, native-tft, native-fb, native-tft-debug, coverage, coverage-event-policy, native-macos, native-windows and native-wasm. This does change Linux and macOS: TEST_ASSERT_DOUBLE_* becomes a real comparison instead of a stub. test_gps_update_scheduling is the only suite using those macros and its arithmetic is integer-based and bit-identical across platforms, so it should pass wherever it runs. Note it currently reports PASSED on CI in 0.03s while emitting no Unity output at all, so those assertions appear never to execute there; that is tracked separately and is not addressed here. --- test/TestUtil.cpp | 16 +++++++++++++++- test/test_default/test_main.cpp | 4 ++++ test/test_fscommon_getfiles/test_main.cpp | 6 ++++++ test/test_http_content_handler/test_main.cpp | 4 ++++ .../test_serializer.cpp | 4 ++++ test/test_mqtt/MQTT.cpp | 6 ++++++ test/test_serial/SerialModule.cpp | 5 +++++ variants/native/portduino.ini | 3 +++ 8 files changed, 47 insertions(+), 1 deletion(-) diff --git a/test/TestUtil.cpp b/test/TestUtil.cpp index f9e14373d9..58cd34c151 100644 --- a/test/TestUtil.cpp +++ b/test/TestUtil.cpp @@ -63,6 +63,20 @@ void testStateCheckpoint(const char *, const char *) {} namespace { +/// MinGW-w64 has no lstat(): Windows has no POSIX symlink stat, and nothing in a test sandbox +/// creates a symlink, so stat() sees the same thing for every entry walk() can reach. +#ifdef _WIN32 +inline int lstatCompat(const char *path, struct stat *st) +{ + return stat(path, st); +} +#else +inline int lstatCompat(const char *path, struct stat *st) +{ + return lstat(path, st); +} +#endif + /// Content fingerprint, used only to answer "did this file change?". FNV-1a rather than a real /// digest because the answer is a boolean and the files are a few KB of protobuf; nothing here /// records a hash as an expected value, which is what would make this a snapshot test. @@ -96,7 +110,7 @@ void walk(const std::string &root, const std::string &rel, std::mapd_name) : rel + "/" + e->d_name; const std::string childPath = root + "/" + childRel; struct stat st; - if (lstat(childPath.c_str(), &st) != 0) + if (lstatCompat(childPath.c_str(), &st) != 0) continue; if (S_ISDIR(st.st_mode)) walk(root, childRel, out); diff --git a/test/test_default/test_main.cpp b/test/test_default/test_main.cpp index ee4fc16279..36c06e9773 100644 --- a/test/test_default/test_main.cpp +++ b/test/test_default/test_main.cpp @@ -277,6 +277,10 @@ void test_trafficType_overflowSaturates() TEST_ASSERT_EQUAL_UINT32(static_cast(INT32_MAX), res); } +// Required by Unity: PlatformIO's weak defaults do not link on MinGW (PE-COFF weak externals). +void setUp(void) {} +void tearDown(void) {} + void setup() { // Small delay to match other test mains diff --git a/test/test_fscommon_getfiles/test_main.cpp b/test/test_fscommon_getfiles/test_main.cpp index 943bc43a7f..eaa776d1be 100644 --- a/test/test_fscommon_getfiles/test_main.cpp +++ b/test/test_fscommon_getfiles/test_main.cpp @@ -112,6 +112,9 @@ void test_getfiles_depth_limit(void) // 4. A path that will not fit meshtastic_FileInfo::file_name is dropped, not truncated into the // manifest, and the drop is reported. +// Not built on Windows: any path long enough to overrun the 228-byte file_name also exceeds the +// 260-byte MAX_PATH, so the tree is never created and there is nothing to drop. +#ifndef _WIN32 void test_getfiles_rejects_overlong_path(void) { // file_name is 228 bytes; build a nested path that overruns it while each component stays @@ -148,6 +151,7 @@ void test_getfiles_rejects_overlong_path(void) *strrchr(dir, '/') = '\0'; } } +#endif // 5. pathEndsWithDot() - no entry in the manifest may end in '.', which is how the walk filters the // "." and ".." pseudo-entries some backends return. @@ -231,7 +235,9 @@ void setup() RUN_TEST(test_getfiles_respects_max_count); RUN_TEST(test_getfiles_unlimited_when_under_cap); RUN_TEST(test_getfiles_depth_limit); +#ifndef _WIN32 RUN_TEST(test_getfiles_rejects_overlong_path); +#endif RUN_TEST(test_getfiles_skips_dot_entries); RUN_TEST(test_getfiles_reports_sizes); RUN_TEST(test_getfiles_missing_dir_is_empty); diff --git a/test/test_http_content_handler/test_main.cpp b/test/test_http_content_handler/test_main.cpp index 3b628a2b21..c5b5d32a17 100644 --- a/test/test_http_content_handler/test_main.cpp +++ b/test/test_http_content_handler/test_main.cpp @@ -8,6 +8,10 @@ static void test_placeholder() } extern "C" { +// Required by Unity: PlatformIO's weak defaults do not link on MinGW (PE-COFF weak externals). +void setUp(void) {} +void tearDown(void) {} + void setup() { initializeTestEnvironment(); diff --git a/test/test_meshpacket_serializer/test_serializer.cpp b/test/test_meshpacket_serializer/test_serializer.cpp index 82e79f8e1a..db863ca3c2 100644 --- a/test/test_meshpacket_serializer/test_serializer.cpp +++ b/test/test_meshpacket_serializer/test_serializer.cpp @@ -23,6 +23,10 @@ void test_timestamp_present_when_has_rx_time(); void test_timestamp_zeroed_when_rx_time_absent(); void test_encrypted_timestamp_zeroed_when_rx_time_absent(); +// Required by Unity: PlatformIO's weak defaults do not link on MinGW (PE-COFF weak externals). +void setUp(void) {} +void tearDown(void) {} + void setup() { UNITY_BEGIN(); diff --git a/test/test_mqtt/MQTT.cpp b/test/test_mqtt/MQTT.cpp index 3c4f1ab6af..b67cf31abe 100644 --- a/test/test_mqtt/MQTT.cpp +++ b/test/test_mqtt/MQTT.cpp @@ -17,7 +17,13 @@ #include #include +// htonl() for remoteIP() below. MinGW has no ; the byte-order helpers live in +// winsock2.h, which must precede any the Arduino shims pull in. +#ifdef _WIN32 +#include +#else #include +#endif #include #include diff --git a/test/test_serial/SerialModule.cpp b/test/test_serial/SerialModule.cpp index 6539d0ad34..48808db855 100644 --- a/test/test_serial/SerialModule.cpp +++ b/test/test_serial/SerialModule.cpp @@ -2,6 +2,11 @@ #include "TestUtil.h" #include +// Required by Unity: PlatformIO's weak defaults do not link on MinGW (PE-COFF weak externals). +// Outside the guard below so both the portduino and the stub setup() get them. +void setUp(void) {} +void tearDown(void) {} + #ifdef ARCH_PORTDUINO #include "configuration.h" diff --git a/variants/native/portduino.ini b/variants/native/portduino.ini index 5997cf1fbf..7787adc9c4 100644 --- a/variants/native/portduino.ini +++ b/variants/native/portduino.ini @@ -57,6 +57,9 @@ build_flags_common = -std=gnu17 -std=gnu++17 -DMAX_TFT_COLOR_REGIONS=64 + ; Unity omits double support unless asked, compiling TEST_ASSERT_DOUBLE_* into an + ; unconditional "Unity Double Precision Disabled" failure (test_gps_update_scheduling). + -DUNITY_INCLUDE_DOUBLE build_flags = ${portduino_base.build_flags_common}