From 3f3ead0f084bbe8726571d6da3656d272bb55f5f Mon Sep 17 00:00:00 2001 From: Jeffery Date: Wed, 13 May 2026 06:23:45 +0000 Subject: [PATCH] test: cover review edge cases and repair paths --- .gitea/ai-review/exclusions.json | 10 ++++ app/comments.js | 12 ++--- app/comments.test.js | 31 ++++++++++++ app/findings.js | 10 ++-- app/findings.test.js | 83 ++++++++++++++++++++++++++++++++ app/git.test.js | 14 +++++- app/main.js | 75 ++++++++++++++++++----------- app/main.test.js | 49 +++++++++++++++++++ 8 files changed, 243 insertions(+), 41 deletions(-) create mode 100644 app/comments.test.js create mode 100644 app/findings.test.js create mode 100644 app/main.test.js diff --git a/.gitea/ai-review/exclusions.json b/.gitea/ai-review/exclusions.json index 1b5b004..0c2bbb8 100644 --- a/.gitea/ai-review/exclusions.json +++ b/.gitea/ai-review/exclusions.json @@ -224,6 +224,16 @@ "location": "TODO.md", "suggestion": "TODO.md 的階段編號僅供內部開發追蹤，無外部文件引用，階段編號調整不影響任何外部一致性" }, + { + "role": "Leo", + "location": "TODO.md", + "suggestion": "TODO.md 已清楚區分已驗收、部分驗收與可驗收紀錄情境，文件結構本身不是問題" + }, + { + "role": "Leo", + "location": "TODO.md", + "suggestion": "TODO.md 中針對 critical 阻擋與 JSON 驗證的驗收說明屬文件補充，不是程式碼缺陷" + }, { "role": "Rex", "location": "app/gitea.js", diff --git a/app/comments.js b/app/comments.js index a8f2198..8ec3390 100644 --- a/app/comments.js +++ b/app/comments.js @@ -28,35 +28,35 @@ export function saveFindings(workspace, findings) { /** * 發布所有舊問題 comment（一次發布，依等級排序） */ -export async function postOldFindingsComment(findings) { +export async function postOldFindingsComment(findings, postFn = postComment) { const old = findings.filter(f => !f.is_new); if (old.length === 0) { console.log(' 無舊問題，跳過'); return; } const body = `## 📋 舊有未解決問題（${old.length} 筆）\n\n${buildTable(old)}`; - await postComment(body); + await postFn(body); console.log(` ✅ 舊問題 comment 發布 (${old.length} 筆)`); } /** * 發布新問題中非 critical 的 comment（一次發布） */ -export async function postNewNonCriticalComment(findings) { +export async function postNewNonCriticalComment(findings, postFn = postComment) { const items = findings.filter(f => f.is_new && f.level !== 'critical'); if (items.length === 0) { console.log(' 無新的非嚴重問題，跳過'); return; } const body = `## 🔍 新發現問題（${items.length} 筆）\n\n${buildTable(items)}`; - await postComment(body); + await postFn(body); console.log(` ✅ 新問題（非嚴重）comment 發布 (${items.length} 筆)`); } /** * 每個新 critical 問題各發一個 comment */ -export async function postNewCriticalComments(findings) { +export async function postNewCriticalComments(findings, postFn = postComment) { const criticals = findings.filter(f => f.is_new && f.level === 'critical'); if (criticals.length === 0) { console.log(' 無新的嚴重問題，跳過'); @@ -64,7 +64,7 @@ export async function postNewCriticalComments(findings) { } for (const f of criticals) { const body = `## 🚨 嚴重問題\n\n${buildTable([f])}`; - await postComment(body); + await postFn(body); console.log(` ✅ 嚴重問題 comment 發布: [${f.role}] ${f.location}`); } } diff --git a/app/comments.test.js b/app/comments.test.js new file mode 100644 index 0000000..5f9ffa5 --- /dev/null +++ b/app/comments.test.js @@ -0,0 +1,31 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { postOldFindingsComment, postNewNonCriticalComment, postNewCriticalComments } from './comments.js'; + +describe('comment publishers', () => { + it('skips publishing when there are no matching findings', async () => { + let called = 0; + await postOldFindingsComment([], async () => { called += 1; }); + await postNewNonCriticalComment([], async () => { called += 1; }); + await postNewCriticalComments([], async () => { called += 1; }); + assert.equal(called, 0); + }); + + it('publishes old, non-critical, and critical comments in separate calls', async () => { + const bodies = []; + const findings = [ + { level: 'warning', role: 'Rex', location: 'a.js:1', suggestion: 'fix', is_new: false }, + { level: 'info', role: 'Maya', location: 'b.js:2', suggestion: 'note', is_new: true }, + { level: 'critical', role: 'Aria', location: 'c.js:3', suggestion: 'stop', is_new: true }, + ]; + + await postOldFindingsComment(findings, async body => bodies.push(body)); + await postNewNonCriticalComment(findings, async body => bodies.push(body)); + await postNewCriticalComments(findings, async body => bodies.push(body)); + + assert.equal(bodies.length, 3); + assert.match(bodies[0], /舊有未解決問題/); + assert.match(bodies[1], /新發現問題/); + assert.match(bodies[2], /嚴重問題/); + }); +}); diff --git a/app/findings.js b/app/findings.js index d045941..d8f1a67 100644 --- a/app/findings.js +++ b/app/findings.js @@ -77,20 +77,20 @@ function fallback(label, findings, e) { } /** 只保留 AI 需要的欄位，減少 token 用量 */ -function toAIPayload(findings) { +export function toAIPayload(findings) { return findings.map(({ level, role, location, suggestion }) => ({ level, role, location, suggestion })); } /** * 呼叫 LLM 進行語意去重，失敗時降級回傳原始 findings */ -export async function deduplicateWithAI(findings) { +export async function deduplicateWithAI(findings, chatFn = chatJSON) { if (findings.length === 0) return findings; const systemPrompt = `移除語意重複的程式碼審查問題（JSON 陣列）。保留等級較高者（critical > warning > info）。只回傳去重後的 JSON 陣列。`; try { - const result = await chatJSON(systemPrompt, JSON.stringify(toAIPayload(findings))); + const result = await chatFn(systemPrompt, JSON.stringify(toAIPayload(findings))); if (Array.isArray(result) && result.length > 0) { console.log(` AI 去重: ${findings.length} -> ${result.length} 筆`); // 以 location+suggestion 為 key，將原始 findings 的完整欄位（含 is_new）補回 @@ -131,7 +131,7 @@ export function applyExclusions(findings, exclusions) { /** * 呼叫 AI 判斷哪些問題是誤報或不需處理，失敗時降級回傳原始 findings */ -export async function filterFalsePositivesWithAI(findings, exclusions = []) { +export async function filterFalsePositivesWithAI(findings, exclusions = [], chatFn = chatJSON) { if (findings.length === 0) return findings; const exclusionHint = exclusions.length > 0 @@ -141,7 +141,7 @@ export async function filterFalsePositivesWithAI(findings, exclusions = []) { const systemPrompt = `判斷以下程式碼審查問題是否為誤報或不適用（如已正確使用 secrets、CI/CD 必要權限等），移除後只回傳需保留的 JSON 陣列。${exclusionHint}`; try { - const result = await chatJSON(systemPrompt, JSON.stringify(toAIPayload(findings))); + const result = await chatFn(systemPrompt, JSON.stringify(toAIPayload(findings))); if (Array.isArray(result) && result.length > 0) { console.log(` AI 誤報過濾: ${findings.length} -> ${result.length} 筆`); const origMap = new Map(findings.map(f => [`${f.location}|${String(f.suggestion).slice(0, 50)}`, f])); diff --git a/app/findings.test.js b/app/findings.test.js new file mode 100644 index 0000000..ad9a6e6 --- /dev/null +++ b/app/findings.test.js @@ -0,0 +1,83 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import { applyExclusions, deduplicateWithAI, filterFalsePositivesWithAI, toAIPayload } from './findings.js'; + +describe('findings helpers', () => { + it('toAIPayload strips internal fields', () => { + const payload = toAIPayload([ + { level: 'critical', role: 'Rex', location: 'a.js:1', suggestion: 'fix', is_new: true, extra: 'x' }, + ]); + assert.deepEqual(payload, [ + { level: 'critical', role: 'Rex', location: 'a.js:1', suggestion: 'fix' }, + ]); + }); + + it('deduplicateWithAI preserves original findings on quota failure', async () => { + const findings = [ + { level: 'warning', role: 'Rex', location: 'a.js:1', suggestion: 'fix', is_new: true }, + ]; + const quotaError = new Error('quota'); + quotaError.response = { status: 402 }; + + const result = await deduplicateWithAI(findings, async () => { + throw quotaError; + }); + + assert.deepEqual(result, findings); + }); + + it('deduplicateWithAI remaps AI output back to original findings', async () => { + const findings = [ + { level: 'warning', role: 'Rex', location: 'a.js:1', suggestion: 'fix' , is_new: true }, + { level: 'info', role: 'Maya', location: 'b.js:2', suggestion: 'note', is_new: false }, + ]; + + const result = await deduplicateWithAI(findings, async () => ([ + { level: 'warning', role: 'Rex', location: 'a.js:1', suggestion: 'fix' }, + ])); + + assert.equal(result[0].is_new, true); + assert.equal(result[0].location, 'a.js:1'); + }); + + it('applyExclusions filters by path and role', () => { + const findings = [ + { level: 'warning', role: 'Rex', location: 'src/a.js:1', suggestion: 'fix' }, + { level: 'info', role: 'Maya', location: 'src/b.js:2', suggestion: 'note' }, + ]; + const exclusions = [ + { location: 'src/a.js', role: 'Rex' }, + ]; + + assert.deepEqual(applyExclusions(findings, exclusions), [ + { level: 'info', role: 'Maya', location: 'src/b.js:2', suggestion: 'note' }, + ]); + }); + + it('filterFalsePositivesWithAI preserves findings on error', async () => { + const findings = [ + { level: 'critical', role: 'Aria', location: 'main.js:1', suggestion: 'fix', is_new: true }, + ]; + + const result = await filterFalsePositivesWithAI(findings, [], async () => { + throw new Error('api down'); + }); + + assert.deepEqual(result, findings); + }); + + it('filterFalsePositivesWithAI returns AI filtered findings', async () => { + const findings = [ + { level: 'critical', role: 'Aria', location: 'main.js:1', suggestion: 'fix', is_new: true }, + { level: 'warning', role: 'Rex', location: 'git.js:2', suggestion: 'note', is_new: false }, + ]; + + const result = await filterFalsePositivesWithAI(findings, [{ location: 'git.js', suggestion: 'note' }], async () => ([ + { level: 'critical', role: 'Aria', location: 'main.js:1', suggestion: 'fix' }, + ])); + + assert.equal(result.length, 1); + assert.equal(result[0].location, 'main.js:1'); + assert.equal(result[0].is_new, true); + }); +}); diff --git a/app/git.test.js b/app/git.test.js index bbf92e3..c92fdba 100644 --- a/app/git.test.js +++ b/app/git.test.js @@ -1,4 +1,4 @@ -import { describe, it, before, after, beforeEach } from 'node:test'; +import { describe, it, before, after, beforeEach, mock } from 'node:test'; import assert from 'node:assert/strict'; import fs from 'fs'; import os from 'os'; @@ -89,6 +89,18 @@ describe('commitAndPush', () => { const failSpawn = () => ({ status: 1, stdout: '', stderr: 'fatal: error', error: null }); await assert.doesNotReject(() => commitAndPush(workspace, path.join(workspace, 'repo'), failSpawn)); }); + + it('logs failure when git command fails', async () => { + const failSpawn = () => ({ status: 1, stdout: '', stderr: 'fatal: error', error: null }); + const logs = []; + mock.method(console, 'log', (...args) => { logs.push(args.join(' ')); }); + try { + await commitAndPush(workspace, path.join(workspace, 'repo'), failSpawn); + assert.ok(logs.some(line => line.includes('Runner failed: commit/push 失敗')), 'expected failure log'); + } finally { + mock.restoreAll(); + } + }); }); describe('cloneRepo', () => { diff --git a/app/main.js b/app/main.js index c51e4ee..de31ddd 100644 --- a/app/main.js +++ b/app/main.js @@ -1,5 +1,6 @@ import fs from 'fs'; import path from 'path'; +import { fileURLToPath } from 'url'; import { GITEA_REPOSITORY, PR_NUMBER, PR_HEAD_BRANCH, PR_BASE_BRANCH, getLLMConfig, FINDINGS_PATH, EXCLUSIONS_PATH } from './config.js'; import { loadRoles, getRoleIntro } from './roles.js'; import { getPRDiff, postComment } from './gitea.js'; @@ -9,6 +10,42 @@ import { cloneRepo, commitAndPush } from './git.js'; const WORKSPACE = process.env.GITHUB_WORKSPACE || '/workspace'; +export function validateAndRepairJsonFile(fullPath, relPath = fullPath) { + if (!fs.existsSync(fullPath)) { + console.log(` ⚠️ ${relPath} 不存在，跳過驗證`); + return true; + } + try { + JSON.parse(fs.readFileSync(fullPath, 'utf8')); + console.log(` ✅ ${relPath} JSON 格式正確`); + return true; + } catch (e) { + console.error(` ❌ ${relPath} JSON 格式錯誤: ${e.message}，嘗試修正...`); + try { + const backupPath = fullPath + '.bak'; + fs.copyFileSync(fullPath, backupPath); + fs.writeFileSync(fullPath, '[]\n', 'utf8'); + console.log(` ✅ ${relPath} 已重置為空陣列（原檔備份至 ${relPath}.bak）`); + return true; + } catch (repairErr) { + console.error(` ❌ ${relPath} 修正失敗: ${repairErr.message}`); + return false; + } + } +} + +export function handleCriticalFindings(findings, exitFn = process.exit) { + const criticalCount = findings.filter(f => f.level === 'critical').length; + if (criticalCount > 0) { + console.log(` ❌ 發現 ${criticalCount} 個嚴重問題，workflow 結束（exit 1）`); + console.log('='.repeat(60)); + exitFn(1); + return true; + } + console.log(' ✅ 無嚴重問題'); + return false; +} + async function main() { console.log('='.repeat(60)); console.log('🚀 Step1: Pipeline 啟動'); @@ -102,24 +139,8 @@ async function main() { console.log('\n🔎 Step6: JSON 格式驗證'); for (const relPath of [FINDINGS_PATH, EXCLUSIONS_PATH]) { const fullPath = path.join(repoDir || WORKSPACE, relPath); - if (!fs.existsSync(fullPath)) { - console.log(` ⚠️ ${relPath} 不存在，跳過驗證`); - continue; - } - try { - JSON.parse(fs.readFileSync(fullPath, 'utf8')); - console.log(` ✅ ${relPath} JSON 格式正確`); - } catch (e) { - console.error(` ❌ ${relPath} JSON 格式錯誤: ${e.message}，嘗試修正...`); - try { - const backupPath = fullPath + '.bak'; - fs.copyFileSync(fullPath, backupPath); - fs.writeFileSync(fullPath, '[]\n', 'utf8'); - console.log(` ✅ ${relPath} 已重置為空陣列（原檔備份至 ${relPath}.bak）`); - } catch (repairErr) { - console.error(` ❌ ${relPath} 修正失敗: ${repairErr.message}`); - process.exit(1); - } + if (!validateAndRepairJsonFile(fullPath, relPath)) { + process.exit(1); } } @@ -129,18 +150,14 @@ async function main() { // Step9: 有 critical 問題則 exit 1 console.log('\n🚦 Step8: 嚴重問題檢查'); - const criticalCount = filtered.filter(f => f.level === 'critical').length; - if (criticalCount > 0) { - console.log(` ❌ 發現 ${criticalCount} 個嚴重問題，workflow 結束（exit 1）`); - console.log('='.repeat(60)); - process.exit(1); - } - console.log(' ✅ 無嚴重問題'); + handleCriticalFindings(filtered); console.log('\n✅ Pipeline 完成'); console.log('='.repeat(60)); } -main().catch(e => { - console.error('❌ Runner failed:', e.message); - process.exit(1); -}); +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main().catch(e => { + console.error('❌ Runner failed:', e.message); + process.exit(1); + }); +} diff --git a/app/main.test.js b/app/main.test.js new file mode 100644 index 0000000..91e0f38 --- /dev/null +++ b/app/main.test.js @@ -0,0 +1,49 @@ +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { handleCriticalFindings, validateAndRepairJsonFile } from './main.js'; + +describe('main helpers', () => { + it('validateAndRepairJsonFile accepts valid JSON', () => { + const ws = fs.mkdtempSync(path.join(os.tmpdir(), 'main-valid-')); + const file = path.join(ws, 'data.json'); + fs.writeFileSync(file, '[{}]\n'); + + assert.equal(validateAndRepairJsonFile(file, 'data.json'), true); + assert.equal(fs.readFileSync(file, 'utf8'), '[{}]\n'); + fs.rmSync(ws, { recursive: true, force: true }); + }); + + it('validateAndRepairJsonFile repairs invalid JSON to empty array', () => { + const ws = fs.mkdtempSync(path.join(os.tmpdir(), 'main-repair-')); + const file = path.join(ws, 'data.json'); + fs.writeFileSync(file, '{invalid json'); + + assert.equal(validateAndRepairJsonFile(file, 'data.json'), true); + assert.equal(fs.readFileSync(file, 'utf8'), '[]\n'); + assert.equal(fs.existsSync(`${file}.bak`), true); + fs.rmSync(ws, { recursive: true, force: true }); + }); + + it('handleCriticalFindings exits when critical findings exist', () => { + let exitCode = null; + const findings = [ + { level: 'critical' }, + { level: 'warning' }, + ]; + + const result = handleCriticalFindings(findings, code => { exitCode = code; }); + + assert.equal(result, true); + assert.equal(exitCode, 1); + }); + + it('handleCriticalFindings passes when no critical findings exist', () => { + let exitCode = null; + const result = handleCriticalFindings([{ level: 'info' }], code => { exitCode = code; }); + assert.equal(result, false); + assert.equal(exitCode, null); + }); +});