From 35146108b4066f7498d435c91a8f8f583f1101e8 Mon Sep 17 00:00:00 2001 From: NiharDoshi99 <51595473+NiharDoshi99@users.noreply.github.com> Date: Tue, 23 Jun 2026 22:05:33 +0530 Subject: [PATCH] Revert "Fix unit tests generation" --- .../lib/src/prompts/test-generation.ts | 16 +------ .../src/processors/pr-analysis.processor.ts | 18 ++----- .../jobs/processors/coverage-analysis.test.ts | 47 ------------------- .../src/jobs/processors/coverage-analysis.ts | 45 ++++++------------ 4 files changed, 20 insertions(+), 106 deletions(-) diff --git a/coverage-service/lib/src/prompts/test-generation.ts b/coverage-service/lib/src/prompts/test-generation.ts index cdd8ac3..62897ab 100644 --- a/coverage-service/lib/src/prompts/test-generation.ts +++ b/coverage-service/lib/src/prompts/test-generation.ts @@ -182,18 +182,6 @@ Do NOT use @jest/globals, jest, vitest, or mocha unless they appear in Repositor : '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. @@ -227,7 +215,6 @@ ${ctx.uncoveredLines} All Symbols in File: ${symbols || '(none detected)'} -${typeScriptSection} ## Test runner ${frameworkSection} @@ -295,7 +282,7 @@ function buildRepairSection(ctx: TestGenerationContext): string { return ` ## Repair (attempt ${ctx.attemptNumber ?? 2}) -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. +The previous generated test failed when executed. Fix the test file so it compiles, runs, and passes. Failure output: ${ctx.failureLogs} @@ -307,7 +294,6 @@ Requirements: - 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.`; } diff --git a/coverage-service/worker/src/processors/pr-analysis.processor.ts b/coverage-service/worker/src/processors/pr-analysis.processor.ts index c184ef3..3bf70f8 100644 --- a/coverage-service/worker/src/processors/pr-analysis.processor.ts +++ b/coverage-service/worker/src/processors/pr-analysis.processor.ts @@ -429,23 +429,13 @@ export class PrAnalysisProcessor { totalGenerationAttempts += outcome.attempts; - if (outcome.test && outcome.passed) { + if (outcome.test) { generatedTests.push(outcome.test); generatedTestResults.push({ filePath: outcome.test.filePath, - passed: true, + passed: outcome.passed, }); 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}`; @@ -933,6 +923,7 @@ export class PrAnalysisProcessor { }> { let lastFailureLogs = ''; let previousContent = ''; + let lastTest: GeneratedTest | null = null; const declaredDeps: string[] = []; for (let attempt = 1; attempt <= this.maxGenerationAttempts; attempt++) { @@ -961,6 +952,7 @@ export class PrAnalysisProcessor { generated.content = parsed.content; declaredDeps.push(...parsed.declaredDeps); previousContent = generated.content; + lastTest = generated; const testDir = join( params.runDir, @@ -1076,7 +1068,7 @@ export class PrAnalysisProcessor { } return { - test: null, + test: lastTest, passed: false, attempts: this.maxGenerationAttempts, declaredDeps, diff --git a/service/src/jobs/processors/coverage-analysis.test.ts b/service/src/jobs/processors/coverage-analysis.test.ts index 71c3518..656c894 100644 --- a/service/src/jobs/processors/coverage-analysis.test.ts +++ b/service/src/jobs/processors/coverage-analysis.test.ts @@ -307,53 +307,6 @@ 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 => ({ - ...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 = { diff --git a/service/src/jobs/processors/coverage-analysis.ts b/service/src/jobs/processors/coverage-analysis.ts index d59fa19..833fe23 100644 --- a/service/src/jobs/processors/coverage-analysis.ts +++ b/service/src/jobs/processors/coverage-analysis.ts @@ -150,32 +150,17 @@ export async function processCoverageAnalysis( let testPrUrl: string | null = null; let testPrFileCount = 0; - 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', - ); - } + 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; } // ------------------------------------------------------------------ // @@ -202,7 +187,6 @@ export async function processCoverageAnalysis( async function openStackedTestPR(args: { job: CoverageAnalysisJob; run: PrRun; - files: PrRun['generatedTestFiles']; client: GitHubClient; cfg: ServiceConfig; log: Logger; @@ -210,10 +194,9 @@ async function openStackedTestPR(args: { }): Promise<{ url: string | null; count: number }> { const { job, run, client, cfg, log } = args; - const files = args.files.map((t) => ({ - path: t.filePath, - content: t.fileContent, - })); + const files = run.generatedTestFiles + .filter((t) => t.fileContent.length > 0) + .map((t) => ({ path: t.filePath, content: t.fileContent })); if (files.length === 0) { log.warn('coverage service reported tests with empty content; skipping PR');