mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-08 03:08:13 +00:00
fix(mcp): honor includeTests for C#, Java, Swift and PHP test paths (#2866)
* fix(mcp): honor includeTests for C#, Java, Swift and PHP test paths
Test-file classification had two hand-maintained implementations that had drifted:
core/ingestion/entry-point-scoring.ts isTestFile — excludes tests from
process entry points
mcp/local/local-backend.ts isTestFilePath — backs `includeTests`
on impact/trace/context
The MCP copy recognized no C#, Java or Swift test convention and neither PHP
form, so `includeTests: false` silently failed to filter them — a C# project's
`*.Tests/` and a Maven project's `src/test/` landed in blast-radius output as
though they were production callers. The scoring copy missed `/fixtures/` and
`/conftest.`, so those could be selected as process entry points.
Both now delegate to one predicate carrying the union of the two pattern sets.
Public names are unchanged, so importers are unaffected.
The duplication was not gratuitous: `entry-point-scoring.ts` imports the
language-provider registry, and #2802 deliberately cut that closure out of MCP
server startup — importing it from `local-backend.ts` would put it back. The
shared predicate therefore lives in its own module with NO imports, and a test
asserts it declares none, so the startup cost cannot be reintroduced by a future
import added there.
19 new tests: the previously-missed paths per language, nullish and
Windows-separator handling, production paths that must NOT match, and a
cross-check that both public names agree on every case — the regression guard for
the drift itself.
Behavior-neutral on a Python/Go/TypeScript repository (204,336 nodes / 299,580
edges / 813 flows before and after), since the newly-recognized patterns are
languages it does not contain. `npx tsc --noEmit` clean; 728 tests pass across
the entry-point, process, impact and test-file suites.
* Address PR review feedback (#2866)
Tighten the shared test-path predicate so includeTests filtering no longer
treats Contest.swift/Latest.php as tests, unanchored uitests/ as a substring,
or production /fixtures/ trees as test code.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Simplify the shared test-path matcher
Drop substring needles already covered by /test/, /tests/, and /spec/,
and inline the one-off slash-prefix helper.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#2866)
Correct the module header: scoring already matched /test/, so
/test/fixtures/ was never the scoring gap — only /conftest. was.
Co-authored-by: Cursor <cursoragent@cursor.com>
* Address PR review feedback (#2866)
Classify Xcode *UITests path segments without restoring the
unanchored uitests/ substring that also matches fruitests.
Co-authored-by: Cursor <cursoragent@cursor.com>
---------
Co-authored-by: Carter LaSalle <carterlasalle@gmail.com>
Co-authored-by: Gergő Magyar <gergomagyar@icloud.com>
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
f34daea86a
commit
c283b21e9a
4 changed files with 204 additions and 71 deletions
|
|
@ -13,6 +13,7 @@
|
|||
import { detectFrameworkFromPath } from './framework-detection.js';
|
||||
import { SupportedLanguages } from 'gitnexus-shared';
|
||||
import { providers } from './languages/index.js';
|
||||
import { isTestFilePath } from './utils/test-file-path.js';
|
||||
|
||||
// ============================================================================
|
||||
// NAME PATTERNS
|
||||
|
|
@ -164,54 +165,15 @@ export function calculateEntryPointScore(
|
|||
// ============================================================================
|
||||
|
||||
/**
|
||||
* Check if a file path is a test file (should be excluded from entry points)
|
||||
* Covers common test file patterns across all supported languages
|
||||
* Check if a file path is a test file (should be excluded from entry points).
|
||||
*
|
||||
* Delegates to the shared predicate in `utils/test-file-path.ts`. This used to be
|
||||
* a second, hand-maintained copy that had drifted from the one backing the MCP
|
||||
* `includeTests` flag — see that module's header. Re-exported under this name so
|
||||
* existing importers are unaffected.
|
||||
*/
|
||||
export function isTestFile(filePath: string): boolean {
|
||||
const p = filePath.toLowerCase().replace(/\\/g, '/');
|
||||
|
||||
return (
|
||||
// JavaScript/TypeScript test patterns
|
||||
p.includes('.test.') ||
|
||||
p.includes('.spec.') ||
|
||||
p.includes('__tests__/') ||
|
||||
p.includes('__mocks__/') ||
|
||||
// Generic test folders
|
||||
p.includes('/test/') ||
|
||||
p.includes('/tests/') ||
|
||||
p.includes('/testing/') ||
|
||||
// Python test patterns
|
||||
p.endsWith('_test.py') ||
|
||||
p.includes('/test_') ||
|
||||
// Go test patterns
|
||||
p.endsWith('_test.go') ||
|
||||
// Java test patterns
|
||||
p.includes('/src/test/') ||
|
||||
// Rust test patterns (inline tests are different, but test files)
|
||||
p.includes('/tests/') ||
|
||||
// Swift/iOS test patterns
|
||||
p.endsWith('tests.swift') ||
|
||||
p.endsWith('test.swift') ||
|
||||
p.includes('uitests/') ||
|
||||
// C# test patterns
|
||||
p.endsWith('tests.cs') ||
|
||||
p.endsWith('test.cs') ||
|
||||
p.includes('.tests/') ||
|
||||
p.includes('.test/') ||
|
||||
p.includes('.integrationtests/') ||
|
||||
p.includes('.unittests/') ||
|
||||
p.includes('/testproject/') ||
|
||||
// PHP/Laravel test patterns
|
||||
p.endsWith('test.php') ||
|
||||
p.endsWith('spec.php') ||
|
||||
p.includes('/tests/feature/') ||
|
||||
p.includes('/tests/unit/') ||
|
||||
// Ruby test patterns
|
||||
p.endsWith('_spec.rb') ||
|
||||
p.endsWith('_test.rb') ||
|
||||
p.includes('/spec/') ||
|
||||
p.includes('/test/fixtures/')
|
||||
);
|
||||
return isTestFilePath(filePath);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
|||
87
gitnexus/src/core/ingestion/utils/test-file-path.ts
Normal file
87
gitnexus/src/core/ingestion/utils/test-file-path.ts
Normal file
|
|
@ -0,0 +1,87 @@
|
|||
/**
|
||||
* Test-file path classification — the single source of truth.
|
||||
*
|
||||
* WHY THIS MODULE EXISTS
|
||||
*
|
||||
* Two independent copies of this predicate existed and had drifted apart:
|
||||
*
|
||||
* - `core/ingestion/entry-point-scoring.ts` `isTestFile` — excludes test
|
||||
* files from process entry-point detection.
|
||||
* - `mcp/local/local-backend.ts` `isTestFilePath` — backs the
|
||||
* `includeTests` flag on `impact` / `trace` / `context`.
|
||||
*
|
||||
* They answered "is this a test file?" differently, so the same path could be a
|
||||
* test in one code path and not the other. The MCP copy recognized no C#, Java,
|
||||
* or Swift test convention at all, meaning `includeTests: false` silently failed
|
||||
* to filter them; the scoring copy missed `/conftest.` (it already matched
|
||||
* `/test/`, so `/test/fixtures/` was never a scoring gap).
|
||||
*
|
||||
* The duplication was not gratuitous: `entry-point-scoring.ts` imports the
|
||||
* language-provider registry, and #2802 deliberately cut that closure out of MCP
|
||||
* server startup. Importing it back into `local-backend.ts` would reintroduce
|
||||
* that cost. So the shared predicate lives here instead, with NO imports — pure
|
||||
* string matching — and both callers delegate to it.
|
||||
*
|
||||
* Keep it dependency-free. Anything imported here lands in MCP startup.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Lowercase forward-slash path substrings. Directory needles include a leading
|
||||
* slash so they match path components after the caller slash-prefixes relative
|
||||
* paths. `/test/` already covers Maven `src/test` and `/test/fixtures/`;
|
||||
* `/tests/` covers Laravel `tests/Feature` and `/tests/fixtures/`.
|
||||
*/
|
||||
const TEST_PATH_SUBSTRINGS: readonly string[] = [
|
||||
'.test.',
|
||||
'.spec.',
|
||||
'__tests__/',
|
||||
'__mocks__/',
|
||||
'/test/',
|
||||
'/tests/',
|
||||
'/testing/',
|
||||
'/spec/',
|
||||
'/test_',
|
||||
'/conftest.',
|
||||
'/uitests/',
|
||||
'.tests/',
|
||||
'.test/',
|
||||
'.integrationtests/',
|
||||
'.unittests/',
|
||||
'/testproject/',
|
||||
];
|
||||
|
||||
/** Case-insensitive suffixes that already include a delimiter (`_test.py`, not `test.py`). */
|
||||
const TEST_PATH_DELIMITED_SUFFIXES: readonly string[] = [
|
||||
'_test.py',
|
||||
'_test.go',
|
||||
'_spec.rb',
|
||||
'_test.rb',
|
||||
];
|
||||
|
||||
/**
|
||||
* Case-sensitive `Test`/`Tests`/`Spec` suffixes. Lowercasing first would also
|
||||
* match production names such as `Contest.swift` and `Latest.php`.
|
||||
*/
|
||||
const TEST_PATH_CASED_SUFFIXES: readonly string[] = [
|
||||
'Tests.swift',
|
||||
'Test.swift',
|
||||
'Tests.cs',
|
||||
'Test.cs',
|
||||
'Test.php',
|
||||
'Spec.php',
|
||||
];
|
||||
|
||||
/** Absent / empty paths are not test paths. */
|
||||
export function isTestFilePath(filePath: string | null | undefined): boolean {
|
||||
if (!filePath) return false;
|
||||
const slashed = filePath.replace(/\\/g, '/');
|
||||
const prefixed = slashed.startsWith('/') ? slashed : `/${slashed}`;
|
||||
const lower = prefixed.toLowerCase();
|
||||
if (TEST_PATH_SUBSTRINGS.some((needle) => lower.includes(needle))) return true;
|
||||
if (TEST_PATH_DELIMITED_SUFFIXES.some((suffix) => lower.endsWith(suffix))) return true;
|
||||
// Xcode `{Product}UITests` targets. Slash-anchored `/uitests/` does not match
|
||||
// `MyAppUITests`; an unanchored `uitests/` substring also matches `fruitests`.
|
||||
if (prefixed.split('/').some((seg) => seg.endsWith('UITests'))) return true;
|
||||
const basename = prefixed.slice(prefixed.lastIndexOf('/') + 1);
|
||||
return TEST_PATH_CASED_SUFFIXES.some((suffix) => basename.endsWith(suffix));
|
||||
}
|
||||
|
|
@ -29,6 +29,7 @@ import { LBUG_ID_PROBE_BATCH_SIZE, LBUG_QUERY_BATCH_SIZE } from '../../core/lbug
|
|||
import { chunk, mapConcurrent } from '../../lib/utils.js';
|
||||
import { pathSuffixOf } from './path-predicate.js';
|
||||
import { toOneBasedLine } from '../../core/ingestion/utils/line-base.js';
|
||||
import { isTestFilePath } from '../../core/ingestion/utils/test-file-path.js';
|
||||
import { isWalCorruptionError, WAL_RECOVERY_SUGGESTION } from '../../core/lbug/lbug-config.js';
|
||||
// Embedding imports are lazy (dynamic import) to avoid loading onnxruntime-node
|
||||
// at MCP server startup — crashes on unsupported Node ABI versions (#89)
|
||||
|
|
@ -313,31 +314,8 @@ function normalizeToolParams(
|
|||
// AI context generation is CLI-only (gitnexus analyze)
|
||||
// import { generateAIContextFiles } from '../../cli/ai-context.js';
|
||||
|
||||
/**
|
||||
* Quick test-file detection for filtering impact results.
|
||||
* Matches common test file patterns across all supported languages.
|
||||
*/
|
||||
export function isTestFilePath(filePath: string | null | undefined): boolean {
|
||||
if (!filePath) return false;
|
||||
const p = filePath.toLowerCase().replace(/\\/g, '/');
|
||||
return (
|
||||
p.includes('.test.') ||
|
||||
p.includes('.spec.') ||
|
||||
p.includes('__tests__/') ||
|
||||
p.includes('__mocks__/') ||
|
||||
p.includes('/test/') ||
|
||||
p.includes('/tests/') ||
|
||||
p.includes('/testing/') ||
|
||||
p.includes('/fixtures/') ||
|
||||
p.endsWith('_test.go') ||
|
||||
p.endsWith('_test.py') ||
|
||||
p.endsWith('_spec.rb') ||
|
||||
p.endsWith('_test.rb') ||
|
||||
p.includes('/spec/') ||
|
||||
p.includes('/test_') ||
|
||||
p.includes('/conftest.')
|
||||
);
|
||||
}
|
||||
/** Shared predicate; re-exported so MCP importers keep the old public name. */
|
||||
export { isTestFilePath };
|
||||
|
||||
/** Valid LadybugDB node labels for safe Cypher query construction */
|
||||
export const VALID_NODE_LABELS = new Set([
|
||||
|
|
|
|||
106
gitnexus/test/unit/test-file-path.test.ts
Normal file
106
gitnexus/test/unit/test-file-path.test.ts
Normal file
|
|
@ -0,0 +1,106 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { isTestFilePath } from '../../src/core/ingestion/utils/test-file-path.js';
|
||||
import { isTestFile } from '../../src/core/ingestion/entry-point-scoring.js';
|
||||
import { isTestFilePath as backendIsTestFilePath } from '../../src/mcp/local/local-backend.js';
|
||||
|
||||
describe('isTestFilePath — shared predicate', () => {
|
||||
it('returns false for nullish input rather than throwing', () => {
|
||||
expect(isTestFilePath(undefined)).toBe(false);
|
||||
expect(isTestFilePath(null)).toBe(false);
|
||||
expect(isTestFilePath('')).toBe(false);
|
||||
});
|
||||
|
||||
it('normalizes Windows separators and casing', () => {
|
||||
expect(isTestFilePath('SRC\\Test\\FooTests.cs')).toBe(true);
|
||||
expect(isTestFilePath('pkg\\thing_test.go')).toBe(true);
|
||||
expect(isTestFilePath('src\\Widgets.Tests\\WidgetTests.cs')).toBe(true);
|
||||
});
|
||||
|
||||
// These were recognized by entry-point scoring but NOT by the MCP copy, so
|
||||
// `includeTests: false` silently failed to filter them.
|
||||
for (const p of [
|
||||
'app/src/test/java/com/x/FooTest.java',
|
||||
'ios/MyAppTests/LoginTests.swift',
|
||||
'ios/MyAppUITests/FlowTest.swift',
|
||||
'ios/MyAppUITests/Flow.swift',
|
||||
'ios/MyAppUITests/README.swift',
|
||||
'src/Widgets.Tests/WidgetTests.cs',
|
||||
'src/Widgets.UnitTests/Thing.cs',
|
||||
'src/Widgets.IntegrationTests/Thing.cs',
|
||||
'tests/Feature/LoginTest.php',
|
||||
'tests/Unit/ThingSpec.php',
|
||||
'tests/Feature/Support/FakeGateway.php',
|
||||
]) {
|
||||
it(`detects a test path the MCP copy used to miss: ${p}`, () => {
|
||||
expect(isTestFilePath(p)).toBe(true);
|
||||
});
|
||||
}
|
||||
|
||||
// These were recognized by the MCP copy but NOT by entry-point scoring, so
|
||||
// they could be selected as process entry points.
|
||||
for (const p of ['tests/fixtures/sample.py', 'tests/conftest.py']) {
|
||||
it(`detects a test path entry-point scoring used to miss: ${p}`, () => {
|
||||
expect(isTestFilePath(p)).toBe(true);
|
||||
});
|
||||
}
|
||||
|
||||
for (const p of [
|
||||
'src/app/widgets.ts',
|
||||
'src/core/ingestion/utils/test-file-path.ts',
|
||||
'pkg/service/handler.go',
|
||||
'app/models/user.rb',
|
||||
'Contest.swift',
|
||||
'Contest.cs',
|
||||
'Protest.cs',
|
||||
'Latest.php',
|
||||
'src/fixtures/schema.ts',
|
||||
'src/fruitests/helpers.swift',
|
||||
]) {
|
||||
it(`does not classify production code as test: ${p}`, () => {
|
||||
expect(isTestFilePath(p)).toBe(false);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
describe('test-file classification has exactly one implementation', () => {
|
||||
// Regression guard: two hand-maintained copies drifted apart once already.
|
||||
// Both public names must delegate to the same predicate.
|
||||
const paths = [
|
||||
'src/app/widgets.ts',
|
||||
'app/src/test/java/com/x/FooTest.java',
|
||||
'ios/MyAppTests/LoginTests.swift',
|
||||
'src/Widgets.Tests/WidgetTests.cs',
|
||||
'tests/conftest.py',
|
||||
'tests/fixtures/sample.py',
|
||||
'src/fixtures/schema.ts',
|
||||
'Contest.swift',
|
||||
'spec/models/user_spec.rb',
|
||||
'pkg/thing_test.go',
|
||||
'tests/Feature/LoginTest.php',
|
||||
];
|
||||
|
||||
it('entry-point-scoring isTestFile agrees with the shared predicate', () => {
|
||||
for (const p of paths) expect(isTestFile(p)).toBe(isTestFilePath(p));
|
||||
});
|
||||
|
||||
it('local-backend isTestFilePath agrees with the shared predicate', () => {
|
||||
for (const p of paths) expect(backendIsTestFilePath(p)).toBe(isTestFilePath(p));
|
||||
});
|
||||
});
|
||||
|
||||
describe('shared predicate stays dependency-free', () => {
|
||||
// Poka-yoke for #2802: `local-backend.ts` imports this module, so anything
|
||||
// imported here lands in MCP server startup. The duplication this module
|
||||
// replaced existed precisely because `entry-point-scoring.ts` pulls in the
|
||||
// language-provider registry. An import added here would silently reintroduce
|
||||
// that startup cost.
|
||||
it('declares no imports', () => {
|
||||
const src = readFileSync(
|
||||
new URL('../../src/core/ingestion/utils/test-file-path.ts', import.meta.url),
|
||||
'utf8',
|
||||
);
|
||||
const imports = src.split('\n').filter((l) => /^\s*(import\b|export\s.*\sfrom\s)/.test(l));
|
||||
expect(imports).toEqual([]);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue