From 7ec13add9825bb47823787174828497086a48616 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 18 Oct 2026 16:43:26 +0200 Subject: [PATCH 1/2] fix(test-impact): mark conditionally registered runner suites CONDITIONAL on every selection path A runner suite registered only under a preprocessor condition (the four coverage_* suites under `#if defined(CBM_TEST_COVERAGE) && !defined(_WIN32)` in tests/test_main.c) is absent from builds without that condition. The selection marked such a suite CONDITIONAL only on the per-registration path, and only when a registration of its own was conditional and contributed a reason. Every suite-level path (setup unknown or hit, suite-body, changed and rule triggers, inventory, rejected artifact) made the suite whole without it, and the runner registration's own condition was never recorded. PR CI on the x86 Linux gcc leg then failed with "the test selection names suites this build does not register: coverage_setup_success coverage_setup_uncertain" (#2679, #2680): the suite-body trigger and an incomplete setup row selected both suites whole without CONDITIONAL, so selection.py left them out of optional-suites.txt. The model now records whether a runner registration sits under a condition (conditional only when every site of the suite is), and the selection marks any selected conditionally registered suite whole with CONDITIONAL after every whole-suite path. Unconditionally registered suites keep the strict contract. Signed-off-by: Martin Vogel --- src/mcp/test_impact.c | 12 +++- src/mcp/test_impact.h | 3 + src/mcp/test_impact_select.c | 7 ++- tests/test_test_impact.c | 111 +++++++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 4 deletions(-) diff --git a/src/mcp/test_impact.c b/src/mcp/test_impact.c index fe678c5fdd..a674ffcc35 100644 --- a/src/mcp/test_impact.c +++ b/src/mcp/test_impact.c @@ -630,8 +630,11 @@ static void tm_add_runner_suite(tm_scan_t *sc, const tm_tok_t *name, bool perf) return; } m->runner = items; - items[m->runner_count++] = - (cbm_test_runner_suite_t){.name = copy, .perf = perf, .file = sc->file, .line = name->line}; + items[m->runner_count++] = (cbm_test_runner_suite_t){.name = copy, + .perf = perf, + .file = sc->file, + .line = name->line, + .conditional = sc->conditions > 0}; } /* Virtual preset records are immutable. Original configured records are kept @@ -1407,11 +1410,14 @@ bool cbm_test_model_finish(cbm_test_model_t *m) { } if (m->runner_count > 1) { qsort(m->runner, (size_t)m->runner_count, sizeof(*m->runner), tm_cmp_runner); - /* One entry per suite; it is a perf suite when any site says so. */ + /* One entry per suite; it is a perf suite when any site says so, and + * conditional only when every site is. */ int kept = 0; for (int i = 1; i < m->runner_count; i++) { if (strcmp(m->runner[kept].name, m->runner[i].name) == 0) { m->runner[kept].perf = m->runner[kept].perf || m->runner[i].perf; + m->runner[kept].conditional = + m->runner[kept].conditional && m->runner[i].conditional; } else { m->runner[++kept] = m->runner[i]; } diff --git a/src/mcp/test_impact.h b/src/mcp/test_impact.h index e0363ead17..0dab762467 100644 --- a/src/mcp/test_impact.h +++ b/src/mcp/test_impact.h @@ -71,6 +71,9 @@ typedef struct { bool perf; const char *file; /* the runner file that registers it */ int line; + /* Every site registers it under a preprocessor condition: some builds do + * not register this suite at all. */ + bool conditional; } cbm_test_runner_suite_t; typedef struct cbm_test_model cbm_test_model_t; diff --git a/src/mcp/test_impact_select.c b/src/mcp/test_impact_select.c index 44e423f999..3e4967149c 100644 --- a/src/mcp/test_impact_select.c +++ b/src/mcp/test_impact_select.c @@ -290,9 +290,14 @@ cbm_test_selection_t *cbm_test_select(const cbm_test_selection_input_t *input) { for (int i = 0; i < input->reach_count; i++) if (reach[i].row->changed && !reach[i].registered) return sl_full(result, CBM_TEST_SELECT_CHANGED_UNREGISTERED); - for (int i = 0; i < runner_count; i++) + for (int i = 0; i < runner_count; i++) { if (definition_counts[i] != 1 || !registration_counts[i]) sl_whole(&result->suites[i], CBM_TEST_SELECT_INVENTORY_UNKNOWN); + /* After every whole-suite path: a selected suite that some builds do + * not register at all may be absent from the runner that executes it. */ + if ((result->suites[i].whole || result->suites[i].reasons) && runners[i].conditional) + sl_whole(&result->suites[i], CBM_TEST_SELECT_CONDITIONAL); + } /* A later registration can promote an earlier test's suite to whole. * Compact cases while the complete, sorted suite table is still present. */ int used = 0; diff --git a/tests/test_test_impact.c b/tests/test_test_impact.c index 0236535ea6..210aee288b 100644 --- a/tests/test_test_impact.c +++ b/tests/test_test_impact.c @@ -274,6 +274,33 @@ TEST(test_model_marks_what_is_under_a_condition) { PASS(); } +/* A runner registration under a condition means some builds lack the suite. + * A suite registered twice is conditional only when every site is. */ +TEST(test_model_marks_runner_suites_registered_under_a_condition) { + static const tm_source_t src[] = {{"tests/test_main.c", "int main(void) {\n" + " RUN_SELECTED_SUITE(always);\n" + "#if defined(COV) && !defined(_WIN32)\n" + " RUN_SELECTED_SUITE(guarded);\n" + " RUN_SELECTED_SUITE(twice);\n" + "#endif\n" + " RUN_SELECTED_SUITE(twice);\n" + " return 0;\n" + "}\n"}}; + cbm_test_model_t *m = model_of(src, 1); + ASSERT_NOT_NULL(m); + int n = 0; + const cbm_test_runner_suite_t *suites = cbm_test_model_runner_suites(m, &n); + ASSERT_EQ(n, 3); + ASSERT_STR_EQ(suites[0].name, "always"); + ASSERT_FALSE(suites[0].conditional); + ASSERT_STR_EQ(suites[1].name, "guarded"); + ASSERT_TRUE(suites[1].conditional); + ASSERT_STR_EQ(suites[2].name, "twice"); + ASSERT_FALSE(suites[2].conditional); + cbm_test_model_free(m); + PASS(); +} + /* Both branches of an #if may open the same block. Counting both braces would * leave the scanner one level deep for the rest of the file, and every later * test and suite would be missed. */ @@ -1688,6 +1715,88 @@ TEST(test_selection_inventory_ambiguity_and_conditionals_use_whole_suite) { PASS(); } +/* A suite the runner registers only under a preprocessor condition is absent + * from some builds, so however it is selected it must carry CONDITIONAL: the + * runner that lacks it then tolerates its absence. Its cases and registrations + * are unconditional here, so only the runner registration can say so. */ +TEST(test_selection_conditionally_registered_suite_is_conditional_on_every_path) { + static const char *quiet_rows = "alpha:*\tcomplete\t\t1\nalpha:stat\tcomplete\t\t1\n" + "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\n" + "alpha:quiet\tcomplete\t\t1\n"; + struct { + const char *rows; + bool admitted; + bool reached, changed, rule; + unsigned path; + } variants[] = { + {"alpha:*\tincomplete\tlost setup\t1\nalpha:stat\tcomplete\t\t1\n" + "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n", + true, false, false, false, CBM_TEST_SELECT_SETUP_UNKNOWN}, + {"alpha:*\tcomplete\t\t0\nalpha:stat\tcomplete\t\t1\n" + "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n", + true, false, false, false, CBM_TEST_SELECT_SETUP_HIT}, + {quiet_rows, true, true, false, false, CBM_TEST_SELECT_STATIC}, + {quiet_rows, true, false, true, false, CBM_TEST_SELECT_CHANGED}, + {quiet_rows, true, false, false, true, CBM_TEST_SELECT_RULE}, + {quiet_rows, false, false, false, false, CBM_TEST_SELECT_ARTIFACT_REJECTED}, + /* One covered case alone would otherwise name `alpha:cov`. */ + {"alpha:*\tcomplete\t\t1\nalpha:stat\tcomplete\t\t1\n" + "alpha:cov\tcomplete\t\t0\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n", + true, false, false, false, CBM_TEST_SELECT_COVERAGE}, + }; + const char *runners[] = { + "void main(void) {\n#if defined(FEATURE) && !defined(_WIN32)\nRUN_SELECTED_SUITE(alpha);\n" + "#endif\n}\n", + "void main(void) { RUN_SELECTED_SUITE(alpha); }\n", + }; + for (int r = 0; r < 2; r++) { + char source[1024]; + snprintf(source, sizeof(source), + "TEST(stat) {}\nTEST(cov) {}\nTEST(changed) {}\nTEST(quiet) {}\n" + "SUITE(alpha) { RUN_TEST(stat); RUN_TEST(cov); RUN_TEST(changed); " + "RUN_TEST(quiet); }\n%s", + runners[r]); + cbm_test_model_t *model = selection_model(source); + ASSERT_NOT_NULL(model); + for (int i = 0; i < (int)(sizeof(variants) / sizeof(variants[0])); i++) { + cbm_coverage_map_t *map = selection_map(variants[i].rows); + ASSERT_NOT_NULL(map); + cbm_test_reach_t reach[] = SELECTION_REACH; + for (int k = 0; k < 4; k++) + reach[k].reached = reach[k].changed = false; + int changed[] = {0}; + cbm_test_suite_trigger_t trigger = {.suite = "alpha", + .reached = variants[i].reached, + .changed = variants[i].changed, + .rule = variants[i].rule}; + cbm_test_selection_input_t input = selection_input(model, map, reach, changed); + input.coverage_admitted = variants[i].admitted; + input.suite_triggers = &trigger; + input.suite_trigger_count = 1; + cbm_test_selection_t *result = cbm_test_select(&input); + ASSERT_NOT_NULL(result); + ASSERT_EQ(cbm_test_selection_run_all(result), 0); + int count; + const cbm_test_selected_suite_t *suites = cbm_test_selection_suites(result, &count); + ASSERT_EQ(count, 1); + ASSERT_TRUE(suites[0].reasons & variants[i].path); + if (r == 0) { + ASSERT_TRUE(suites[0].whole); + ASSERT_TRUE(suites[0].reasons & CBM_TEST_SELECT_CONDITIONAL); + cbm_test_selection_cases(result, &count); + ASSERT_EQ(count, 0); + } else { + /* Registered in every build: the strict contract stays. */ + ASSERT_FALSE(suites[0].reasons & CBM_TEST_SELECT_CONDITIONAL); + } + cbm_test_selection_free(result); + cbm_coverage_map_free(map); + } + cbm_test_model_free(model); + } + PASS(); +} + /* Uncertainty is scoped to the file it was found in. A file the model cannot * read with certainty, but which defines its own suite, makes that suite * uncertain (it can only run whole) and leaves every other suite narrowable. @@ -3804,6 +3913,7 @@ SUITE(test_impact) { RUN_TEST(test_selection_setup_and_rejected_artifacts_select_whole_suites); RUN_TEST(test_selection_incomplete_global_evidence_never_narrows); RUN_TEST(test_selection_inventory_ambiguity_and_conditionals_use_whole_suite); + RUN_TEST(test_selection_conditionally_registered_suite_is_conditional_on_every_path); RUN_TEST(test_model_uncertainty_is_scoped_to_the_suites_of_its_file); RUN_TEST(test_model_only_code_continuations_are_uncertain); RUN_TEST(test_selection_scoped_uncertainty_runs_only_its_suite_whole); @@ -3834,6 +3944,7 @@ SUITE(test_impact) { RUN_TEST(test_model_ignores_comments_and_literals); RUN_TEST(test_model_reads_suites_written_as_functions); RUN_TEST(test_model_marks_what_is_under_a_condition); + RUN_TEST(test_model_marks_runner_suites_registered_under_a_condition); RUN_TEST(test_model_keeps_brace_depth_across_else_branches); RUN_TEST(test_model_resolves_registrations_in_their_own_file); RUN_TEST(test_model_marks_suites_that_register_through_a_macro); From fe19efb8b5d3a30a7e6fb771e0a71d87fd13ac50 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Sun, 18 Oct 2026 17:10:45 +0200 Subject: [PATCH 2/2] test(test-impact): make the conditional-path fixture a valid coverage map A coverage map must cover every function it lists, and five of the seven variants covered only function 1, so the map failed to parse and the test stopped at its setup ("map is NULL") with the fix applied or not. The changed function (0) is now covered by the setup row of a suite no runner registers, so alpha's own rows stay quiet and each variant still reaches alpha by exactly one path. RED with the selection fix reverted (alpha lacks CONDITIONAL), green with it. Signed-off-by: Martin Vogel --- tests/test_test_impact.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/test_test_impact.c b/tests/test_test_impact.c index 210aee288b..17b4b3ccba 100644 --- a/tests/test_test_impact.c +++ b/tests/test_test_impact.c @@ -1720,9 +1720,12 @@ TEST(test_selection_inventory_ambiguity_and_conditionals_use_whole_suite) { * runner that lacks it then tolerates its absence. Its cases and registrations * are unconditional here, so only the runner registration can say so. */ TEST(test_selection_conditionally_registered_suite_is_conditional_on_every_path) { + /* A map covers every function it lists, so the changed one (0) is covered + * by the setup of a suite no runner registers: alpha's own rows stay + * quiet and each variant reaches alpha by its one path. */ static const char *quiet_rows = "alpha:*\tcomplete\t\t1\nalpha:stat\tcomplete\t\t1\n" "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\n" - "alpha:quiet\tcomplete\t\t1\n"; + "alpha:quiet\tcomplete\t\t1\nomega:*\tcomplete\t\t0\n"; struct { const char *rows; bool admitted; @@ -1730,7 +1733,8 @@ TEST(test_selection_conditionally_registered_suite_is_conditional_on_every_path) unsigned path; } variants[] = { {"alpha:*\tincomplete\tlost setup\t1\nalpha:stat\tcomplete\t\t1\n" - "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n", + "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n" + "omega:*\tcomplete\t\t0\n", true, false, false, false, CBM_TEST_SELECT_SETUP_UNKNOWN}, {"alpha:*\tcomplete\t\t0\nalpha:stat\tcomplete\t\t1\n" "alpha:cov\tcomplete\t\t1\nalpha:changed\tcomplete\t\t1\nalpha:quiet\tcomplete\t\t1\n",