From dd598ef4262454763503974bdbcac1c53783b83b Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 6 Oct 2026 18:07:16 +0200 Subject: [PATCH] fix(test-impact): a changed code line is never a comment line Seeding drops a hunk whose lines are all comments. The check took any line starting with `*` for a block-comment line and any line ending in a block comment's close for a comment, so a dereference (`*out = f(a, b, true);`) and code with a trailing comment counted as comments and their edits seeded nothing. On the oracle, PR #2158 changed two `*out_* = extract_route_path_from_args(...)` lines in scan_route_annotations. Both hunks were dropped, the function was never a seed, and its coverage id never reached the changed set. The six artifact_reconcile_* tests that execute it, recorded as complete coverage rows of the merge-base map, were missed (6 per job, two jobs: the only misses among 191 jobs). With the check fixed, all six are selected for COVERAGE again; until now the new graph walk had reached them as STATIC only by chance. A line now counts as a comment only when it is nothing but comment: blank, `//`, a block opened with nothing after its close, `*` followed by whitespace or the end, a lone close, or the closing line of a block opened earlier. `*name`, `**`, `*(` and code before a trailing comment are code. When unsure the check says code, which only seeds more. Signed-off-by: Martin Vogel --- src/mcp/test_impact_source.c | 49 +++++++++++++++++++++++++---- tests/test_test_impact_seed.c | 56 +++++++++++++++++++++++++++++++++ tests/test_test_impact_source.c | 40 ++++++++++++++++++++++- 3 files changed, 138 insertions(+), 7 deletions(-) diff --git a/src/mcp/test_impact_source.c b/src/mcp/test_impact_source.c index 25a1ffca8..6fb66dfeb 100644 --- a/src/mcp/test_impact_source.c +++ b/src/mcp/test_impact_source.c @@ -127,17 +127,54 @@ bool cbm_ti_is_keyword(const char *word, size_t len) { /* ── Line patterns ─────────────────────────────────────────────────── */ -/* `^\s*(//.*|/\*.*|\*.*|.*\*\/\s*)?$` */ +/* The offset of the first "*" + "/" in s[from..to), or `to`. */ +static size_t ti_comment_close(const char *s, size_t from, size_t to) { + for (size_t i = from; i + 1 < to; i++) { + if (s[i] == '*' && s[i + 1] == '/') { + return i; + } + } + return to; +} + +/* A line that is nothing but comment: blank; `//`; a block opened here with + * nothing after its first close; a block-comment line (`*` then whitespace or + * the end); a close with nothing after it; or the closing line of a block + * opened earlier (ends in the close, opens none). Anything else is code: a + * line starting `*name`, `**` or `*(` dereferences, and code before a + * trailing comment is a change. The reference pattern + * `^\s*(//.*|/\*.*|\*.*|.*\*\/\s*)?$` took both for comments, and a PR's + * `*out = f(..., true);` edits seeded nothing. A line-local view cannot tell + * `* text` from a multiplication continuation; the project's format keeps + * binary operators at line ends, so that spelling stays a comment line. */ bool cbm_ti_line_is_comment(const char *s, size_t n) { size_t i = ti_skip_space(s, 0, n); - if (i == n || s[i] == '*') { - return true; - } - if (s[i] == '/' && i + 1 < n && (s[i + 1] == '/' || s[i + 1] == '*')) { + if (i == n) { return true; } size_t e = ti_trim_end(s, i, n); - return e - i >= 2 && s[e - 2] == '*' && s[e - 1] == '/'; + if (s[i] == '/' && i + 1 < e && s[i + 1] == '/') { + return true; + } + if (s[i] == '/' && i + 1 < e && s[i + 1] == '*') { + size_t close = ti_comment_close(s, i + 2, e); + return close == e || close + 2 == e; + } + if (s[i] == '*') { + if (i + 1 < e && s[i + 1] == '/') { + return i + 2 == e; + } + return i + 1 == e || ti_space(s[i + 1]); + } + if (e - i >= 2 && s[e - 2] == '*' && s[e - 1] == '/') { + for (size_t k = i; k + 1 < e; k++) { + if (s[k] == '/' && s[k + 1] == '*') { + return false; + } + } + return true; + } + return false; } /* `^\s*#\s*(if|ifdef|ifndef|elif|else|endif|undef|include)\b` */ diff --git a/tests/test_test_impact_seed.c b/tests/test_test_impact_seed.c index 47f3c9c01..9ac19dc29 100644 --- a/tests/test_test_impact_seed.c +++ b/tests/test_test_impact_seed.c @@ -153,6 +153,61 @@ TEST(test_impact_seed_hunks_follow_the_reference) { PASS(); } +/* A line that starts with a dereference is code, and so is code with a + * trailing comment: each such change seeds its definition. The comment + * filter read `*out = f(...)` as a block-comment line, so a PR's two real + * edits in scan_route_annotations seeded nothing, and six tests that execute + * it were not selected (oracle job pr2158). A comment-only edit still + * seeds nothing. */ +TEST(test_impact_seed_dereference_and_trailing_comment_lines_are_code) { + const char *files[] = {"src/a.c", "void f(int *out) {\n *out = g(1);\n}\n" + "int h(void) {\n return 1; /* one */\n}\n"}; + tsd_fixture_t f; + ASSERT_TRUE(tsd_open(&f, files, 2)); + int64_t fn = tsd_node(&f, "Function", "f", "src/a.c", 1, 3); + int64_t h = tsd_node(&f, "Function", "h", "src/a.c", 4, 6); + + static const char *const deref[] = {" *out = g(1, true);"}; + static const char *const old_deref[] = {" *out = g(1);"}; + cbm_diff_hunk_t edit = {.start = 2, + .count = 1, + .added = deref, + .added_count = 1, + .removed = old_deref, + .removed_count = 1}; + cbm_ti_change_t change = {.path = "src/a.c", .hunks = &edit, .hunk_count = 1}; + cbm_ti_seeds_t *s = tsd_seed(&f, &change, 1); + ASSERT_NOT_NULL(s); + ASSERT_EQ(tsd_seed_count(s), 1); + ASSERT_TRUE(tsd_seeded(s, fn)); + cbm_ti_seeds_free(s); + + static const char *const value[] = {" return 2; /* one */"}; + static const char *const old_value[] = {" return 1; /* one */"}; + cbm_diff_hunk_t trailing = {.start = 5, + .count = 1, + .added = value, + .added_count = 1, + .removed = old_value, + .removed_count = 1}; + change.hunks = &trailing; + s = tsd_seed(&f, &change, 1); + ASSERT_NOT_NULL(s); + ASSERT_EQ(tsd_seed_count(s), 1); + ASSERT_TRUE(tsd_seeded(s, h)); + cbm_ti_seeds_free(s); + + static const char *const note[] = {" * why the dereference stays"}; + cbm_diff_hunk_t comment = {.start = 2, .count = 1, .added = note, .added_count = 1}; + change.hunks = &comment; + s = tsd_seed(&f, &change, 1); + ASSERT_NOT_NULL(s); + ASSERT_EQ(tsd_seed_count(s), 0); + cbm_ti_seeds_free(s); + tsd_close(&f); + PASS(); +} + /* A changed test case or suite body is reported, never seeded. */ TEST(test_impact_seed_changed_cases_and_suites_are_reported) { const char *files[] = {"tests/t.c", "TEST(alpha) {\n PASS();\n}\n" @@ -372,6 +427,7 @@ TEST(test_impact_seed_variant_spans_seed_their_definition) { SUITE(test_impact_seed) { RUN_TEST(test_impact_seed_hunks_follow_the_reference); + RUN_TEST(test_impact_seed_dereference_and_trailing_comment_lines_are_code); RUN_TEST(test_impact_seed_variant_spans_seed_their_definition); RUN_TEST(test_impact_seed_changed_cases_and_suites_are_reported); RUN_TEST(test_impact_seed_deleted_names_are_found_by_text); diff --git a/tests/test_test_impact_source.c b/tests/test_test_impact_source.c index e76cd0d38..7c7c4002c 100644 --- a/tests/test_test_impact_source.c +++ b/tests/test_test_impact_source.c @@ -33,7 +33,8 @@ static const tis_line_t tis_lines[] = { {"// x", true, false, "", "", "", "", "", ""}, {" /* x", true, false, "", "", "", "", "", ""}, {" * x", true, false, "", "", "", "", "", ""}, - {"int x; /* c */", true, false, "", "", "", "", "", ""}, + /* Code with a trailing comment is code (the reference called it a comment). */ + {"int x; /* c */", false, false, "", "", "", "", "", ""}, {"int x;", false, false, "", "", "", "", "", ""}, {"x */ ", true, false, "", "", "", "", "", ""}, {"*/", true, false, "", "", "", "", "", ""}, @@ -136,6 +137,42 @@ TEST(test_impact_source_line_patterns_match_the_reference) { PASS(); } +/* Only a line that is nothing but comment counts as one: a change to any + * other line is a change to code. A dereference at the start of a line read + * as a block-comment line once and dropped a real edit's seed. */ +TEST(test_impact_source_only_whole_comment_lines_are_comments) { + static const struct { + const char *line; + bool comment; + } cases[] = { + {" *out_jax_path = extract_route_path_from_args(a, args, source, true);", + false}, + {"*out = 1;", false}, + {" **pp = q;", false}, + {" *(p + 1) = 0;", false}, + {" x = f(); /* why */", false}, + {"/* a */ x = 1;", false}, + {"*/ x = 1;", false}, + {" * a block-comment line", true}, + {" *", true}, + {" */", true}, + {"\t*\tindented */", true}, + {"/* whole */", true}, + {"/** doc", true}, + {" end of the comment */", true}, + {"// line", true}, + {"", true}, + }; + for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) { + if (cbm_ti_line_is_comment(cases[i].line, strlen(cases[i].line)) != cases[i].comment) { + printf(" comment-line case %zu: \"%s\" expected %d\n", i, cases[i].line, + cases[i].comment); + FAIL("a line was classified wrongly"); + } + } + PASS(); +} + /* The declaration of a changed header line tries define, typedef, tag, * extern, prototype in that order. */ TEST(test_impact_source_declaration_tries_the_patterns_in_order) { @@ -316,6 +353,7 @@ TEST(test_impact_source_lines_split_like_the_reference) { SUITE(test_impact_source) { RUN_TEST(test_impact_source_line_patterns_match_the_reference); + RUN_TEST(test_impact_source_only_whole_comment_lines_are_comments); RUN_TEST(test_impact_source_declaration_tries_the_patterns_in_order); RUN_TEST(test_impact_source_macros_carry_body_names_and_pastes); RUN_TEST(test_impact_source_paste_scan_follows_the_reference);