diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 06057e28b..d08ae5882 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3461,6 +3461,14 @@ static void extract_func_def(CBMExtractCtx *ctx, TSNode node, const CBMLangSpec def.is_test = true; } + // A function/method defined in a file cbm treats as a test file is itself + // a test, regardless of its own name (tests/helpers/fixtures.c has none + // of the test_/_test naming conventions, but every function in it is + // still test code) (#1294). OR'd so the Rust attribute check above, which + // additionally catches #[test] functions embedded in an otherwise regular + // .rs file, is never downgraded by this. + def.is_test = def.is_test || ctx->result->is_test_file; + // Docstring def.docstring = extract_docstring(a, node, ctx->source, ctx->language); @@ -4425,6 +4433,10 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node, // MinHash fingerprint compute_fingerprint(ctx, &def, child); + // A method on a class defined in a test file (e.g. a JUnit/pytest + // TestFoo.test_bar) is itself a test, same as free functions (#1294). + def.is_test = def.is_test || ctx->result->is_test_file; + cbm_defs_push(&ctx->result->defs, a, def); } diff --git a/internal/cbm/helpers.c b/internal/cbm/helpers.c index 027c82a21..4b1b78ddd 100644 --- a/internal/cbm/helpers.c +++ b/internal/cbm/helpers.c @@ -295,6 +295,19 @@ bool cbm_is_test_file(const char *rel_path, CBMLanguage lang) { } const char *base = path_basename(rel_path); + /* Directory-based, language-agnostic: a file under a conventional test + * directory is a test file regardless of its own basename (e.g. + * tests/helpers/fixtures.c, which no per-language suffix/prefix rule + * below would otherwise catch). Mirrors the directory set cbm_is_test_path + * (src/pipeline/pass_tests.c) already uses for TESTS-edge detection, so + * the extraction-time is_test_file flag agrees with it (#1294). */ + if (strstr(rel_path, "__tests__/") || strstr(rel_path, "/tests/") || + strstr(rel_path, "/test/") || strstr(rel_path, "/spec/") || + has_prefix(rel_path, "tests/") || has_prefix(rel_path, "test/") || + has_prefix(rel_path, "spec/") || has_prefix(rel_path, "__tests__/")) { + return true; + } + switch (lang) { case CBM_LANG_GO: return has_suffix(base, "_test.go"); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 9cafa0836..df9cd05e7 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -5628,14 +5628,23 @@ static yyjson_doc *resolve_trace_edge_types(const char *args, const char *mode, return NULL; } -/* Check if a file path looks like a test file. */ +/* Check if a file path looks like a test file. The substring checks below + * only catch a tests/ directory nested under another path component + * (".../tests/foo"); a project-root-relative path like "tests/repro/foo.c" + * has no leading slash before "tests" and fell through undetected, leaking + * whole test subtrees into query_graph/trace_path results with the default + * include_tests=false (#1294). */ static bool is_test_file(const char *path) { if (!path) { return false; } return strstr(path, "/test") != NULL || strstr(path, "test_") != NULL || strstr(path, "_test.") != NULL || strstr(path, "/tests/") != NULL || - strstr(path, "/spec/") != NULL || strstr(path, ".test.") != NULL; + strstr(path, "/spec/") != NULL || strstr(path, ".test.") != NULL || + strncmp(path, "tests/", SLEN("tests/")) == 0 || + strncmp(path, "test/", SLEN("test/")) == 0 || + strncmp(path, "spec/", SLEN("spec/")) == 0 || + strncmp(path, "__tests__/", SLEN("__tests__/")) == 0; } /* Convert BFS traversal results into a yyjson_mut array. */ diff --git a/tests/test_extraction.c b/tests/test_extraction.c index 0122200e2..9777a41dc 100644 --- a/tests/test_extraction.c +++ b/tests/test_extraction.c @@ -4093,6 +4093,111 @@ TEST(extract_rust_test_attr_marks_is_test_issue855) { PASS(); } +/* Function.is_test must follow the file's test-directory membership, not just + * a test_/_test basename convention: a helper under tests/ with none of those + * naming conventions (tests/helpers/fixtures.c) was previously indexed as an + * ordinary, non-test Function — invisible to store.c's `is_test != 1` + * dead-code/search filters even though trace_path's own independent path + * check already treated the same file as a test (#1294). */ +TEST(extract_c_test_dir_marks_is_test_issue1294) { + const char *src = "void helper(void) {}\n"; + + /* Regression: a test_*-named file directly under tests/ must remain a + * test (this already worked before the fix). */ + CBMFileResult *r1 = extract(src, CBM_LANG_C, "t", "tests/test_pipeline.c"); + ASSERT_NOT_NULL(r1); + ASSERT_FALSE(r1->has_error); + ASSERT(has_def(r1, "Function", "helper")); + int reg = -1; + for (int i = 0; i < r1->defs.count; i++) { + if (strcmp(r1->defs.items[i].label, "Function") == 0 && r1->defs.items[i].name && + strcmp(r1->defs.items[i].name, "helper") == 0) { + reg = r1->defs.items[i].is_test ? 1 : 0; + } + } + ASSERT(reg == 1 && "test_*.c directly under tests/ is a test (regression)"); + cbm_free_result(r1); + + /* Positive: a file under tests/ that matches none of the test_/_test + * naming conventions must now ALSO be a test. */ + CBMFileResult *r2 = extract(src, CBM_LANG_C, "t", "tests/helpers/fixtures.c"); + ASSERT_NOT_NULL(r2); + ASSERT_FALSE(r2->has_error); + ASSERT(has_def(r2, "Function", "helper")); + int nonconv = -1; + for (int i = 0; i < r2->defs.count; i++) { + if (strcmp(r2->defs.items[i].label, "Function") == 0 && r2->defs.items[i].name && + strcmp(r2->defs.items[i].name, "helper") == 0) { + nonconv = r2->defs.items[i].is_test ? 1 : 0; + } + } + ASSERT(nonconv == 1 && "non-test_-named file under tests/ is now a test"); + cbm_free_result(r2); + + /* Negative: a file with no test/ directory or test_/_test naming in its + * path must never be flagged — this fix must not widen detection beyond + * the tests/ (and sibling) directory tree. */ + CBMFileResult *r3 = extract(src, CBM_LANG_C, "t", "src/pipeline/helper.c"); + ASSERT_NOT_NULL(r3); + ASSERT_FALSE(r3->has_error); + ASSERT(has_def(r3, "Function", "helper")); + int outside = -1; + for (int i = 0; i < r3->defs.count; i++) { + if (strcmp(r3->defs.items[i].label, "Function") == 0 && r3->defs.items[i].name && + strcmp(r3->defs.items[i].name, "helper") == 0) { + outside = r3->defs.items[i].is_test ? 1 : 0; + } + } + ASSERT(outside == 0 && "file outside tests/ is never a test"); + cbm_free_result(r3); + + PASS(); +} + +/* Same convergence for Method definitions (push_method_def), which take a + * separate code path from free functions: a class method on a class defined + * under tests/ with no test_/_test naming (e.g. tests/helpers/base.py) must + * be marked is_test too, and a method on the same class outside tests/ must + * not be (#1294). */ +TEST(extract_python_method_test_dir_marks_is_test_issue1294) { + const char *src = "class Foo:\n" + " def helper(self):\n" + " pass\n"; + + /* Python's LSP layer injects synthetic builtin stub Methods (str.upper, + * dict.get, ...) into defs.items alongside real ones (py_builtins.c), so + * matching must key on name, not just label="Method". */ + CBMFileResult *r1 = extract(src, CBM_LANG_PYTHON, "t", "tests/helpers/base.py"); + ASSERT_NOT_NULL(r1); + ASSERT_FALSE(r1->has_error); + ASSERT(has_def(r1, "Method", "helper")); + int in_tests = -1; + for (int i = 0; i < r1->defs.count; i++) { + if (strcmp(r1->defs.items[i].label, "Method") == 0 && r1->defs.items[i].name && + strcmp(r1->defs.items[i].name, "helper") == 0) { + in_tests = r1->defs.items[i].is_test ? 1 : 0; + } + } + ASSERT(in_tests == 1 && "method on a class under tests/helpers/ is a test"); + cbm_free_result(r1); + + CBMFileResult *r2 = extract(src, CBM_LANG_PYTHON, "t", "app/models/base.py"); + ASSERT_NOT_NULL(r2); + ASSERT_FALSE(r2->has_error); + ASSERT(has_def(r2, "Method", "helper")); + int outside_tests = -1; + for (int i = 0; i < r2->defs.count; i++) { + if (strcmp(r2->defs.items[i].label, "Method") == 0 && r2->defs.items[i].name && + strcmp(r2->defs.items[i].name, "helper") == 0) { + outside_tests = r2->defs.items[i].is_test ? 1 : 0; + } + } + ASSERT(outside_tests == 0 && "method on a class outside tests/ is never a test"); + cbm_free_result(r2); + + PASS(); +} + /* #1017: docstring truncation at MAX_COMMENT_LEN (500 bytes) can split a * multi-byte UTF-8 character, leaving an incomplete byte sequence. * Craft a Go comment whose 498th-500th bytes are a 3-byte CJK character @@ -5340,6 +5445,8 @@ SUITE(extraction) { RUN_TEST(extract_c_clean_file_no_recovery_duplicates_issue961); RUN_TEST(walk_defs_no_truncation_over_4096_issue668); RUN_TEST(extract_rust_test_attr_marks_is_test_issue855); + RUN_TEST(extract_c_test_dir_marks_is_test_issue1294); + RUN_TEST(extract_python_method_test_dir_marks_is_test_issue1294); RUN_TEST(docstring_utf8_truncation_boundary_issue1017); RUN_TEST(extract_ts_decorators_survive_interleaved_comment); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index d9a0c43a2..ccb10e3ef 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -1679,6 +1679,77 @@ TEST(tool_trace_totals_respect_test_filter) { PASS(); } +/* Same as above, but the test-side caller lives directly under a + * project-root tests/ directory with no test_/_test naming convention + * (tests/repro/helper.c) rather than tests/test_x.c. is_test_file() in + * mcp.c matched a NESTED ".../tests/..." path but not a project-root- + * relative one, so this row leaked into results with the default + * include_tests=false (#1294, secondary bug). */ +TEST(tool_trace_totals_respect_test_filter_tests_root_subtree_issue1294) { + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + const char *proj = "totproj2"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, "/tmp/tot2"); + + cbm_node_t tgt = {.project = proj, + .label = "Function", + .name = "tgt2", + .qualified_name = "totproj2.a.tgt2", + .file_path = "a.c", + .start_line = 1, + .end_line = 5}; + int64_t tid = cbm_store_upsert_node(st, &tgt); + ASSERT_GT(tid, 0); + cbm_node_t prod = {.project = proj, + .label = "Function", + .name = "prod_caller2", + .qualified_name = "totproj2.a.prod_caller2", + .file_path = "a.c", + .start_line = 10, + .end_line = 15}; + int64_t pid = cbm_store_upsert_node(st, &prod); + ASSERT_GT(pid, 0); + cbm_node_t tst = {.project = proj, + .label = "Function", + .name = "repro_caller", + .qualified_name = "totproj2.t.repro_caller", + .file_path = "tests/repro/helper.c", + .start_line = 1, + .end_line = 5}; + int64_t xid = cbm_store_upsert_node(st, &tst); + ASSERT_GT(xid, 0); + cbm_edge_t e1 = {.project = proj, .source_id = pid, .target_id = tid, .type = "CALLS"}; + ASSERT_GT(cbm_store_insert_edge(st, &e1), 0); + cbm_edge_t e2 = {.project = proj, .source_id = xid, .target_id = tid, .type = "CALLS"}; + ASSERT_GT(cbm_store_insert_edge(st, &e2), 0); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":92,\"method\":\"tools/call\",\"params\":{" + "\"name\":\"trace_call_path\",\"arguments\":{\"project\":\"totproj2\"," + "\"function_name\":\"tgt2\",\"direction\":\"inbound\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "callers_total: 1")); /* tests/repro/ row filtered by default */ + free(inner); + + resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":93,\"method\":\"tools/call\",\"params\":{" + "\"name\":\"trace_call_path\",\"arguments\":{\"project\":\"totproj2\"," + "\"function_name\":\"tgt2\",\"direction\":\"inbound\",\"include_tests\":true}}}"); + ASSERT_NOT_NULL(resp); + inner = extract_text_content(resp); + free(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "callers_total: 2")); /* both visible now */ + free(inner); + cbm_mcp_server_free(srv); + PASS(); +} + /* SCC condensation (get_architecture aspect "cycles"): a 3-function CALLS * cycle A->B->C->A must be reported as one circular dependency of size 3 with * all three members; a separate acyclic chain (D->E) must NOT appear. The @@ -10139,6 +10210,7 @@ SUITE(mcp) { RUN_TEST(tool_unknown_tool); RUN_TEST(tool_search_graph_basic); RUN_TEST(tool_trace_totals_respect_test_filter); + RUN_TEST(tool_trace_totals_respect_test_filter_tests_root_subtree_issue1294); RUN_TEST(tool_get_architecture_cycles_detects_scc); RUN_TEST(tool_get_code_snippet_clips_whole_file_node); RUN_TEST(tool_search_graph_includes_node_properties);