[On Hold] Add stage 1 host test rollout#1698
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a “stage 1” host-test rollout for FluidNC by introducing a structured host test layout (unit + integration suites), tightening CI around that matrix, and adding coverage/report guardrails to prevent regressions.
Changes:
- Add new PlatformIO native test suite layout (
test_unit,test_integration_*) plus supporting stubs/capture shims for WebUI and bus integration coverage. - Add host coverage reporting + guardrails (
coverage.py,tools/coverage_guard.py) and CI jobs to run ASan/integration/coverage flows. - Align VFD build manifests across PlatformIO and CMake, and add Python tests intended to validate coverage/manifests.
Reviewed changes
Copilot reviewed 66 out of 77 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/test_coverage_guard.py | Unit tests for tools/coverage_guard.py behaviors/exit codes. |
| tools/test_build_manifests.py | Python test to validate VFD source references exist in build manifests. |
| tools/coverage_guard.py | Enforces minimum coverage thresholds for critical modules + active-host metrics. |
| platformio.ini | Adds/adjusts host test envs (unit/integration/coverage/ASan) and build filters. |
| coverage.py | Reworks coverage generation across unit + integration suites and emits JSON artifacts. |
| FluidNC/tests/test_unit/test_main.cpp | GTest main for the test_unit suite. |
| FluidNC/tests/test_unit/test_UtilityTest.cpp | Adds unit tests for utility templates/constants (currently local re-implementations). |
| FluidNC/tests/test_unit/test_UTF8Test.cpp | Adds UTF8 encode/decode unit tests. |
| FluidNC/tests/test_unit/test_StringUtilTest.cpp | Adds string_util unit tests. |
| FluidNC/tests/test_unit/test_StateTest.cpp | Adds behavior-focused State consumer test stub. |
| FluidNC/tests/test_unit/test_RegexprTest.cpp | Adds unit tests for regexMatch(). |
| FluidNC/tests/test_unit/test_RealtimeCmdTest.cpp | Adds unit tests for realtime command enum values/relationships. |
| FluidNC/tests/test_unit/test_PinOptionsParserTest.cpp | Moves/adds PinOptionsParser tests under test_unit. |
| FluidNC/tests/test_unit/test_FluidErrorTest.cpp | Adds tests for FluidError error_code/category/message. |
| FluidNC/tests/test_unit/test_ErrorBehaviorTest.cpp | Adds tests for observable ErrorNames behavior. |
| FluidNC/tests/test_unit/test_CommandCompletionTest.cpp | Minor updates/cleanups to existing completion tests. |
| FluidNC/tests/test_integration_webui/test_main.cpp | Integration suite main shim for WebUI integration tests. |
| FluidNC/tests/test_integration_webui/test_WebUiNativeIntegrationTest.cpp | WebUI host integration tests using capture stubs and direct product code paths. |
| FluidNC/tests/test_integration_webui/src_001.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_002.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_003.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_004.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_005.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_006.cpp | Compiles capture/product sources into the WebUI integration suite. |
| FluidNC/tests/test_integration_webui/src_007.cpp | Host-only placeholder TU for suite wiring. |
| FluidNC/tests/test_integration_webui/src_008.cpp | Host-only placeholder TU for suite wiring. |
| FluidNC/tests/test_integration_webui/src_009.cpp | Compiles WebUI Authentication product code into suite. |
| FluidNC/tests/test_integration_webui/src_010.cpp | Compiles WebUI mDNS product code into suite. |
| FluidNC/tests/test_integration_webui/src_011.cpp | Compiles NotificationsService product code into suite. |
| FluidNC/tests/test_integration_webui/src_012.cpp | Compiles OTA product code into suite. |
| FluidNC/tests/test_integration_webui/src_013.cpp | Compiles Mime product code into suite. |
| FluidNC/tests/test_integration_webui/src_014.cpp | Host-only placeholder TU for suite wiring. |
| FluidNC/tests/test_integration_webui/src_015.cpp | Host-only placeholder TU for suite wiring. |
| FluidNC/tests/test_integration_webui/src_016.cpp | Compiles string_util.cpp into suite. |
| FluidNC/tests/test_integration_machine_buses/test_main.cpp | Integration suite main shim for machine bus integration tests. |
| FluidNC/tests/test_integration_machine_buses/test_MachineBusIntegrationTest.cpp | Host integration tests for I2C/SPI/I2SO bus behavior with extensive stubbing. |
| FluidNC/tests/test_integration_machine_buses/src_001.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_002.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_003.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_004.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_005.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_006.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_007.cpp | Compiles capture/product sources into the machine-bus integration suite. |
| FluidNC/tests/test_integration_machine_buses/src_008.cpp | Compiles I2CBus product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_009.cpp | Compiles I2SOBus product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_010.cpp | Compiles MachineConfig product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_011.cpp | Compiles SPIBus product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_012.cpp | Compiles pin detail product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_013.cpp | Compiles pin attrs product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_014.cpp | Compiles pin capabilities product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_015.cpp | Compiles pin detail product code into suite. |
| FluidNC/tests/test_integration_machine_buses/src_016.cpp | Compiles VoidPinDetail product code into suite. |
| FluidNC/tests/support/integration_gtest_main.cpp | Shared GTest main for integration suites. |
| FluidNC/tests/TEST_PLAN.md | Documents testing strategy, suite placement, and wiring patterns. |
| FluidNC/src/WebUI/OTA.cpp | Skips OTA module factory registration under PIO_UNIT_TESTING. |
| FluidNC/src/WebUI/NotificationsService.cpp | Makes NotificationsService more host-test friendly via PIO_UNIT_TESTING guards and adds missing include. |
| FluidNC/src/WebUI/Mdns.cpp | Adds PIO_UNIT_TESTING behavior to avoid settings allocation and module registration during host tests. |
| FluidNC/src/WebUI/Authentication.cpp | Fixes type name typo and adds default handling (currently problematic macro-based approach). |
| FluidNC/src/Machine/MachineConfig.cpp | Fixes preprocessor guards for I2C section emission and SPI deletion. |
| FluidNC/src/CMakeLists.txt | Updates VFD source list to match current VFD source tree. |
| FluidNC/capture/nvsfile.cpp | Adds missing standard headers for capture implementation. |
| FluidNC/capture/mdns.h | Adds capture stub header for mDNS APIs/observability. |
| FluidNC/capture/i2s_out.cpp | Adds observable capture implementation for I2S out read/write/delay. |
| FluidNC/capture/gpio.cpp | Adds observable capture implementation for GPIO read/write/mode/drive/event state. |
| FluidNC/capture/freertos/task.h | Adds missing pdMS_TO_TICKS macro for host builds. |
| FluidNC/capture/esp_wifi.h | Adds wifi_mode_t stub for host builds. |
| FluidNC/capture/base64.h | Adds base64 encode stub for host builds. |
| FluidNC/capture/WiFiClientSecure.h | Adds WiFiClientSecure stub with observable IO for host tests. |
| FluidNC/capture/WiFi.h | Adds WiFi stub for mode/hostname in host tests. |
| FluidNC/capture/WString.cpp | Fixes trim behavior for all-whitespace strings in host stub. |
| FluidNC/capture/TestStubs.h | Declares host-test stubs for state/log filtering. |
| FluidNC/capture/TestStubs.cpp | Implements many host-test stubs and weak fallbacks to enable integration suites. |
| FluidNC/capture/PwmPin.cpp | Adds observable capture implementation for PWM pin construction/duty. |
| FluidNC/capture/Platform.h | Allows MAX_N_* platform constants to be overridden via defines in host builds. |
| FluidNC/capture/ArduinoOTA.h | Adds ArduinoOTA stub for host tests. |
| .gitignore | Ignores newly generated coverage artifacts. |
| .github/workflows/ci.yml | Adds host ASan/integration/coverage jobs and refines build matrix for stage-1 rollout. |
You can also share your feedback on Copilot code review. Take the survey.
|
I have been working on a very-complex patch to make FluidNC run on MacOS and Linux, with full network support. It is almost ready; I am in the process of upstreaming fixes to several public libraries, and will then submit the FluidNC patch. I am worried that we are setting ourselves up for the merge from hell between my work and yours. |
|
Addressed the coverage finding and CI path issues in 10f55c7. Changes:
Local validation run:
|
10f55c7 to
15782cb
Compare
@MitchBradley , I’m trying to make your life easier, not harder. I still haven’t fully nailed down the CI pipeline, so please don’t factor my work into your patch. Go ahead with your changes and I’ll handle the merge pain on my side. My work is still pretty Windows-centric, and I may need to significantly rework it once your patch lands anyway. I also know you’ve been building out your own unit tests, so I don’t want my in-progress changes adding friction. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a “stage 1” host-side test rollout for FluidNC, adding dedicated unit/integration suites plus CI/coverage guardrails intended to prevent regressions in a small set of critical modules (machine buses + select WebUI paths), and aligns build manifests with the current source tree.
Changes:
- Add stage-1 host test suites (unit + machine-bus integration + WebUI integration) and supporting host stubs/shims.
- Add coverage reporting + guardrails and build-manifest self-tests; narrow CI coverage flow to stage-1 matrix.
- Refactor parts of WebUI module registration to ensure host integration tests execute real product code paths.
Reviewed changes
Copilot reviewed 54 out of 64 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/test_coverage_guard.py | Adds Python unittests for the coverage guard script. |
| tools/test_build_manifests.py | Adds Python unittests to ensure build manifests reference existing VFD sources. |
| tools/ranlib.cmd | Windows wrapper to route ranlib through the integration compiler wrapper. |
| tools/integration_path_aliases.py | Creates build-dir path aliases/symlinks/junctions and sets env vars for host integration builds. |
| tools/integration_compiler_wrapper.py | Rewrites compiler/archiver args to stabilize paths for host integration builds. |
| tools/git_version_build.py | Moves git-version generation into a PlatformIO pre-script. |
| tools/gcc.cmd | Windows wrapper to route gcc through the integration compiler wrapper. |
| tools/g++.cmd | Windows wrapper to route g++ through the integration compiler wrapper. |
| tools/coverage_guard.py | Adds coverage guardrails for critical modules + active-host called-file threshold enforcement. |
| tools/ar.cmd | Windows wrapper to route ar through the integration compiler wrapper. |
| platformio.ini | Adds stage-1 test env wiring, integration envs, extra_scripts, and build metadata defines. |
| git-version.py | Makes version file generation robust to being run outside repo root by using absolute paths. |
| FluidNC/tests/test_unit/test_UtilityTest.cpp | Adds unit tests for small utility templates/constants (self-contained). |
| FluidNC/tests/test_unit/test_UTF8Test.cpp | Adds unit tests for UTF8 encode/decode behavior. |
| FluidNC/tests/test_unit/test_StringUtilTest.cpp | Adds unit tests for string_util helpers. |
| FluidNC/tests/test_unit/test_StateTest.cpp | Adds behavior-focused tests for State enum consumption patterns. |
| FluidNC/tests/test_unit/test_RegexprTest.cpp | Adds unit tests for the lightweight regex matcher behavior. |
| FluidNC/tests/test_unit/test_RealtimeCmdTest.cpp | Adds unit tests asserting realtime command enum values and ordering. |
| FluidNC/tests/test_unit/test_PinOptionsParserTest.cpp | Moves/organizes PinOptionsParser tests into test_unit layout. |
| FluidNC/tests/test_unit/test_main.cpp | Adds a unit-test main for the test_unit suite. |
| FluidNC/tests/test_unit/test_FluidErrorTest.cpp | Adds unit tests for FluidError error_code integration. |
| FluidNC/tests/test_unit/test_ErrorBehaviorTest.cpp | Adds unit tests covering ErrorNames map behaviors. |
| FluidNC/tests/test_unit/test_CommandCompletionTest.cpp | Small updates to existing command completion tests (includes + output reuse). |
| FluidNC/tests/TEST_PLAN.md | Documents the test suite structure, wiring patterns, and anti-patterns. |
| FluidNC/tests/test_integration_webui/test_WebUiNativeIntegrationTest.cpp | Adds host integration tests for WebUI MIME/mDNS/OTA/notifications paths via capture shims. |
| FluidNC/tests/test_integration_webui/test_main.cpp | Adds integration suite main (includes shared gtest main). |
| FluidNC/tests/test_integration_machine_buses/test_main.cpp | Adds integration suite main (includes shared gtest main). |
| FluidNC/tests/test_integration_machine_buses/test_MachineBusIntegrationTest.cpp | Adds host integration tests for I2C/SPI/I2SO bus validation/init behavior. |
| FluidNC/tests/support/integration_gtest_main.cpp | Shared gtest main implementation for integration suites. |
| FluidNC/src/WebUI/OTARegistration.cpp | Moves OTA module registration into its own TU for clearer linkage/control. |
| FluidNC/src/WebUI/OTA.h | Adds a proper OTA module header (class declaration). |
| FluidNC/src/WebUI/OTA.cpp | Refactors OTA module implementation into WebUI::OTA (separate from registration). |
| FluidNC/src/WebUI/NotificationsServiceRegistration.cpp | Moves notifications settings/bootstrap + module registration into its own TU. |
| FluidNC/src/WebUI/NotificationsService.h | Adds Settings include + exposes configureSettings/sendMessage declarations for bootstrap wiring. |
| FluidNC/src/WebUI/NotificationsService.cpp | Refactors notifications init to rely on injected settings; adds null-guards for settings parsing. |
| FluidNC/src/WebUI/MdnsRegistration.cpp | Moves mDNS setting creation + module registration into its own TU. |
| FluidNC/src/WebUI/Mdns.h | Adds API for injecting enable-setting and checking enabled state. |
| FluidNC/src/WebUI/Mdns.cpp | Refactors mDNS to use enabled() and injected enable setting; removes in-file module registration. |
| FluidNC/src/WebUI/Authentication.cpp | Fixes admin_password type name and includes Config.h/Settings.h under ENABLE_AUTHENTICATION. |
| FluidNC/src/Report.cpp | Adds build-metadata strings (MCU/VARIANT) with host-safe fallbacks for reporting. |
| FluidNC/src/Machine/MachineConfig.cpp | Fixes preprocessor guards around I2C sections and SPI deletion. |
| FluidNC/src/CMakeLists.txt | Updates VFD source list and adds WebUI registration translation units to the manifest. |
| FluidNC/capture/WString.cpp | Fixes trim() to correctly handle all-whitespace strings and substring bounds. |
| FluidNC/capture/WiFiClientSecure.h | Adds host-side WiFiClientSecure shim capturing writes/reads for integration tests. |
| FluidNC/capture/WiFi.h | Adds host-side WiFi shim (mode/hostname). |
| FluidNC/capture/TestStubs.h | Declares shared test stub APIs for host runs. |
| FluidNC/capture/TestStubs.cpp | Implements a broad set of weak stubs for host builds/tests. |
| FluidNC/capture/Stage1IntegrationSupport.cpp | Adds stage-1 integration host surface stubs (settings, channels, machine skeleton, etc.). |
| FluidNC/capture/Stage1HostSupport.h | Declares shared “stage 1” capture state for buses and WebUI. |
| FluidNC/capture/Stage1HostSupport.cpp | Implements reset helpers and capture hooks for I2C read/write and WebUI state. |
| FluidNC/capture/spi.cpp | Implements capture SPI init/deinit hooks for integration assertions. |
| FluidNC/capture/PwmPin.cpp | Adds call tracking for PWM pin construction + duty writes in host builds. |
| FluidNC/capture/Platform.h | Makes MAX_N_* macros overrideable for host/integration builds. |
| FluidNC/capture/nvsfile.cpp | Adds missing standard headers for host-side NVS file shim compilation. |
| FluidNC/capture/mdns.h | Adds host-side mdns shim capturing init/hostname/services/free calls. |
| FluidNC/capture/i2s_out.cpp | Adds capture implementation of i2s_out_* APIs plus call counters/state. |
| FluidNC/capture/gpio.cpp | Adds capture GPIO state tracking (write/mode/drive strength/event) for host assertions. |
| FluidNC/capture/freertos/task.h | Adds delay() declaration and pdMS_TO_TICKS macro for host FreeRTOS shims. |
| FluidNC/capture/freertos/semphr.h | Adds a host-side semaphore shim for FreeRTOS API compatibility. |
| FluidNC/capture/esp_wifi.h | Adds minimal wifi_mode_t for host builds. |
| FluidNC/capture/base64.h | Adds minimal base64::encode shim for host builds. |
| FluidNC/capture/ArduinoOTA.h | Adds ArduinoOTA shim to support OTA integration tests. |
| coverage.py | Expands coverage generation to multiple suites + emits JSON summary + gaps inventory and guard inputs. |
| .github/workflows/ci.yml | Adds stage-1 CI jobs: ASan host runs, integration host runs, coverage reports + guardrails, manifest tests. |
| inline BaseType_t xSemaphoreTake(SemaphoreHandle_t semaphore, TickType_t ticks_to_wait) { | ||
| if (semaphore == nullptr) { | ||
| return pdFALSE; | ||
| } | ||
|
|
||
| std::unique_lock<std::mutex> lock(semaphore->mutex); | ||
| if (ticks_to_wait == 0) { | ||
| if (!semaphore->available) { | ||
| return pdFALSE; | ||
| } | ||
| } else { | ||
| semaphore->cv.wait(lock, [&]() { return semaphore->available; }); | ||
| } |
There was a problem hiding this comment.
xSemaphoreTake() ignores the ticks_to_wait timeout when it is non-zero and waits indefinitely on the condition variable. Code paths that rely on bounded waits (or expect timeouts) can hang host tests/builds. Implement a timed wait (e.g., wait_for using ticks_to_wait * portTICK_PERIOD_MS) and return pdFALSE on timeout.
| EnumSetting* notification_type = nullptr; | ||
| StringSetting* notification_t1 = nullptr; | ||
| StringSetting* notification_t2 = nullptr; | ||
| StringSetting* notification_ts = nullptr; |
There was a problem hiding this comment.
These notification_* setting pointers are only used within this translation unit after being configured via configureSettings(). Giving them external linkage (non-static namespace-scope definitions) is unnecessary and increases the risk of ODR/symbol collisions. Consider placing them in an anonymous namespace or marking them static.
| EnumSetting* notification_type = nullptr; | |
| StringSetting* notification_t1 = nullptr; | |
| StringSetting* notification_t2 = nullptr; | |
| StringSetting* notification_ts = nullptr; | |
| namespace { | |
| EnumSetting* notification_type = nullptr; | |
| StringSetting* notification_t1 = nullptr; | |
| StringSetting* notification_t2 = nullptr; | |
| StringSetting* notification_ts = nullptr; | |
| } // namespace |
Summary
Validation
Notes