From 6bcc3672b98dfed58823bf03e6d6fd696c2a6f5b Mon Sep 17 00:00:00 2001 From: Kenil Shah Date: Mon, 22 Jun 2026 09:52:09 +0530 Subject: [PATCH] feat: add test file preparation and path resolution utilities --- coverage-service/lib/src/index.ts | 1 + .../lib/src/prepare-test-generation.ts | 54 ++++++++ .../lib/src/prompts/test-generation.ts | 42 +++++- .../lib/src/providers/anthropic-provider.ts | 6 +- .../lib/src/providers/github-provider.ts | 81 ++++++------ .../lib/src/providers/local-llm-provider.ts | 6 +- .../lib/src/providers/openai-provider.ts | 6 +- .../lib/src/providers/repository-provider.ts | 6 +- coverage-service/lib/src/test-paths.ts | 121 ++++++++++++++++++ coverage-service/lib/src/types/index.ts | 4 + .../worker/src/lib/js-coverage.ts | 24 +++- .../src/processors/pr-analysis.processor.ts | 41 ++++-- .../processors/test-generation.processor.ts | 16 +-- tests/coverage-service/test-paths.test.ts | 97 ++++++++++++++ 14 files changed, 432 insertions(+), 73 deletions(-) create mode 100644 coverage-service/lib/src/prepare-test-generation.ts create mode 100644 tests/coverage-service/test-paths.test.ts diff --git a/coverage-service/lib/src/index.ts b/coverage-service/lib/src/index.ts index 7661a66..f605b6b 100644 --- a/coverage-service/lib/src/index.ts +++ b/coverage-service/lib/src/index.ts @@ -14,6 +14,7 @@ export * from './providers/anthropic-provider'; export * from './providers/local-llm-provider'; export * from './prompts/test-generation'; export * from './test-paths'; +export * from './prepare-test-generation'; export * from './analysis/code-analyzer'; import { AnthropicProvider } from './providers/anthropic-provider'; diff --git a/coverage-service/lib/src/prepare-test-generation.ts b/coverage-service/lib/src/prepare-test-generation.ts new file mode 100644 index 0000000..0874af5 --- /dev/null +++ b/coverage-service/lib/src/prepare-test-generation.ts @@ -0,0 +1,54 @@ +import type { RepositoryProvider } from './providers/repository-provider'; +import { resolveTestFileTarget } from './test-paths'; + +export interface PreparedTestFileContext { + existingTestPaths: string[]; + existingTests: string; + testOutputPath: string; + isUpdatingExistingTest: boolean; +} + +/** + * Discover existing tests for a source file and decide whether to update + * an existing test file or create a new one at the inferred path. + */ +export async function prepareTestFileContext( + repoProvider: RepositoryProvider, + repoDir: string, + sourceFile: string, + framework: string, +): Promise { + const existingTestPaths = await repoProvider.findExistingTests( + repoDir, + sourceFile, + framework, + ); + + const { testOutputPath, isUpdatingExistingTest } = resolveTestFileTarget( + existingTestPaths, + sourceFile, + framework, + ); + + const pathsToRead = isUpdatingExistingTest + ? [testOutputPath] + : existingTestPaths; + + const parts = await Promise.all( + pathsToRead.map((p) => repoProvider.getFileContent(repoDir, p)), + ); + const existingTests = + parts + .map((content, i) => + content.length > 0 ? `// ${pathsToRead[i]}\n${content}` : '', + ) + .filter(Boolean) + .join('\n\n---\n\n') || '(none found)'; + + return { + existingTestPaths, + existingTests, + testOutputPath, + isUpdatingExistingTest, + }; +} diff --git a/coverage-service/lib/src/prompts/test-generation.ts b/coverage-service/lib/src/prompts/test-generation.ts index f0b09cc..62897ab 100644 --- a/coverage-service/lib/src/prompts/test-generation.ts +++ b/coverage-service/lib/src/prompts/test-generation.ts @@ -1,4 +1,4 @@ -import { inferJavaScriptTestFilePath, inferSourceImportPath, sourceFileExtension } from '../test-paths'; +import { inferJavaScriptTestFilePath, inferSourceImportPath, inferTestFilePath, sourceFileExtension } from '../test-paths'; import type { TestGenerationContext } from '../types'; function formatRepoPackages(packages: string[]): string { @@ -28,12 +28,43 @@ function formatExportedSymbols(ctx: TestGenerationContext): string { return ctx.exportedSymbols.map((s) => `- ${s}`).join('\n'); } +function outputTestPath(ctx: TestGenerationContext): string { + return ctx.testOutputPath ?? inferTestFilePath(ctx.file, ctx.framework); +} + +function formatTestCountGuidance(ctx: TestGenerationContext): string { + const exports = ctx.exportedSymbols ?? []; + if (exports.length > 4) { + return `- Write focused tests covering **each exported symbol** (${exports.length} total: ${exports.join(', ')}). At least one test per symbol, especially for uncovered lines.`; + } + if (exports.length > 0) { + return `- Write 2–${Math.max(4, exports.length)} focused tests covering exported symbols (${exports.join(', ')}). At least one behavior per symbol when uncovered.`; + } + return '- Write 2–4 short, focused tests. One behavior per test.'; +} + +function formatTestOutputMode(ctx: TestGenerationContext): string { + const path = outputTestPath(ctx); + const countGuidance = formatTestCountGuidance(ctx); + if (ctx.isUpdatingExistingTest) { + return `## Mode: UPDATE existing test file +- Output path: \`${path}\` (this file already exists — do NOT create a new file elsewhere). +- Keep all existing tests that still apply; do not remove or rewrite passing tests unnecessarily. +${countGuidance} +- Do not duplicate test names or behaviors already present in Existing Tests. +- Return the **complete updated test file** (existing + new tests).`; + } + return `## Mode: CREATE new test file +- Output path: \`${path}\` +${countGuidance}`; +} + function buildPythonPrompt(ctx: TestGenerationContext, symbols: string): string { return `You are writing Python unit tests for ONE production file. Tests must pass on first run with no manual fixes. ## Scope - Generate ONLY the complete test file content. No markdown, no explanations, no extra files unless required for imports to work. -- Write 2–4 short, focused tests. One behavior per test. +${formatTestOutputMode(ctx)} - Test the file: ${ctx.file} ${formatFullSourceHeader(ctx)}## Context @@ -134,7 +165,7 @@ Return only the complete test file.`; } function buildJavaScriptPrompt(ctx: TestGenerationContext, symbols: string): string { - const testFile = inferJavaScriptTestFilePath(ctx.file); + const testFile = outputTestPath(ctx); const importPath = inferSourceImportPath(testFile, ctx.file); const importExt = sourceFileExtension(ctx.file); const repoHasJest = ctx.repoPackages.some((p) => normalizePackageName(p) === 'jest'); @@ -156,9 +187,8 @@ Do NOT use @jest/globals, jest, vitest, or mocha unless they appear in Repositor ## Scope - Generate ONLY the complete test file content. No markdown, no explanations. -- Write 2–4 short, focused tests. One behavior per test. +${formatTestOutputMode(ctx)} - Production file: ${ctx.file} -- Save tests to: ${testFile} ${formatFullSourceHeader(ctx)}## Context @@ -290,6 +320,7 @@ export function buildTestGenerationSystemPrompt(language: string): string { return ( 'You write simple, correct JavaScript unit tests that pass on first run. ' + 'Prefer node:test and node:assert unless the repo already uses Jest/Vitest. ' + + 'When updating an existing test file, preserve working tests and append new cases for uncovered lines. ' + 'Use correct relative ESM import paths with the source file extension (.js, .jsx, .ts, .tsx). ' + 'Only import symbols listed as exported; never import private functions. ' + 'For CLI files with no exports, test via child_process.spawnSync instead of importing. ' + @@ -300,6 +331,7 @@ export function buildTestGenerationSystemPrompt(language: string): string { return ( 'You write simple, correct unit tests that pass on first run. Mock all external I/O. ' + + 'When updating an existing test file, preserve working tests and append new cases for uncovered lines. ' + 'Use exact types and defaults from source models. Return only runnable test file code. ' + 'Declare minimal extra test-only packages on the first line as # test-deps: pkg1, pkg2' ); diff --git a/coverage-service/lib/src/providers/anthropic-provider.ts b/coverage-service/lib/src/providers/anthropic-provider.ts index c59a4ae..6b41b43 100644 --- a/coverage-service/lib/src/providers/anthropic-provider.ts +++ b/coverage-service/lib/src/providers/anthropic-provider.ts @@ -4,6 +4,10 @@ import type { GeneratedTest, TestGenerationContext } from '../types'; import type { TestGenerationProvider } from './test-generation-provider'; +function outputPathFor(context: TestGenerationContext): string { + return context.testOutputPath ?? inferTestFilePath(context.file, context.framework); +} + export interface AnthropicProviderConfig { apiKey: string; model?: string; @@ -43,7 +47,7 @@ export class AnthropicProvider implements TestGenerationProvider { const content = raw.replace(/^```[\w]*\n?/gm, '').replace(/```$/gm, '').trim(); return { - filePath: inferTestFilePath(context.file, context.framework), + filePath: outputPathFor(context), content, targetFile: context.file, }; diff --git a/coverage-service/lib/src/providers/github-provider.ts b/coverage-service/lib/src/providers/github-provider.ts index 11ea13a..b282110 100644 --- a/coverage-service/lib/src/providers/github-provider.ts +++ b/coverage-service/lib/src/providers/github-provider.ts @@ -7,6 +7,7 @@ import { createAppAuth } from '@octokit/auth-app'; import { Octokit } from '@octokit/rest'; import type { ChangedFile } from '../types'; +import { orderedTestFileCandidates } from '../test-paths'; import type { CheckoutPROptions, @@ -261,59 +262,55 @@ export class GitHubProvider implements RepositoryProvider { async findExistingTests( repoDir: string, sourceFile: string, + framework = 'pytest', ): Promise { const { readdir } = await import('fs/promises'); - const baseName = sourceFile.replace(/\.[^.]+$/, ''); - const dir = join(repoDir, sourceFile.split('/').slice(0, -1).join('/')); - const patterns = [ - `${baseName}.test.ts`, - `${baseName}.spec.ts`, - `${baseName}.test.js`, - `${baseName}.spec.js`, - `test_${baseName.split('/').pop()}.py`, - ]; - const results: string[] = []; - for (const pattern of patterns) { - const full = join(dir, pattern.split('/').pop()!); - if (existsSync(full)) results.push(full); - } + const found = new Set(); + const add = (rel: string) => found.add(rel.replace(/\\/g, '/')); - try { - const entries = await readdir(join(repoDir, 'test'), { recursive: true }); - for (const entry of entries) { - const name = String(entry); - if ( - name.includes(baseName.split('/').pop()!) && - (name.endsWith('.test.ts') || - name.endsWith('.spec.ts') || - name.startsWith('test_')) - ) { - results.push(join(repoDir, 'test', name)); - } + for (const candidate of orderedTestFileCandidates(sourceFile, framework)) { + const full = join(repoDir, candidate); + if (existsSync(full)) { + add(candidate); } - } catch { - // no test directory } - try { - const entries = await readdir(join(repoDir, 'tests'), { recursive: true }); - for (const entry of entries) { - const name = String(entry); - if ( - name.includes(baseName.split('/').pop()!) && - (name.endsWith('.test.ts') || - name.endsWith('.spec.ts') || - name.startsWith('test_')) - ) { - results.push(join(repoDir, 'tests', name)); + const baseName = sourceFile.split('/').pop()!.replace(/\.[^.]+$/, ''); + + for (const testRoot of ['tests', 'test', 'src'] as const) { + try { + const entries = await readdir(join(repoDir, testRoot), { + recursive: true, + }); + for (const entry of entries) { + const name = String(entry); + if ( + !name.includes(baseName) || + !( + name.endsWith('.test.ts') || + name.endsWith('.test.tsx') || + name.endsWith('.test.js') || + name.endsWith('.test.jsx') || + name.endsWith('.spec.ts') || + name.endsWith('.spec.js') || + name.startsWith('test_') + ) + ) { + continue; + } + const rel = `${testRoot}/${name}`.replace(/\\/g, '/'); + const full = join(repoDir, rel); + if (existsSync(full)) { + add(rel); + } } + } catch { + // no directory } - } catch { - // no tests directory } - return results; + return [...found]; } private async runGit(args: string[], cwd?: string): Promise { diff --git a/coverage-service/lib/src/providers/local-llm-provider.ts b/coverage-service/lib/src/providers/local-llm-provider.ts index 21617d4..0e0c7e7 100644 --- a/coverage-service/lib/src/providers/local-llm-provider.ts +++ b/coverage-service/lib/src/providers/local-llm-provider.ts @@ -4,6 +4,10 @@ import type { GeneratedTest, TestGenerationContext } from '../types'; import type { TestGenerationProvider } from './test-generation-provider'; +function outputPathFor(context: TestGenerationContext): string { + return context.testOutputPath ?? inferTestFilePath(context.file, context.framework); +} + export interface LocalLLMProviderConfig { baseUrl?: string; model?: string; @@ -41,7 +45,7 @@ export class LocalLLMProvider implements TestGenerationProvider { .trim(); return { - filePath: inferTestFilePath(context.file, context.framework), + filePath: outputPathFor(context), content, targetFile: context.file, }; diff --git a/coverage-service/lib/src/providers/openai-provider.ts b/coverage-service/lib/src/providers/openai-provider.ts index 98342c7..38a5988 100644 --- a/coverage-service/lib/src/providers/openai-provider.ts +++ b/coverage-service/lib/src/providers/openai-provider.ts @@ -7,6 +7,10 @@ import type { GeneratedTest, TestGenerationContext } from '../types'; import type { TestGenerationProvider } from './test-generation-provider'; +function outputPathFor(context: TestGenerationContext): string { + return context.testOutputPath ?? inferTestFilePath(context.file, context.framework); +} + export interface OpenAIProviderConfig { apiKey: string; model?: string; @@ -50,7 +54,7 @@ export class OpenAIProvider implements TestGenerationProvider { const content = this.stripMarkdown(data.choices[0]?.message?.content ?? ''); return { - filePath: inferTestFilePath(context.file, context.framework), + filePath: outputPathFor(context), content, targetFile: context.file, }; diff --git a/coverage-service/lib/src/providers/repository-provider.ts b/coverage-service/lib/src/providers/repository-provider.ts index 1bf1eb0..9a98343 100644 --- a/coverage-service/lib/src/providers/repository-provider.ts +++ b/coverage-service/lib/src/providers/repository-provider.ts @@ -25,5 +25,9 @@ export interface RepositoryProvider { headBranch: string, ): Promise; getFileContent(repoDir: string, filePath: string): Promise; - findExistingTests(repoDir: string, sourceFile: string): Promise; + findExistingTests( + repoDir: string, + sourceFile: string, + framework?: string, + ): Promise; } diff --git a/coverage-service/lib/src/test-paths.ts b/coverage-service/lib/src/test-paths.ts index a820dc5..52fd362 100644 --- a/coverage-service/lib/src/test-paths.ts +++ b/coverage-service/lib/src/test-paths.ts @@ -21,6 +21,22 @@ export function colocatedJavaScriptTestPaths(sourceFile: string): string[] { return JS_COLOCATED_TEST_SUFFIXES.map((suffix) => `${baseName}${suffix}`); } +/** Common __test__ / __tests__ directory layouts (e.g. src/utils/__test__/mathUtils.test.js). */ +export function nestedJavaScriptTestPaths(sourceFile: string): string[] { + const dir = sourceFile.split('/').slice(0, -1).join('/'); + const base = sourceFile.split('/').pop()!.replace(/\.[^.]+$/, ''); + const ext = sourceFileExtension(sourceFile); + const suffixes = ['.test', '.spec'] as const; + + const paths: string[] = []; + for (const folder of ['__test__', '__tests__'] as const) { + for (const suffix of suffixes) { + paths.push(`${dir}/${folder}/${base}${suffix}${ext}`); + } + } + return paths; +} + export function isJavaScriptTestFileName(fileName: string): boolean { return ( JS_COLOCATED_TEST_SUFFIXES.some((suffix) => fileName.endsWith(suffix)) || @@ -75,3 +91,108 @@ export function inferTestFilePath(sourceFile: string, framework: string): string } return inferJavaScriptTestFilePath(sourceFile); } + +/** All plausible test paths for a source file (tests/ mirror first). */ +export function orderedTestFileCandidates( + sourceFile: string, + framework: string, +): string[] { + const seen = new Set(); + const candidates: string[] = []; + const add = (path: string) => { + if (seen.has(path)) return; + seen.add(path); + candidates.push(path); + }; + + add(inferTestFilePath(sourceFile, framework)); + + for (const path of nestedJavaScriptTestPaths(sourceFile)) { + add(path); + } + + for (const path of colocatedJavaScriptTestPaths(sourceFile)) { + add(path); + } + + if (framework.toLowerCase().includes('pytest')) { + const base = sourceFile.split('/').pop()!.replace(/\.py$/, ''); + add(`test_${base}.py`); + } + + return candidates; +} + +function normalizeRepoPath(path: string): string { + return path.replace(/\\/g, '/').replace(/^\.\//, ''); +} + +/** + * Pick an existing test file to update. Prefers the tests/ mirror path + * (same as inferTestFilePath) when it exists on disk. + */ +export function pickExistingTestFilePath( + existingRelativePaths: readonly string[], + sourceFile: string, + framework: string, +): string | null { + if (existingRelativePaths.length === 0) return null; + + const normalized = new Map( + existingRelativePaths.map((p) => [normalizeRepoPath(p), p]), + ); + + for (const candidate of orderedTestFileCandidates(sourceFile, framework)) { + const match = normalized.get(normalizeRepoPath(candidate)); + if (match) return match; + } + + const mirror = normalizeRepoPath(inferTestFilePath(sourceFile, framework)); + const underTests = existingRelativePaths.filter((p) => { + const n = normalizeRepoPath(p); + return n.startsWith('tests/') || n.startsWith('test/'); + }); + if (underTests.length > 0) { + const mirrorMatch = underTests.find( + (p) => normalizeRepoPath(p) === mirror, + ); + if (mirrorMatch) return mirrorMatch; + return [...underTests].sort()[0]!; + } + + const baseName = sourceFile.split('/').pop()!.replace(/\.[^.]+$/, ''); + const nestedMatch = existingRelativePaths.find((p) => { + const n = normalizeRepoPath(p); + return ( + (n.includes('/__test__/') || n.includes('/__tests__/')) && + n.includes(baseName) + ); + }); + if (nestedMatch) return nestedMatch; + + if (existingRelativePaths.length === 1) { + return existingRelativePaths[0]!; + } + + return null; +} + +/** Resolve where generated tests should be written (update vs create). */ +export function resolveTestFileTarget( + existingRelativePaths: readonly string[], + sourceFile: string, + framework: string, +): { testOutputPath: string; isUpdatingExistingTest: boolean } { + const existing = pickExistingTestFilePath( + existingRelativePaths, + sourceFile, + framework, + ); + if (existing) { + return { testOutputPath: existing, isUpdatingExistingTest: true }; + } + return { + testOutputPath: inferTestFilePath(sourceFile, framework), + isUpdatingExistingTest: false, + }; +} diff --git a/coverage-service/lib/src/types/index.ts b/coverage-service/lib/src/types/index.ts index 349e10b..421c1ca 100644 --- a/coverage-service/lib/src/types/index.ts +++ b/coverage-service/lib/src/types/index.ts @@ -120,6 +120,10 @@ export interface TestGenerationContext { failureLogs?: string; previousTestContent?: string; attemptNumber?: number; + /** Repo-relative path where the generated test file will be written. */ + testOutputPath?: string; + /** True when updating an existing test file instead of creating a new one. */ + isUpdatingExistingTest?: boolean; } export interface PrAnalysisJobData { diff --git a/coverage-service/worker/src/lib/js-coverage.ts b/coverage-service/worker/src/lib/js-coverage.ts index dc0f23e..898572c 100644 --- a/coverage-service/worker/src/lib/js-coverage.ts +++ b/coverage-service/worker/src/lib/js-coverage.ts @@ -181,6 +181,9 @@ function walkJsTestFiles(dir: string, repoDir: string, results: string[]): void function guessTestFileNames(sourcePath: string): string[] { const normalized = normalizePath(sourcePath); const baseName = normalized.split('/').pop()?.replace(/\.[^.]+$/, '') ?? ''; + const dir = normalized.includes('/') + ? normalized.replace(/\/[^/]+$/, '') + : ''; const dirPart = normalized.includes('/') ? normalized.replace(/\.[^.]+$/, '').replace(/\//g, '_') : baseName; @@ -192,6 +195,12 @@ function guessTestFileNames(sourcePath: string): string[] { return [ `${underTests}.test${ext}`, `${underTests}.spec${ext}`, + ...(dir + ? [ + `${dir}/__test__/${baseName}.test${ext}`, + `${dir}/__tests__/${baseName}.test${ext}`, + ] + : []), `${baseName}.test${ext}`, `${baseName}.spec${ext}`, `test_${baseName}${ext}`, @@ -231,9 +240,18 @@ export async function collectJsTestPaths( } for (const sourcePath of sourcePaths) { - const existing = await repoProvider.findExistingTests(repoDir, sourcePath); - for (const absPath of existing) { - testPaths.add(normalizePath(relative(repoDir, absPath))); + const existing = await repoProvider.findExistingTests( + repoDir, + sourcePath, + 'node:test', + ); + for (const testPath of existing) { + const rel = normalizePath( + testPath.startsWith(repoDir) + ? relative(repoDir, testPath) + : testPath, + ); + testPaths.add(rel); } for (const candidate of guessTestFileNames(sourcePath)) { diff --git a/coverage-service/worker/src/processors/pr-analysis.processor.ts b/coverage-service/worker/src/processors/pr-analysis.processor.ts index 63e1edd..3bf70f8 100644 --- a/coverage-service/worker/src/processors/pr-analysis.processor.ts +++ b/coverage-service/worker/src/processors/pr-analysis.processor.ts @@ -27,6 +27,7 @@ import { selectFilesForGeneration, classifyCoverageBlockers, buildWorkflowSummary, + prepareTestFileContext, } from '@openreview/coverage-lib'; import type { Prisma } from '@prisma/client'; import type { Job } from 'bullmq'; @@ -395,6 +396,21 @@ export class PrAnalysisProcessor { `Generating tests for ${file.path} (diff coverage ${fileDiff ?? 'n/a'}%, full PR branch source)`, ); try { + const framework = detectFramework(runDir, detectLanguage(file.path), testCommand); + const testCtx = await prepareTestFileContext( + this.repoProvider, + runDir, + file.path, + framework, + ); + if (testCtx.isUpdatingExistingTest) { + await this.log( + data.prRunId, + 'info', + `Updating existing test file: ${testCtx.testOutputPath}`, + ); + } + const outcome = await this.generateAndValidateTest({ prRunId: data.prRunId, runDir, @@ -510,11 +526,17 @@ export class PrAnalysisProcessor { 'Recalculating coverage after generated tests', ); + // Run only generated tests for post-coverage — pre-existing broken tests + // (e.g. src/**/__test__ with bad ESM imports) must not fail the recalc. const postTestPaths = useCoveragePackageOnly - ? [...new Set([...pythonTestPaths, ...generatedTestPaths])] + ? generatedTestPaths.length > 0 + ? generatedTestPaths + : pythonTestPaths : useAutoJsCoverage - ? [...new Set([...jsTestPaths, ...generatedTestPaths])] - : []; + ? generatedTestPaths.length > 0 + ? generatedTestPaths + : jsTestPaths + : generatedTestPaths; const postCoverageCommand = useCoveragePackageOnly ? repoSetup.wrapCommand( buildPythonCoverageCommand(sourcePaths, postTestPaths, runDir), @@ -1091,15 +1113,12 @@ export class PrAnalysisProcessor { const symbols = await analyzer.extractSymbols(source, filePath, []); const exportedSymbols = extractExportedSymbols(source, filePath); - const existingTestPaths = await this.repoProvider.findExistingTests( + const testFile = await prepareTestFileContext( + this.repoProvider, repoDir, filePath, + framework, ); - const existingTests = ( - await Promise.all( - existingTestPaths.map((p) => this.repoProvider.getFileContent(repoDir, p)), - ) - ).join('\n\n---\n\n'); return this.llmProvider.generateTests({ language, @@ -1107,7 +1126,7 @@ export class PrAnalysisProcessor { file: filePath, diff, source, - existingTests, + existingTests: testFile.existingTests, uncoveredLines: uncoveredForFile.join(', ') || 'unknown', symbols, repoPackages, @@ -1117,6 +1136,8 @@ export class PrAnalysisProcessor { failureLogs: repair?.failureLogs, previousTestContent: repair?.previousTestContent, attemptNumber: repair?.attemptNumber, + testOutputPath: testFile.testOutputPath, + isUpdatingExistingTest: testFile.isUpdatingExistingTest, }); } diff --git a/coverage-service/worker/src/processors/test-generation.processor.ts b/coverage-service/worker/src/processors/test-generation.processor.ts index 9619556..f74ae32 100644 --- a/coverage-service/worker/src/processors/test-generation.processor.ts +++ b/coverage-service/worker/src/processors/test-generation.processor.ts @@ -19,6 +19,7 @@ import { pickTargetFileForTestGeneration, getTestThresholdPercent, pathsMatch, + prepareTestFileContext, } from '@openreview/coverage-lib'; import type { Job } from 'bullmq'; @@ -472,17 +473,12 @@ export class TestGenerationProcessor { ); const exportedSymbols = extractExportedSymbols(source, params.filePath); - const existingTestPaths = await this.repoProvider.findExistingTests( + const testFile = await prepareTestFileContext( + this.repoProvider, params.runDir, params.filePath, + framework, ); - const existingTests = ( - await Promise.all( - existingTestPaths.map((p) => - this.repoProvider.getFileContent(params.runDir, p), - ), - ) - ).join('\n\n---\n\n'); return this.llmProvider.generateTests({ language, @@ -490,13 +486,15 @@ export class TestGenerationProcessor { file: params.filePath, diff, source, - existingTests, + existingTests: testFile.existingTests, uncoveredLines: uncoveredForFile.join(', ') || 'unknown', symbols, repoPackages: params.repoPackages, useFullSource: true, fileDiffCoverage: fileCoverageEntry?.diffCoveragePercent ?? null, exportedSymbols, + testOutputPath: testFile.testOutputPath, + isUpdatingExistingTest: testFile.isUpdatingExistingTest, }); } diff --git a/tests/coverage-service/test-paths.test.ts b/tests/coverage-service/test-paths.test.ts new file mode 100644 index 0000000..a10bce2 --- /dev/null +++ b/tests/coverage-service/test-paths.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from 'vitest'; + +import { + pickExistingTestFilePath, + resolveTestFileTarget, +} from '../../coverage-service/lib/src/test-paths'; + +describe('pickExistingTestFilePath', () => { + it('prefers the tests/ mirror over colocated tests', () => { + const picked = pickExistingTestFilePath( + [ + 'src/utils/mathUtils.test.js', + 'tests/utils/mathUtils.test.js', + ], + 'src/utils/mathUtils.js', + 'node:test', + ); + expect(picked).toBe('tests/utils/mathUtils.test.js'); + }); + + it('returns null when no existing tests match', () => { + expect( + pickExistingTestFilePath([], 'src/foo.js', 'node:test'), + ).toBeNull(); + }); + + it('falls back to tests/ directory match when mirror name differs slightly', () => { + const picked = pickExistingTestFilePath( + ['tests/utils/mathUtils.spec.js'], + 'src/utils/mathUtils.js', + 'node:test', + ); + expect(picked).toBe('tests/utils/mathUtils.spec.js'); + }); +}); + +describe('resolveTestFileTarget', () => { + it('creates at inferred path when no tests exist', () => { + expect( + resolveTestFileTarget([], 'src/utils/mathUtils.js', 'node:test'), + ).toEqual({ + testOutputPath: 'tests/utils/mathUtils.test.js', + isUpdatingExistingTest: false, + }); + }); + + it('updates existing tests/ mirror when present', () => { + expect( + resolveTestFileTarget( + ['tests/utils/mathUtils.test.js'], + 'src/utils/mathUtils.js', + 'node:test', + ), + ).toEqual({ + testOutputPath: 'tests/utils/mathUtils.test.js', + isUpdatingExistingTest: true, + }); + }); + + it('uses pytest path for python sources', () => { + expect( + resolveTestFileTarget( + ['tests/test_math_utils.py'], + 'src/math_utils.py', + 'pytest', + ), + ).toEqual({ + testOutputPath: 'tests/test_math_utils.py', + isUpdatingExistingTest: true, + }); + }); + + it('updates __test__ directory layout when present', () => { + expect( + resolveTestFileTarget( + ['src/utils/__test__/mathUtils.test.js'], + 'src/utils/mathUtils.js', + 'node:test', + ), + ).toEqual({ + testOutputPath: 'src/utils/__test__/mathUtils.test.js', + isUpdatingExistingTest: true, + }); + }); + + it('prefers tests/ mirror over __test__ when both exist', () => { + const picked = pickExistingTestFilePath( + [ + 'src/utils/__test__/mathUtils.test.js', + 'tests/utils/mathUtils.test.js', + ], + 'src/utils/mathUtils.js', + 'node:test', + ); + expect(picked).toBe('tests/utils/mathUtils.test.js'); + }); +});