Skip to content
Merged
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
16 changes: 15 additions & 1 deletion coverage-service/lib/src/prompts/test-generation.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { inferJavaScriptTestFilePath, inferSourceImportPath, inferTestFilePath, sourceFileExtension } from '../test-paths';

Check failure on line 1 in coverage-service/lib/src/prompts/test-generation.ts

View workflow job for this annotation

GitHub Actions / lint-typecheck-test (22)

'inferJavaScriptTestFilePath' is defined but never used. Allowed unused vars must match /^_/u
import type { TestGenerationContext } from '../types';

function formatRepoPackages(packages: string[]): string {
Expand Down Expand Up @@ -182,6 +182,18 @@
: 'Use Jest APIs matching existing tests in the repo.';

const hasExports = (ctx.exportedSymbols?.length ?? 0) > 0;
const isTypeScript = ctx.language === 'typescript';

const typeScriptSection = isTypeScript
? `
## TypeScript (critical)
- NEVER read \`.ts\` source with fs.readFileSync and eval/new Function — TypeScript syntax fails at runtime.
- Use the repo test runner from Repository Packages (vitest, jest, etc.) with normal imports of the module under test.
- Mock all external I/O: database (Sequelize/db), Redis, HTTP, filesystem, dynamic import() side effects.
- Prefer testing pure exported functions with vi.mock/jest.mock for dependencies — match patterns in Existing Tests.
- Do not import paths that only exist after compilation unless the repo already uses tsx/ts-jest that way.
`
: '';

return `You are writing JavaScript unit tests for ONE production file. Tests must pass on first run with no manual fixes.

Expand Down Expand Up @@ -215,6 +227,7 @@

All Symbols in File:
${symbols || '(none detected)'}
${typeScriptSection}

## Test runner
${frameworkSection}
Expand Down Expand Up @@ -282,7 +295,7 @@
return `

## Repair (attempt ${ctx.attemptNumber ?? 2})
The previous generated test failed when executed. Fix the test file so it compiles, runs, and passes.
The previous generated test failed when executed. **Fix the test file so it compiles, runs, and passes** — failed tests are never included in the PR.

Failure output:
${ctx.failureLogs}
Expand All @@ -294,6 +307,7 @@
- Fix syntax, import paths, mocks, and assertions only — do not change production code.
- Reuse existing fixtures/mocks from Existing Tests when possible.
- Do not add new npm/pip dependencies unless declared via test-deps header.
- For TypeScript: never use fs.readFileSync + new Function on .ts source; use proper imports and mocks.
- Return the complete corrected test file only.`;
}

Expand Down
18 changes: 13 additions & 5 deletions coverage-service/worker/src/processors/pr-analysis.processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -429,13 +429,23 @@ export class PrAnalysisProcessor {

totalGenerationAttempts += outcome.attempts;

if (outcome.test) {
if (outcome.test && outcome.passed) {
generatedTests.push(outcome.test);
generatedTestResults.push({
filePath: outcome.test.filePath,
passed: outcome.passed,
passed: true,
});
declaredTestDeps.push(...outcome.declaredDeps);
} else if (outcome.test && !outcome.passed) {
await this.log(
data.prRunId,
'warn',
`Excluding ${outcome.test.filePath} from PR — test did not pass after ${outcome.attempts} repair attempt(s)`,
);
generatedTestResults.push({
filePath: outcome.test.filePath,
passed: false,
});
}
} catch (err) {
const msg = `Test generation failed for ${file.path}: ${(err as Error).message}`;
Expand Down Expand Up @@ -923,7 +933,6 @@ export class PrAnalysisProcessor {
}> {
let lastFailureLogs = '';
let previousContent = '';
let lastTest: GeneratedTest | null = null;
const declaredDeps: string[] = [];

for (let attempt = 1; attempt <= this.maxGenerationAttempts; attempt++) {
Expand Down Expand Up @@ -952,7 +961,6 @@ export class PrAnalysisProcessor {
generated.content = parsed.content;
declaredDeps.push(...parsed.declaredDeps);
previousContent = generated.content;
lastTest = generated;

const testDir = join(
params.runDir,
Expand Down Expand Up @@ -1068,7 +1076,7 @@ export class PrAnalysisProcessor {
}

return {
test: lastTest,
test: null,
passed: false,
attempts: this.maxGenerationAttempts,
declaredDeps,
Expand Down
47 changes: 47 additions & 0 deletions service/src/jobs/processors/coverage-analysis.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,53 @@ describe('processCoverageAnalysis', () => {
expect(prAuthor.openOrUpdatePR).not.toHaveBeenCalled();
});

it('does not open a stacked PR when generated tests failed validation', async () => {
makeRuntime();

const coverageClient = {
findOrCreateRepository: vi.fn(async () => baseRepo),
triggerAnalysis: vi.fn(async () => ({ prRunId: 'run-1', status: 'enqueued' })),
waitForPrRun: vi.fn(
async (): Promise<PrRun> => ({
...baseRun,
generatedTestFiles: [
{
id: 't1',
filePath: 'tests/match.service.test.ts',
targetFile: 'src/match.service.ts',
passed: false,
fileContent: 'broken test content',
},
],
}),
),
};
const prAuthor = {
branchExists: vi.fn(),
commitFiles: vi.fn(),
openOrUpdatePR: vi.fn(),
};

await processCoverageAnalysis(job, {
auth,
logger: makeLogger(),
cfg,
coverageClientFactory: () =>
coverageClient as unknown as Pick<
CoverageServiceClient,
'findOrCreateRepository' | 'triggerAnalysis' | 'waitForPrRun'
>,
prAuthorFactory: () =>
prAuthor as unknown as Pick<
PRAuthor,
'branchExists' | 'commitFiles' | 'openOrUpdatePR'
>,
});

expect(prAuthor.commitFiles).not.toHaveBeenCalled();
expect(prAuthor.openOrUpdatePR).not.toHaveBeenCalled();
});

it('throws when the coverage run terminates with FAILED', async () => {
makeRuntime();
const coverageClient = {
Expand Down
45 changes: 31 additions & 14 deletions service/src/jobs/processors/coverage-analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,32 @@ export async function processCoverageAnalysis(
let testPrUrl: string | null = null;
let testPrFileCount = 0;

if (run.status === 'COMPLETED' && run.generatedTestFiles.length > 0) {
const { url, count } = await openStackedTestPR({
job,
run,
client,
cfg: deps.cfg,
log,
prAuthorFactory: deps.prAuthorFactory,
});
testPrUrl = url;
testPrFileCount = count;
if (run.status === 'COMPLETED') {
const passingFiles = run.generatedTestFiles.filter(
(t) => t.passed === true && t.fileContent.length > 0,
);

if (passingFiles.length > 0) {
const { url, count } = await openStackedTestPR({
job,
run,
files: passingFiles,
client,
cfg: deps.cfg,
log,
prAuthorFactory: deps.prAuthorFactory,
});
testPrUrl = url;
testPrFileCount = count;
} else if (run.generatedTestFiles.length > 0) {
log.warn(
{
generated: run.generatedTestFiles.length,
passed: 0,
},
'generated tests did not pass validation; skipping stacked PR',
);
}
}

// ------------------------------------------------------------------ //
Expand All @@ -187,16 +202,18 @@ export async function processCoverageAnalysis(
async function openStackedTestPR(args: {
job: CoverageAnalysisJob;
run: PrRun;
files: PrRun['generatedTestFiles'];
client: GitHubClient;
cfg: ServiceConfig;
log: Logger;
prAuthorFactory?: CoverageAnalysisDeps['prAuthorFactory'];
}): Promise<{ url: string | null; count: number }> {
const { job, run, client, cfg, log } = args;

const files = run.generatedTestFiles
.filter((t) => t.fileContent.length > 0)
.map((t) => ({ path: t.filePath, content: t.fileContent }));
const files = args.files.map((t) => ({
path: t.filePath,
content: t.fileContent,
}));

if (files.length === 0) {
log.warn('coverage service reported tests with empty content; skipping PR');
Expand Down
Loading