ci: run the test suite on every PR; add fs_Hz and S-param reload regression coverage

.github/workflows/ci.yml

@@ -33,7 +33,7 @@ jobs:
       - name: Install Linux dependencies
         run: |
           sudo apt-get update -qq
-          sudo apt-get install -y -qq ninja-build cmake pkg-config g++-14 \
+          sudo apt-get install -y -qq ninja-build cmake pkg-config xvfb \
             libxrandr-dev libxinerama-dev libxcursor-dev libxi-dev libxext-dev libgl1-mesa-dev
 
       - name: Configure
@@ -46,3 +46,8 @@ jobs:
 
       - name: Build
         run: cmake --build build
+
+      # Run the full suite on every PR (not just release tags). Mirrors the
+      # release.yml strict-build invocation; xvfb-run gives test_ui a display.
+      - name: Test
+        run: xvfb-run --auto-servernum ctest --test-dir build --output-on-failure -E "Benchmark"

.github/workflows/release.yml

@@ -134,7 +134,35 @@ jobs:
           if [ "${{ runner.os }}" = "Linux" ]; then
             xvfb-run --auto-servernum ctest --test-dir build --output-on-failure -E "Benchmark"
           else
-            ctest --test-dir build --output-on-failure -E "Benchmark|test_ui|ADC DDC output grid spans"
+            # test_ui is excluded on Windows: it drives an interactive ImGui
+            # window and needs a display, so it cannot run headless in CI.
+            # ("ADC DDC output grid spans [-Fs/4, Fs/4)" was previously excluded
+            # by name too. Root cause: the bracket-paren pattern in that name
+            # broke catch_discover_tests' unquoted list parsing, merging it and
+            # every later test into one ';'-joined CTest entry that Catch2
+            # rejects as "Invalid Filter". The test was renamed to "ADC DDC
+            # output grid spans from -Fs/4 to Fs/4" (tests/test_adc.cpp) and now
+            # registers as its own CTest entry, so the exclusion is removed.)
+            ctest --test-dir build --output-on-failure -E "Benchmark|test_ui"
+          fi
+
+      - name: Verify MinGW TEST_CASE registration count
+        if: runner.os == 'Windows'
+        run: |
+          # The MinGW-w64 toolchain silently drops TEST_CASEs registered beyond
+          # a ceiling in the main `tests` binary (see tests/AGENTS.md). Count
+          # the registered cases (each emits one indented tag line) and fail if
+          # the count ever falls below the verified floor, so a silent drop is
+          # caught in CI instead of shipping unrun tests. Bump EXPECTED when the
+          # registered count changes; new coverage that must run on Windows goes
+          # in a standalone executable (e.g. test_signal_domain), not the main
+          # tests binary.
+          EXPECTED=223
+          COUNT=$(build/bin/tests.exe --list-tests | grep -c '^      \[')
+          echo "registered TEST_CASEs: $COUNT (expected >= $EXPECTED)"
+          if [ "$COUNT" -lt "$EXPECTED" ]; then
+            echo "ERROR: tests.exe registered $COUNT TEST_CASEs, below the verified floor of $EXPECTED"
+            exit 1
           fi
 
   sanitizer:

tests/AGENTS.md

@@ -20,9 +20,9 @@ Own the Catch2 v3.4.0 unit test suite and ImGui test engine UI tests. Verify all
 - Test files are named `test_<component>.cpp` matching the module name
 - Build via `cmake --build build && ctest --test-dir build` or direct `build/bin/tests`
 - Adding a new module? Add its test source to `TEST_SOURCES` in `CMakeLists.txt` and link the library target
-- **MinGW-w64 test-registration ceiling:** this toolchain silently drops any `TEST_CASE` registered beyond the ~217 already linked into the main `tests` executable (confirmed via a from-scratch clean rebuild; see the comment above `test_component_authoring` in `CMakeLists.txt`). Do not add new `TEST_CASE`s to `test_main.cpp` or any file already compiled into the `tests` target — give the new coverage its own standalone executable instead (`add_executable(test_<name> test_<name>.cpp)` + `target_link_libraries` + `add_test`, following `test_attenuator`/`test_combiner`/`test_component_authoring`/`test_extensions`/`test_signal_domain`), and run it directly (`build/bin/test_<name>.exe`) rather than relying on `ctest`.
+- **MinGW-w64 test-registration ceiling:** this toolchain silently drops any `TEST_CASE` registered beyond the ~223 already linked into the main `tests` executable (verified 2026-08-09; the release.yml Windows job enforces the 223 floor with a `--list-tests` count guard). Do not add new `TEST_CASE`s to `test_main.cpp` or any file already compiled into the `tests` target — give the new coverage its own standalone executable instead (`add_executable(test_<name> test_<name>.cpp)` + `target_link_libraries` + `add_test`, following `test_attenuator`/`test_combiner`/`test_component_authoring`/`test_extensions`/`test_signal_domain`), and run it directly (`build/bin/test_<name>.exe`) rather than relying on `ctest`.
 - Platform-specific tests (e.g., Windows-only session state) are gated with `#ifdef WIN32` in CMakeLists.txt
-- `test_component_authoring` and `test_tutorial_state` are standalone executables, not part of `TEST_SOURCES`: the MinGW-w64 toolchain silently drops `TEST_CASE`s registered beyond the ~217 already linked into `tests`. New test files that must run on Windows should follow that pattern.
+- `test_component_authoring` and `test_tutorial_state` are standalone executables, not part of `TEST_SOURCES`: the MinGW-w64 toolchain silently drops `TEST_CASE`s registered beyond the ~223 already linked into `tests`. New test files that must run on Windows should follow that pattern.
 
 ## Work Guidance
 

tests/test_adc.cpp

@@ -79,7 +79,13 @@ TEST_CASE("ADC DDC tone at 0 Hz maps to -Fs/4", "[adc]") {
     REQUIRE(out.tones[0].freq_Hz == Approx(-Fs / 4.0).margin(1.0));
 }
 
-TEST_CASE("ADC DDC output grid spans [-Fs/4, Fs/4)", "[adc]") {
+// Name avoids the "[-Fs/4, Fs/4)" bracket-paren pattern: catch_discover_tests
+// (Catch2 v3.4.0) expands the discovered name list unquoted, and an unclosed
+// '[' followed by ')' breaks CMake's list splitting, merging this test and
+// every name after it into one ';'-joined CTest entry that Catch2 rejects as
+// an "Invalid Filter" (observed on CMake 4.x; the test otherwise passes on
+// MinGW).
+TEST_CASE("ADC DDC output grid spans from -Fs/4 to Fs/4", "[adc]") {
     NodeGraphEngine graph;
     AdcEngine adc(3, graph);
     adc.setFs_Hz(Fs);

tests/test_ideal_filter.cpp

@@ -141,17 +141,25 @@ TEST_CASE("IdealFilter passes noise density unchanged in passband", "[filter]")
 
 TEST_CASE("IdealFilter preserves fs_Hz from input", "[filter]") {
     NodeGraphEngine graph;
-    SignalGeneratorEngine gen(0, graph);
-    gen.addTone(100e6, -20.0);
-    gen.update(0.0);
+
+    // Drive from a synthetic spectrum with a NONZERO fs_Hz. The generator's
+    // output fs_Hz is 0.0, so the old version of this test compared 0.0 == 0.0
+    // and could never catch a dropped fs_Hz (issues #43/#54).
+    Spectrum in;
+    in.frequencies = {100e6, 200e6};
+    in.tones = {{100e6, -20.0, 0.0}};
+    in.noise_W.assign(2, 1e-20);
+    in.noise_added_W.assign(2, 0.0);
+    in.noise_total_W.assign(2, 1e-20);
+    in.fs_Hz = 500e6;
 
     IdealFilterEngine filt(0, graph);
     filt.setFilterType(FilterType::LPF);
     filt.setCutoff_Hz(200e6);
-    filt.node().inputs[0] = &gen.node().outputs[0];
+    filt.node().inputs[0] = ∈
     filt.update(0.0);
 
-    REQUIRE(filt.node().outputs[0].fs_Hz == gen.node().outputs[0].fs_Hz);
+    REQUIRE(filt.node().outputs[0].fs_Hz == Approx(500e6));
 }
 
 TEST_CASE("IdealFilter dirty flag skips when input unchanged", "[filter]") {

tests/test_project_file.cpp

@@ -6,10 +6,13 @@
 #include "implot.h"
 #include <catch2/catch_approx.hpp>
 #include <catch2/catch_test_macros.hpp>
+#include <cmath>
+#include <complex>
 #include <cstdio>
 #include <filesystem>
 #include <fstream>
 #include <nlohmann/json.hpp>
+#include <numbers>
 #include <string>
 
 using Catch::Approx;
@@ -440,7 +443,7 @@ TEST_CASE_METHOD(ImGuiFixture, "Round-trip: default component positions are (0,0
 }
 
 // ---------------------------------------------------------------------------
-// 13 — Issue #44: S-param mode survives save/load for amplifier, ideal filter,
+// 13 — Issue #56: S-param mode survives save/load for amplifier, ideal filter,
 //      equalizer, attenuator, and combiner. Previously deserialize() restored
 //      sparam_mode/sparam_filepath but never reloaded the Touchstone file, so
 //      a reloaded project silently fell back to ideal/manual mode.
@@ -450,7 +453,7 @@ static std::string sparamFixturePath() {
            "/component_data/amplifiers/adm-3844psm/ADM-8344PSM_SM_A_25C_De_5V_5V_102mA.s2p";
 }
 
-TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (issue #44)",
+TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (issue #56)",
                  "[project_file][sparam]") {
     auto path = tempPath();
     std::remove(path.c_str());
@@ -492,6 +495,32 @@ TEST_CASE_METHOD(ImGuiFixture, "Round-trip: S-param mode survives save/load (iss
         CHECK(amps[0]->sparamMode() == true);
         CHECK(amps[0]->sparamLoaded() == true);
 
+        // Issue #56 regression: sparamMode()/sparamLoaded() are necessary but
+        // not sufficient — the old deserialize() restored the mode flags and
+        // filepath without reloading the Touchstone file, so a reloaded
+        // amplifier reported sparamMode()==true but applied ideal gain. Drive
+        // a tone through the loaded amplifier via the app DSP chain and
+        // compare the output against the S21-derived expectation (mirrors
+        // tests/test_amplifier_sparam.cpp).
+        auto &gen = app.testComponents().add<SignalGeneratorEngine>(20001, app.testGraphEngine());
+        gen.addTone(1e9, -20.0);
+        gen.update(0.0);
+
+        int gen_pin = gen.outputPinId();
+        int amp_pin = amps[0]->inputPinId();
+        int link_id = app.testGraphEngine().addLink(gen_pin, amp_pin);
+        REQUIRE(link_id > 0);
+
+        app.update_dsp();
+
+        const auto &amp_out = amps[0]->node().outputs[0];
+        REQUIRE(amp_out.tones.size() == 1);
+        auto S21 = amps[0]->sparamData().interpolate(1e9, 2);
+        double expected_gain = 20.0 * std::log10(std::abs(S21));
+        REQUIRE(amp_out.tones[0].power_dBm == Approx(-20.0 + expected_gain).margin(0.5));
+        double expected_phase = std::arg(S21) * 180.0 / std::numbers::pi;
+        REQUIRE(amp_out.tones[0].phase_deg == Approx(expected_phase).margin(1.0));
+
         auto flts = app.testComponents().byType<IdealFilterEngine>();
         REQUIRE(flts.size() == 1);
         CHECK(flts[0]->sparamMode() == true);

tests/test_signal_domain.cpp

@@ -419,7 +419,7 @@ TEST_CASE("SpectrumAnalyzer: complex-baseband tone renders unchanged (no mirrori
     REQUIRE(trace[10] < -50.0);
 }
 
-// ---- fs_Hz propagation (issue #43) ----
+// ---- fs_Hz propagation (issues #43/#54) ----
 
 TEST_CASE("Mixer: propagates fs_Hz", "[domain][mixer]") {
     NodeGraphEngine graph;
@@ -505,8 +505,75 @@ TEST_CASE("Amplifier: propagates fs_Hz (S-param mode)", "[domain][amplifier]") {
     REQUIRE(amp.node().outputs[0].fs_Hz == Approx(500e6));
 }
 
+TEST_CASE("Combiner: propagates fs_Hz", "[domain][combiner]") {
+    NodeGraphEngine graph;
+    CombinerEngine comb(0, graph);
+
+    Spectrum in0, in1;
+    in0.frequencies = {1e9, 2e9};
+    in0.noise_total_W.assign(2, 1e-21);
+    in0.fs_Hz = 500e6;
+    in1.frequencies = {1e9, 2e9};
+    in1.noise_total_W.assign(2, 1e-21);
+    in1.fs_Hz = 500e6;
+
+    comb.node().inputs[0] = &in0;
+    comb.node().inputs[1] = &in1;
+    comb.update(0.0);
+
+    REQUIRE(comb.node().outputs[0].fs_Hz == Approx(500e6));
+}
+
+TEST_CASE("Attenuator: propagates fs_Hz (manual mode)", "[domain][attenuator]") {
+    NodeGraphEngine graph;
+    AttenuatorEngine atten(0, graph);
+
+    Spectrum in;
+    in.frequencies = {1e9, 2e9};
+    in.tones = {{1e9, -10.0, 0.0}};
+    in.noise_total_W.assign(2, 1e-21);
+    in.fs_Hz = 500e6;
+
+    atten.node().inputs[0] = &in;
+    atten.update(0.0);
+
+    REQUIRE(atten.node().outputs[0].fs_Hz == Approx(500e6));
+}
+
+TEST_CASE("Equalizer: propagates fs_Hz", "[domain][equalizer]") {
+    NodeGraphEngine graph;
+    EqualizerEngine eq(0, graph);
+
+    Spectrum in;
+    in.frequencies = {1e9, 2e9};
+    in.tones = {{1e9, -10.0, 0.0}};
+    in.noise_total_W.assign(2, 1e-21);
+    in.fs_Hz = 500e6;
+
+    eq.node().inputs[0] = &in;
+    eq.update(0.0);
+
+    REQUIRE(eq.node().outputs[0].fs_Hz == Approx(500e6));
+}
+
+TEST_CASE("IdealFilter: propagates fs_Hz", "[domain][ideal_filter]") {
+    NodeGraphEngine graph;
+    IdealFilterEngine flt(0, graph);
+
+    Spectrum in;
+    in.frequencies = {1e9, 2e9};
+    in.tones = {{1e9, -10.0, 0.0}};
+    in.noise_total_W.assign(2, 1e-21);
+    in.fs_Hz = 500e6;
+
+    flt.node().inputs[0] = &in;
+    flt.update(0.0);
+
+    REQUIRE(flt.node().outputs[0].fs_Hz == Approx(500e6));
+}
+
 // Real multi-engine post-ADC chains: fs_Hz must reach the PFB and channels must be populated
-// without any manual setFs_Hz() (issue #43 regression tests).
+// without any manual setFs_Hz() (issues #43/#54 regression tests).
 
 static bool anyChannelHasContent(const PFBChannelizerEngine &pfb) {
     for (const auto &ch : pfb.channels()) {