feat(test): Add skip ability to the test cases at runtime

[howard0su]

Add more tests into ctest and convert to cppunittest framework. Add the functionality to skip a test due to environment issues like missing model, mismatch hardware.

This enable us to run real tests against GPU and a real model weights. The following env needs to be defined in order to pass the tests which needs Qwen3.5 serial model weights:
LUCE_TEST_MODEL_QWEN35

[cubic-dev-ai]

17 issues found across 26 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="server/CMakeLists.txt">

<violation number="1" location="server/CMakeLists.txt:1574">
P2: When cross-compiling without the model assets, `test_model_smoke` returns 77 because every discovered case skips, but CTest marks this registration failed. Set `SKIP_RETURN_CODE 77` here, as the native discovery and generic cross-compilation helper already do.</violation>

<violation number="2" location="server/CMakeLists.txt:1675">
P2: Converting the listed targets to discovery drops the stable CTest names (`platform_compat`, `cuda_comm_api`, `qwen35_tensor_parallel`, `ggml_rmsnorm_batch`, `feature_gate`, ...) in favor of `test_<target>.<method>` without adding an alias, contradicting this same file's comment about preserving established names for CI filters/downstream scripts. If any filter references those names, tests silently stop matching.</violation>
</file>

<file name="server/test/smoke_load_draft.cpp">

<violation number="1" location="server/test/smoke_load_draft.cpp:82">
P3: The new CHECK_NOT_NULL/CHECK guards are placed after the code has already dereferenced the same pointers (w.fc->ne, w.hidden_norm->ne, w.layers[0].wq, and ggml_backend_tensor_get(w.hidden_norm, ...)). If any were null the test would crash before reaching them, so the guards can never catch a bad load. Move them immediately after load_draft_safetensors succeeds (as REQUIRE_NOT_NULL) or gate the spot-check block on them, if the intent is to fail cleanly on a malformed draft.</violation>
</file>

<file name="server/test/test_platform_compat.cpp">

<violation number="1" location="server/test/test_platform_compat.cpp:29">
P3: The descriptive failure messages from the old `fail()` helper are lost. Each `CHECK(false)` now logs only "CHECK: false" at a file:line, so when a check trips the developer must cross-reference the source to learn which check failed and what it was verifying. Report the failure with a message, e.g. `CHECK(false)` after logging a context string or using a named section, so the failure output stays self-explanatory.</violation>

<violation number="2" location="server/test/test_platform_compat.cpp:29">
P3: Replacing the early `return fail(...)` with a bare `CHECK(false)` and continuing lets later, environment-dependent assertions run against corrupted state. When `unset_environment_variable` fails, the subsequent `set_environment_variable`/`getenv` checks still execute and add misleading cascading failures. Return after each failed step (or use a REQUIRE that aborts) so a failure surfaces the actual root cause.</violation>
</file>

<file name="server/test/test_ggml_rmsnorm_batch.cpp">

<violation number="1" location="server/test/test_ggml_rmsnorm_batch.cpp:112">
P3: The `printf("PASS ...")` now runs before the only assertion, so a compute or finiteness failure prints "PASS" and then fails, which misleads CI/log triage. In the old code the early `return 5`/`return 6` kept PASS off failure runs. Print it only on success, e.g. move it after a passing `REQUIRE(finite)`.</violation>
</file>

<file name="server/test/test_feature_gate.cpp">

<violation number="1" location="server/test/test_feature_gate.cpp:557">
P3: This conversion collapses all 20 gate rules into a single TEST_CASE `feature_gate_suite`, but the old harness ran each `test_*` function as its own RUN_TEST unit and the rest of the repo's cppunit tests use one TEST_CASE per test (see test_server_unit.cpp). With one case, --discover_tests and keyword filtering register a single test, and a failing gate rule reports only as "FeatureGateFixture::feature_gate_suite" instead of naming the broken rule, which makes a failed `ctest test_feature_gate` harder to triage. Split each `test_*` function into its own TEST_CASE, e.g. `TEST_CASE(FeatureGateFixture, feature_gate_accepts_plain_launch) { ... }`, so failures and discovery stay per-rule.</violation>
</file>

<file name="server/test/test_deepseek4_hc_cuda.cpp">

<violation number="1" location="server/test/test_deepseek4_hc_cuda.cpp:213">
P3: When `max_abs > TOL`, the test now prints `FAIL` and then unconditionally `[ds4-hc-test] PASS` before `REQUIRE(max_abs <= TOL)` throws. The removal of the early `return 1` left the PASS banner reachable on the failure path, so failing runs report both PASS and FAIL. Move the PASS print behind the check, or drop the redundant `if (max_abs > TOL)` block and print PASS only after the assertion passes.</violation>
</file>

<file name="server/test/test_rms_norm_hip.cpp">

<violation number="1" location="server/test/test_rms_norm_hip.cpp:50">
P2: When cudaGetDeviceCount fails for a reason other than cudaErrorNoDevice (e.g. cudaErrorInsufficientDriver), device_count stays 0 and the test SKIPs (exit 77) instead of failing, hiding a genuinely broken HIP environment from CI. Only skip when device_status == cudaErrorNoDevice; let any other error reach CK(device_status) so it fails the test.</violation>
</file>

<file name="server/test/CppUnitTestFramework.hpp">

<violation number="1" location="server/test/CppUnitTestFramework.hpp:390">
P3: When a test throws TestSkippedException, m_indent_level is never reset. EnterTest() increments it and only ExitTest() resets it to 0, but the skip path calls SkipTest() instead, so each skipped test leaves the indent one level deeper. Subsequent tests then print failures/sections with progressively wrong indentation. Reset m_indent_level in ConsoleLogger::SkipTest().</violation>

<violation number="2" location="server/test/CppUnitTestFramework.hpp:892">
P2: The skip reason from SKIP("...") is silently dropped. The catch sets test_skipped=true but never logs the message, and SkipTest() only prints the test name, so output shows which test was skipped but not why (e.g. missing model vs. hardware mismatch). Since distinguishing those reasons is the point of the feature, surface the message in the skip output.</violation>
</file>

<file name="server/test/test_rocmfp_mix_glu_fusable.cpp">

<violation number="1" location="server/test/test_rocmfp_mix_glu_fusable.cpp:159">
P3: The conversion keeps the legacy global `g_fails` counter and file-local `CHECK(cond,msg)` macro, so the new TEST_CASE reports success only through the trailing `REQUIRE_TRUE(g_fails == 0)`. This bypasses the framework's per-fixture failure tracking: `g_fails` is a file-static that is never reset, and the framework's own `CHECK` (which records `m_check_has_failed` and continues) is `#undef`-shadowed, leaving the global the only signal. Since CppUnitTestFramework already provides a `CHECK(Expression)` with Continue semantics, prefer replacing the custom macro/global with the framework's `CHECK` and dropping the trailing `REQUIRE_TRUE`; this removes the shared mutable state and the fragile coupling for any future test case added in the same TU.</violation>
</file>

<file name="server/test/test_rocmfp_mix_gateup_glu.cpp">

<violation number="1" location="server/test/test_rocmfp_mix_gateup_glu.cpp:309">
P3: The file #undef's the framework's CHECK and keeps its own g_fails counter only to funnel everything into a single `REQUIRE_TRUE(false)` at the end. Failures print to stderr instead of going through the framework logger, so each failing assertion is not attributed by line and the framework only ever sees one REQUIRE failure. Use the framework's CHECK (which drives HaveChecksFailed) and drop the manual g_fails counter, or reset g_fails at test start since it is a file-static global.</violation>
</file>

<file name="server/test/test_recurrent_snapshot.cpp">

<violation number="1" location="server/test/test_recurrent_snapshot.cpp:38">
P3: The `CHECK(...)` immediately before each `if (!x) SKIP(...)` is redundant: when the condition is false, CHECK records a failure that is then discarded because SKIP marks the test as skipped. Drop the CHECK and keep only the `if (!x) SKIP(...)` (or use REQUIRE_NOT_NULL to fail instead of skip).</violation>
</file>

<file name="server/test/test_flashprefill_kernels.cpp">

<violation number="1" location="server/test/test_flashprefill_kernels.cpp:82">
P3: The skip gate treats every `cudaGetDeviceCount` error as "device unavailable", so genuine driver/runtime errors (not just no dev ignored) become a silent skip (exit 77) instead of a failure. Restrict the SKIP to `cudaErrorNoDevice`/count==0 and let other errors fail, matching `test_rms_norm_hip.cpp`.</violation>
</file>

<file name="server/test/smoke_load_target.cpp">

<violation number="1" location="server/test/smoke_load_target.cpp:30">
P2: The PR states the new skip mechanism should skip tests on both a missing model and hardware mismatch, but only the model case is covered. When `ggml_backend_cuda_init(0)` returns null, `REQUIRE_NOT_NULL(backend)` throws `AssertException`, which the framework counts as a hard failure rather than a skip. To match the stated intent, throw `TestSkippedException` (or the `SKIP` macro) on CUDA init failure so a no-GPU CI host skips these tests instead of failing them.</violation>

<violation number="2" location="server/test/smoke_load_target.cpp:109">
P3: The new null/id-guards are added after the same fields are already dereferenced in the preceding print statements (`w.tok_embd->ne[0]`, `w.output->type`, `w.out_norm->type`). A null there would crash before these guards run, so the guards cannot catch the null case they appear to validate. Move the guards immediately after `load_target_gguf` returns (or rely on the existing loader which requires non-null), so the checks actually guard the dereferences.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When cross-compiling without the model assets, test_model_smoke returns 77 because every discovered case skips, but CTest marks this registration failed. Set SKIP_RETURN_CODE 77 here, as the native discovery and generic cross-compilation helper already do.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 1574:

<comment>When cross-compiling without the model assets, `test_model_smoke` returns 77 because every discovered case skips, but CTest marks this registration failed. Set `SKIP_RETURN_CODE 77` here, as the native discovery and generic cross-compilation helper already do.</comment>

<file context>
@@ -1554,6 +1534,68 @@ if(DFLASH27B_TESTS)
+            target_link_libraries(test_model_smoke PRIVATE hip::host)
+        endif()
+        if(CMAKE_CROSSCOMPILING)
+            add_test(NAME model_smoke COMMAND test_model_smoke)
+        else()
+            set(_model_smoke_ctest_generated
</file context>

Suggested change

	add_test(NAME model_smoke COMMAND test_model_smoke)
	add_test(NAME model_smoke COMMAND test_model_smoke)
	set_tests_properties(model_smoke PROPERTIES SKIP_RETURN_CODE 77)

[cubic-dev-ai]

P2: When cudaGetDeviceCount fails for a reason other than cudaErrorNoDevice (e.g. cudaErrorInsufficientDriver), device_count stays 0 and the test SKIPs (exit 77) instead of failing, hiding a genuinely broken HIP environment from CI. Only skip when device_status == cudaErrorNoDevice; let any other error reach CK(device_status) so it fails the test.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_rms_norm_hip.cpp, line 50:

<comment>When cudaGetDeviceCount fails for a reason other than cudaErrorNoDevice (e.g. cudaErrorInsufficientDriver), device_count stays 0 and the test SKIPs (exit 77) instead of failing, hiding a genuinely broken HIP environment from CI. Only skip when device_status == cudaErrorNoDevice; let any other error reach CK(device_status) so it fails the test.</comment>

<file context>
@@ -28,15 +30,28 @@ extern "C" void launch_rms_norm_mul_w_f32(
 
+    int device_count = 0;
+    const cudaError_t device_status = cudaGetDeviceCount(&device_count);
+    if (device_status == cudaErrorNoDevice || device_count == 0) {
+        SKIP("no HIP device available");
+    }
</file context>

[cubic-dev-ai]

P2: The skip reason from SKIP("...") is silently dropped. The catch sets test_skipped=true but never logs the message, and SkipTest() only prints the test name, so output shows which test was skipped but not why (e.g. missing model vs. hardware mismatch). Since distinguishing those reasons is the point of the feature, surface the message in the skip output.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/CppUnitTestFramework.hpp, line 892:

<comment>The skip reason from SKIP("...") is silently dropped. The catch sets test_skipped=true but never logs the message, and SkipTest() only prints the test name, so output shows which test was skipped but not why (e.g. missing model vs. hardware mismatch). Since distinguishing those reasons is the point of the feature, surface the message in the skip output.</comment>

<file context>
@@ -862,6 +889,8 @@ void TestCase_##TestName::Run()
 #define CHECK_CLOSE_FRACTION(Left, Right, Fraction) \
     CppUnitTestFramework::CommonFixture::HandleAssert(CppUnitTestFramework::AssertType::Continue, _CPPUTF_ASSERT_LOCATION, CppUnitTestFramework::Assert::CloseFraction((Left), (Right), (Fraction)))
 
+#define SKIP(Message) throw CppUnitTestFramework::TestSkippedException((Message))
+
 //------------------------------------------------------------------------------------------------------------
</file context>

[cubic-dev-ai]

P2: Converting the listed targets to discovery drops the stable CTest names (platform_compat, cuda_comm_api, qwen35_tensor_parallel, ggml_rmsnorm_batch, feature_gate, ...) in favor of test_<target>.<method> without adding an alias, contradicting this same file's comment about preserving established names for CI filters/downstream scripts. If any filter references those names, tests silently stop matching.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 1675:

<comment>Converting the listed targets to discovery drops the stable CTest names (`platform_compat`, `cuda_comm_api`, `qwen35_tensor_parallel`, `ggml_rmsnorm_batch`, `feature_gate`, ...) in favor of `test_<target>.<method>` without adding an alias, contradicting this same file's comment about preserving established names for CI filters/downstream scripts. If any filter references those names, tests silently stop matching.</comment>

<file context>
@@ -1578,12 +1620,69 @@ if(DFLASH27B_TESTS)
+    foreach(_cppunit_target IN LISTS _new_cppunit_test_targets)
+        if(TARGET ${_cppunit_target})
+            target_sources(${_cppunit_target} PRIVATE test/test_unit_main.cpp)
+            dflash_discover_cppunit_tests(${_cppunit_target})
+            list(APPEND _cppunit_test_targets ${_cppunit_target})
+            list(APPEND _raw_unit_test_targets ${_cppunit_target})
</file context>

[cubic-dev-ai]

P2: The PR states the new skip mechanism should skip tests on both a missing model and hardware mismatch, but only the model case is covered. When ggml_backend_cuda_init(0) returns null, REQUIRE_NOT_NULL(backend) throws AssertException, which the framework counts as a hard failure rather than a skip. To match the stated intent, throw TestSkippedException (or the SKIP macro) on CUDA init failure so a no-GPU CI host skips these tests instead of failing them.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/smoke_load_target.cpp, line 30:

<comment>The PR states the new skip mechanism should skip tests on both a missing model and hardware mismatch, but only the model case is covered. When `ggml_backend_cuda_init(0)` returns null, `REQUIRE_NOT_NULL(backend)` throws `AssertException`, which the framework counts as a hard failure rather than a skip. To match the stated intent, throw `TestSkippedException` (or the `SKIP` macro) on CUDA init failure so a no-GPU CI host skips these tests instead of failing them.</comment>

<file context>
@@ -17,20 +19,22 @@
+    const auto path = luce_test::require_model(luce_test::kQwen35ModelEnv);
     ggml_backend_t backend = ggml_backend_cuda_init(0);
-    if (!backend) { std::fprintf(stderr, "cuda init failed\n"); return 1; }
+    REQUIRE_NOT_NULL(backend);
 
     TargetWeights w;
</file context>

[cubic-dev-ai]

P3: The file #undef's the framework's CHECK and keeps its own g_fails counter only to funnel everything into a single REQUIRE_TRUE(false) at the end. Failures print to stderr instead of going through the framework logger, so each failing assertion is not attributed by line and the framework only ever sees one REQUIRE failure. Use the framework's CHECK (which drives HaveChecksFailed) and drop the manual g_fails counter, or reset g_fails at test start since it is a file-static global.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_rocmfp_mix_gateup_glu.cpp, line 309:

<comment>The file #undef's the framework's CHECK and keeps its own g_fails counter only to funnel everything into a single `REQUIRE_TRUE(false)` at the end. Failures print to stderr instead of going through the framework logger, so each failing assertion is not attributed by line and the framework only ever sees one REQUIRE failure. Use the framework's CHECK (which drives HaveChecksFailed) and drop the manual g_fails counter, or reset g_fails at test start since it is a file-static global.</comment>

<file context>
@@ -298,8 +306,7 @@ int main() {
     HIP_OK(cudaFree(d_fused));  HIP_OK(cudaFree(d_swapped));
 
-    if (g_fails) { std::fprintf(stderr, "%d FAILURE(S)\n", g_fails); return 1; }
+    if (g_fails) { std::fprintf(stderr, "%d FAILURE(S)\n", g_fails); REQUIRE_TRUE(false); }
     std::fprintf(stderr, "OK: fused gate/up GLU matches the unfused pair, order is respected, "
                          "half-registered/mismatched pairs are refused, and out-of-range ids zero\n");
</file context>

[cubic-dev-ai]

P3: The CHECK(...) immediately before each if (!x) SKIP(...) is redundant: when the condition is false, CHECK records a failure that is then discarded because SKIP marks the test as skipped. Drop the CHECK and keep only the if (!x) SKIP(...) (or use REQUIRE_NOT_NULL to fail instead of skip).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_recurrent_snapshot.cpp, line 38:

<comment>The `CHECK(...)` immediately before each `if (!x) SKIP(...)` is redundant: when the condition is false, CHECK records a failure that is then discarded because SKIP marks the test as skipped. Drop the CHECK and keep only the `if (!x) SKIP(...)` (or use REQUIRE_NOT_NULL to fail instead of skip).</comment>

<file context>
@@ -33,10 +32,10 @@ static std::vector<float> get_tensor(const ggml_tensor * tensor) {
     ggml_backend_t backend = ggml_backend_cpu_init();
     CHECK(backend != nullptr);
-    if (!backend) return 1;
+    if (!backend) SKIP("CPU backend is unavailable");
 
     ggml_init_params params{};
</file context>

[cubic-dev-ai]

P3: The skip gate treats every cudaGetDeviceCount error as "device unavailable", so genuine driver/runtime errors (not just no dev ignored) become a silent skip (exit 77) instead of a failure. Restrict the SKIP to cudaErrorNoDevice/count==0 and let other errors fail, matching test_rms_norm_hip.cpp.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_flashprefill_kernels.cpp, line 82:

<comment>The skip gate treats every `cudaGetDeviceCount` error as "device unavailable", so genuine driver/runtime errors (not just no dev ignored) become a silent skip (exit 77) instead of a failure. Restrict the SKIP to `cudaErrorNoDevice`/count==0 and let other errors fail, matching `test_rms_norm_hip.cpp`.</comment>

<file context>
@@ -59,14 +63,25 @@ void launch_sparse_flash_forward_bf16(
+TEST_CASE(FlashprefillKernelsFixture, flashprefill_kernels) {
+    int device_count = 0;
+    if (cudaGetDeviceCount(&device_count) != cudaSuccess || device_count == 0) {
+        SKIP("CUDA/HIP device unavailable");
+    }
+
</file context>

[cubic-dev-ai]

P3: The new null/id-guards are added after the same fields are already dereferenced in the preceding print statements (w.tok_embd->ne[0], w.output->type, w.out_norm->type). A null there would crash before these guards run, so the guards cannot catch the null case they appear to validate. Move the guards immediately after load_target_gguf returns (or rely on the existing loader which requires non-null), so the checks actually guard the dereferences.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/smoke_load_target.cpp, line 109:

<comment>The new null/id-guards are added after the same fields are already dereferenced in the preceding print statements (`w.tok_embd->ne[0]`, `w.output->type`, `w.out_norm->type`). A null there would crash before these guards run, so the guards cannot catch the null case they appear to validate. Move the guards immediately after `load_target_gguf` returns (or rely on the existing loader which requires non-null), so the checks actually guard the dereferences.</comment>

<file context>
@@ -101,8 +105,12 @@ int main(int argc, char ** argv) {
     if (w.n_layer > 63) print_layer(63);
 
+    CHECK(w.n_layer > 0);
+    CHECK_NOT_NULL(w.tok_embd);
+    CHECK_NOT_NULL(w.output);
+    CHECK_NOT_NULL(w.out_norm);
</file context>

[cubic-dev-ai]

P3: Replacing the early return fail(...) with a bare CHECK(false) and continuing lets later, environment-dependent assertions run against corrupted state. When unset_environment_variable fails, the subsequent set_environment_variable/getenv checks still execute and add misleading cascading failures. Return after each failed step (or use a REQUIRE that aborts) so a failure surfaces the actual root cause.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At server/test/test_platform_compat.cpp, line 29:

<comment>Replacing the early `return fail(...)` with a bare `CHECK(false)` and continuing lets later, environment-dependent assertions run against corrupted state. When `unset_environment_variable` fails, the subsequent `set_environment_variable`/`getenv` checks still execute and add misleading cascading failures. Return after each failed step (or use a REQUIRE that aborts) so a failure surfaces the actual root cause.</comment>

<file context>
@@ -10,47 +11,47 @@
 
     if (unset_environment_variable(kEnvName) != 0) {
-        return fail("could not clear test environment variable");
+        CHECK(false);
     }
     if (set_environment_variable(kEnvName, "original", true) != 0) {
</file context>