Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions internal/cbm/extract_defs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}

Expand Down
13 changes: 13 additions & 0 deletions internal/cbm/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
13 changes: 11 additions & 2 deletions src/mcp/mcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
107 changes: 107 additions & 0 deletions tests/test_extraction.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
72 changes: 72 additions & 0 deletions tests/test_mcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Loading