From cbe17f43cd3cf73d8ae87d3d3f26eb02492e16a8 Mon Sep 17 00:00:00 2001 From: Andrew Hundt Date: Tue, 30 Jun 2026 09:53:51 -0400 Subject: [PATCH] test(pipeline): cover incremental postpass failure Add an internal test-only incremental phase fault hook and a focused pipeline canary that injects postpass failure directly through cbm_pipeline_run_incremental(). The test verifies the existing DB remains intact and the newly edited symbol is not published after the injected failure. Also correct stale incremental comments to describe the current full-graph containment path, and replace local timing conversion macros in pipeline_incremental.c with shared constants. Validation: make -f Makefile.cbm cbm; make -f Makefile.cbm build/c/test-runner; CBM_ONLY_SUITE=pipeline ./build/c/test-runner (226 passed); CBM_ONLY_SUITE=incremental CBM_INCR_TEST_ARTIFACT_DIR=/private/tmp/cbm-incr-artifacts-6ea9927-plus ./build/c/test-runner (160 passed, escalated for FastAPI fixture network); bash scripts/check-source-safety.sh; git diff --check. Signed-off-by: Andrew Hundt --- src/pipeline/pipeline_incremental.c | 40 +++++++---- src/pipeline/pipeline_internal.h | 9 ++- tests/test_pipeline.c | 100 +++++++++++++++++++++++++--- 3 files changed, 122 insertions(+), 27 deletions(-) diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index 3d1d75148..94168048c 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -1,11 +1,10 @@ /* * pipeline_incremental.c — Disk-based incremental re-indexing. * - * Operates on the existing SQLite DB directly (not RAM-first graph buffer). * Compares file mtime+size against stored hashes to classify changed/unchanged. - * Deletes changed files' nodes (edges cascade via ON DELETE CASCADE), - * re-parses only changed files through passes into a temp graph buffer, - * then merges new nodes/edges into the disk DB. Persists updated hashes. + * For non-noop changes, loads the existing SQLite graph into a graph buffer, + * purges changed/deleted file paths, reparses changed files, and republishes + * the full graph. This is correctness containment, not a true delta publish. * * Called from pipeline.c when a DB with stored hashes already exists. */ @@ -33,13 +32,10 @@ enum { INCR_RING_BUF = 4, INCR_RING_MASK = 3, INCR_TS_BUF = 24 }; #include #include -/* ── Constants ───────────────────────────────────────────────────── */ - -#define CBM_MS_PER_SEC 1000.0 -#define CBM_NS_PER_MS 1000000.0 - /* ── Timing helper (same as pipeline.c) ──────────────────────────── */ +static const char cbm_incr_test_env_disabled[] = "0"; + /* Fork renames this elapsed_ms_incr (vs pipeline.c's elapsed_ms) to avoid an * ODR/duplicate-symbol collision when both TUs are linked into the same binary. */ static double elapsed_ms_incr(struct timespec start) { @@ -47,7 +43,7 @@ static double elapsed_ms_incr(struct timespec start) { cbm_clock_gettime(CLOCK_MONOTONIC, &now); double s = (double)(now.tv_sec - start.tv_sec); double ns = (double)(now.tv_nsec - start.tv_nsec); - return (s * CBM_MS_PER_SEC) + (ns / CBM_NS_PER_MS); + return (s * (double)CBM_MSEC_PER_SEC) + (ns / (double)CBM_NSEC_PER_MSEC); } /* itoa into static buffer — matches pipeline.c helper. Fork renames to @@ -61,15 +57,25 @@ static const char *itoa_buf_incr(int v) { return buf[idx]; } +static bool incr_test_fail_phase_enabled(const char *phase) { + char buf[CBM_SZ_64]; + const char *val = + cbm_safe_getenv(CBM_TEST_FAIL_INCREMENTAL_PHASE, buf, sizeof(buf), NULL); + return val && val[0] != '\0' && strcmp(val, cbm_incr_test_env_disabled) != 0 && + phase && strcmp(val, phase) == 0; +} + /* ── Platform-portable mtime_ns ──────────────────────────────────── */ static int64_t stat_mtime_ns(const struct stat *st) { #ifdef __APPLE__ - return ((int64_t)st->st_mtimespec.tv_sec * CBM_NS_PER_SEC) + (int64_t)st->st_mtimespec.tv_nsec; + return ((int64_t)st->st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)st->st_mtimespec.tv_nsec; #elif defined(_WIN32) - return (int64_t)st->st_mtime * CBM_NS_PER_SEC; + return (int64_t)st->st_mtime * (int64_t)CBM_NSEC_PER_SEC; #else - return ((int64_t)st->st_mtim.tv_sec * CBM_NS_PER_SEC) + (int64_t)st->st_mtim.tv_nsec; + return ((int64_t)st->st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + + (int64_t)st->st_mtim.tv_nsec; #endif } @@ -1017,7 +1023,13 @@ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_fil } } if (pipeline_rc == 0) { - pipeline_rc = run_postpasses(&ctx, changed_files, ci, project); + if (incr_test_fail_phase_enabled(CBM_TEST_FAIL_INCREMENTAL_POSTPASS)) { + cbm_log_error("incremental.err", "phase", CBM_TEST_FAIL_INCREMENTAL_POSTPASS, "rc", + itoa_buf_incr(CBM_NOT_FOUND)); + pipeline_rc = CBM_NOT_FOUND; + } else { + pipeline_rc = run_postpasses(&ctx, changed_files, ci, project); + } } free(changed_files); diff --git a/src/pipeline/pipeline_internal.h b/src/pipeline/pipeline_internal.h index 9e0577740..b34cb4064 100644 --- a/src/pipeline/pipeline_internal.h +++ b/src/pipeline/pipeline_internal.h @@ -55,6 +55,11 @@ int64_t cbm_pipeline_upsert_service_route(cbm_gbuf_t *gb, const char *path, cbm_ #define CBM_MS_PER_SEC 1000.0 #define CBM_US_PER_SEC_F 1e6 +/* Test-only incremental fault injection. Values name internal phases and are + * intentionally not user configuration. */ +#define CBM_TEST_FAIL_INCREMENTAL_PHASE "CBM_TEST_FAIL_INCREMENTAL_PHASE" +#define CBM_TEST_FAIL_INCREMENTAL_POSTPASS "postpass" + /* ── Pipeline context (internal) ─────────────────────────────────── */ /* Per-worker manifest collection entry. */ @@ -595,8 +600,8 @@ void cbm_envscan_free_patterns(void); /* ── Incremental pipeline (pipeline_incremental.c) ───────────────── */ /* Run incremental re-index on an existing disk DB. - * Classifies files by mtime+size, deletes changed nodes, re-parses changed - * files, merges into disk DB. Returns 0 on success. */ + * Classifies files by mtime+size, loads the current DB into a graph buffer, + * reparses changed files, and republishes the graph. Returns 0 on success. */ int cbm_pipeline_run_incremental(cbm_pipeline_t *p, const char *db_path, cbm_file_info_t *files, int file_count); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 56d9d2e9d..c48aa7590 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -5721,6 +5721,31 @@ TEST(import_symbol_fallback_prefers_import_path_over_insertion_order) { * Incremental reindex * ═══════════════════════════════════════════════════════════════════ */ +typedef struct { + const char *key; + char value[CBM_SZ_64]; + bool had_value; +} pipeline_env_snapshot_t; + +static const char pipeline_test_env_enabled[] = "1"; + +static pipeline_env_snapshot_t pipeline_env_save(const char *key) { + pipeline_env_snapshot_t snap = {.key = key}; + snap.had_value = cbm_safe_getenv(key, snap.value, sizeof(snap.value), NULL) != NULL; + return snap; +} + +static void pipeline_env_restore(const pipeline_env_snapshot_t *snap) { + if (!snap || !snap->key) { + return; + } + if (snap->had_value) { + cbm_setenv(snap->key, snap->value, 1); + } else { + cbm_unsetenv(snap->key); + } +} + TEST(incremental_full_then_noop) { /* Full index, then re-run → should detect no changes and skip */ if (setup_incremental_repo() != 0) { @@ -5809,11 +5834,7 @@ TEST(incremental_detects_changed_file) { } TEST(incremental_dump_failure_keeps_existing_db) { - static const char *test_env_enabled = "1"; - char saved_fail[CBM_SZ_32] = {0}; - bool had_fail = - cbm_safe_getenv(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE, saved_fail, sizeof(saved_fail), - NULL) != NULL; + pipeline_env_snapshot_t fail_env = pipeline_env_save(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE); if (setup_incremental_repo() != 0) { FAIL("setup failed"); @@ -5832,7 +5853,7 @@ TEST(incremental_dump_failure_keeps_existing_db) { cbm_store_close(s); ASSERT(!pipeline_store_has_function_name(g_incr_dbpath, project, "NewFunc")); - char path[512]; + char path[CBM_PATH_MAX]; snprintf(path, sizeof(path), "%s/helper.go", g_incr_tmpdir); FILE *f = fopen(path, "w"); ASSERT_NOT_NULL(f); @@ -5847,14 +5868,70 @@ TEST(incremental_dump_failure_keeps_existing_db) { ASSERT_NOT_NULL(p); cbm_pipeline_apply_config(p, cfg); - cbm_setenv(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE, test_env_enabled, 1); + cbm_setenv(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE, pipeline_test_env_enabled, 1); int rc = cbm_pipeline_run(p); - if (had_fail) { - cbm_setenv(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE, saved_fail, 1); - } else { - cbm_unsetenv(CBM_TEST_FAIL_GBUF_DUMP_BEFORE_REPLACE); + pipeline_env_restore(&fail_env); + + ASSERT_NEQ(rc, 0); + s = cbm_store_open_path(g_incr_dbpath); + ASSERT_NOT_NULL(s); + ASSERT_EQ(cbm_store_count_nodes(s, project), nodes_before); + cbm_store_close(s); + ASSERT(!pipeline_store_has_function_name(g_incr_dbpath, project, "NewFunc")); + + cbm_pipeline_free(p); + free(project); + cbm_config_close(cfg); + cleanup_incremental_repo(); + PASS(); +} + +TEST(incremental_postpass_failure_keeps_existing_db) { + pipeline_env_snapshot_t fail_env = pipeline_env_save(CBM_TEST_FAIL_INCREMENTAL_PHASE); + + if (setup_incremental_repo() != 0) { + FAIL("setup failed"); } + cbm_pipeline_t *p = cbm_pipeline_new(g_incr_tmpdir, g_incr_dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + char *project = strdup(cbm_pipeline_project_name(p)); + cbm_pipeline_free(p); + + cbm_store_t *s = cbm_store_open_path(g_incr_dbpath); + ASSERT_NOT_NULL(s); + int nodes_before = cbm_store_count_nodes(s, project); + ASSERT_GT(nodes_before, 0); + cbm_store_close(s); + ASSERT(!pipeline_store_has_function_name(g_incr_dbpath, project, "NewFunc")); + + char path[CBM_PATH_MAX]; + snprintf(path, sizeof(path), "%s/helper.go", g_incr_tmpdir); + FILE *f = fopen(path, "w"); + ASSERT_NOT_NULL(f); + fprintf(f, "package main\n\n" + "func Helper() string {\n\treturn \"hello\"\n}\n\n" + "func NewFunc() int {\n\treturn 42\n}\n"); + fclose(f); + + cbm_config_t *cfg = incremental_test_config(g_incr_tmpdir); + ASSERT_NOT_NULL(cfg); + p = cbm_pipeline_new(g_incr_tmpdir, g_incr_dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + cbm_pipeline_apply_config(p, cfg); + + cbm_file_info_t *files = NULL; + int file_count = 0; + cbm_discover_opts_t opts = {.mode = CBM_MODE_FULL, .ignore_file = NULL, .max_file_size = 0}; + ASSERT_EQ(cbm_discover(g_incr_tmpdir, &opts, &files, &file_count), 0); + ASSERT_GT(file_count, 0); + + cbm_setenv(CBM_TEST_FAIL_INCREMENTAL_PHASE, CBM_TEST_FAIL_INCREMENTAL_POSTPASS, 1); + int rc = cbm_pipeline_run_incremental(p, g_incr_dbpath, files, file_count); + pipeline_env_restore(&fail_env); + cbm_discover_free(files, file_count); + ASSERT_NEQ(rc, 0); s = cbm_store_open_path(g_incr_dbpath); ASSERT_NOT_NULL(s); @@ -7418,6 +7495,7 @@ SUITE(pipeline) { RUN_TEST(incremental_full_then_noop); RUN_TEST(incremental_detects_changed_file); RUN_TEST(incremental_dump_failure_keeps_existing_db); + RUN_TEST(incremental_postpass_failure_keeps_existing_db); RUN_TEST(incremental_detects_deleted_file); RUN_TEST(incremental_new_file_added); RUN_TEST(incremental_fast_preserves_mode_skipped_tools_dir);