|
| 1 | +// @ts-check |
| 2 | +import assert from 'node:assert' |
| 3 | +import { readFileSync, readdirSync, statSync } from 'node:fs' |
| 4 | +import { join } from 'node:path' |
| 5 | +import { describe, test } from 'node:test' |
| 6 | +import parseGitDiff from 'parse-git-diff' |
| 7 | +import { generateReviewComments, getGitDiff } from '../index.js' |
| 8 | + |
| 9 | +const fixtureDir = 'test/fixtures' |
| 10 | + |
| 11 | +/** |
| 12 | + * Normalize line endings to LF for cross-platform consistency |
| 13 | + * @param {string} content - File content to normalize |
| 14 | + * @returns {string} Content with normalized line endings |
| 15 | + */ |
| 16 | +function normalizeLineEndings(content) { |
| 17 | + return content.replace(/\r\n/g, '\n') |
| 18 | +} |
| 19 | + |
| 20 | +/** |
| 21 | + * Generate a git diff between two files using the same logic as index.js |
| 22 | + * @param {string} beforeFile - Path to the "before" file |
| 23 | + * @param {string} afterFile - Path to the "after" file |
| 24 | + * @returns {Promise<string>} The git diff output |
| 25 | + */ |
| 26 | +async function generateDiff(beforeFile, afterFile) { |
| 27 | + // Use the shared git diff function with --no-index for comparing files outside git context |
| 28 | + return await getGitDiff(['--no-index', beforeFile, afterFile]) |
| 29 | +} |
| 30 | + |
| 31 | +/** |
| 32 | + * Apply a suggestion to file content |
| 33 | + * @param {string} content - The original file content |
| 34 | + * @param {import('../index.js').ReviewCommentDraft} suggestion - The suggestion to apply |
| 35 | + * @returns {string} The content with the suggestion applied |
| 36 | + */ |
| 37 | +function applySuggestion(content, suggestion) { |
| 38 | + const lines = content.split('\n') |
| 39 | + |
| 40 | + // Extract the suggestion body content (remove the ````suggestion wrapper) |
| 41 | + // Use greedy match (not *?) because the suggestion body always includes a newline before the closing ```` |
| 42 | + const suggestionMatch = suggestion.body.match(/^````suggestion\n([\s\S]*)\n````$/) |
| 43 | + if (!suggestionMatch) { |
| 44 | + throw new Error( |
| 45 | + `Invalid suggestion body format. Expected format: \`\`\`\`suggestion\\n<content>\\n\`\`\`\`\n` + |
| 46 | + `Received: ${suggestion.body}` |
| 47 | + ) |
| 48 | + } |
| 49 | + const suggestionContent = suggestionMatch[1] |
| 50 | + const suggestionLines = suggestionContent === '' ? [] : suggestionContent.split('\n') |
| 51 | + |
| 52 | + // Determine which lines to replace |
| 53 | + // GitHub suggestions use 1-based line numbers |
| 54 | + const startLine = suggestion.start_line ?? suggestion.line |
| 55 | + const endLine = suggestion.line |
| 56 | + |
| 57 | + // Convert to 0-based array indices |
| 58 | + const startIndex = startLine - 1 |
| 59 | + const endIndex = endLine - 1 |
| 60 | + |
| 61 | + // Replace the lines |
| 62 | + const newLines = [ |
| 63 | + ...lines.slice(0, startIndex), |
| 64 | + ...suggestionLines, |
| 65 | + ...lines.slice(endIndex + 1) |
| 66 | + ] |
| 67 | + |
| 68 | + return newLines.join('\n') |
| 69 | +} |
| 70 | + |
| 71 | +/** |
| 72 | + * Apply multiple suggestions to file content in the correct order |
| 73 | + * Suggestions must be applied in reverse order (bottom to top) to avoid line number shifts |
| 74 | + * @param {string} content - The original file content |
| 75 | + * @param {Array<import('../index.js').ReviewCommentDraft>} suggestions - The suggestions to apply |
| 76 | + * @returns {string} The content with all suggestions applied |
| 77 | + */ |
| 78 | +function applySuggestions(content, suggestions) { |
| 79 | + // Sort suggestions by line number in descending order (bottom to top) |
| 80 | + // This ensures that applying one suggestion doesn't shift line numbers for others |
| 81 | + const sortedSuggestions = [...suggestions].sort((a, b) => { |
| 82 | + const aStart = a.start_line ?? a.line |
| 83 | + const bStart = b.start_line ?? b.line |
| 84 | + return bStart - aStart |
| 85 | + }) |
| 86 | + |
| 87 | + let result = content |
| 88 | + for (const suggestion of sortedSuggestions) { |
| 89 | + result = applySuggestion(result, suggestion) |
| 90 | + } |
| 91 | + return result |
| 92 | +} |
| 93 | + |
| 94 | +/** |
| 95 | + * Find before/after file pairs in a directory |
| 96 | + * @param {string} dirPath - Directory to search |
| 97 | + * @returns {Array<{beforeFile: string, afterFile: string, testName: string}>} |
| 98 | + */ |
| 99 | +function findBeforeAfterPairs(dirPath) { |
| 100 | + const files = readdirSync(dirPath) |
| 101 | + |
| 102 | + return files |
| 103 | + .filter((file) => file.startsWith('before.') || file.includes('-before.')) |
| 104 | + .flatMap((beforeFile) => { |
| 105 | + const afterFile = beforeFile.replace(/before(\.|-)/, 'after$1') |
| 106 | + |
| 107 | + if (!files.includes(afterFile)) { |
| 108 | + return [] |
| 109 | + } |
| 110 | + |
| 111 | + // Extract test name: "complex-before.md" → "complex", "before.md" → "default" |
| 112 | + const testName = beforeFile.match(/^(.+)-before\./)?.[1] || 'default' |
| 113 | + |
| 114 | + return [ |
| 115 | + { |
| 116 | + beforeFile: join(dirPath, beforeFile), |
| 117 | + afterFile: join(dirPath, afterFile), |
| 118 | + testName, |
| 119 | + }, |
| 120 | + ] |
| 121 | + }) |
| 122 | +} |
| 123 | + |
| 124 | +describe('Integration Tests', () => { |
| 125 | + // Discover all tool directories and their test pairs |
| 126 | + const toolDirs = readdirSync(fixtureDir).filter((item) => { |
| 127 | + try { |
| 128 | + return statSync(join(fixtureDir, item)).isDirectory() |
| 129 | + } catch { |
| 130 | + return false |
| 131 | + } |
| 132 | + }) |
| 133 | + |
| 134 | + describe('Suggestion Generation', () => { |
| 135 | + // Generate tests for all tool/testcase combinations |
| 136 | + toolDirs |
| 137 | + .flatMap((toolDir) => |
| 138 | + findBeforeAfterPairs(join(fixtureDir, toolDir)).map((pair) => ({ |
| 139 | + toolDir, |
| 140 | + ...pair, |
| 141 | + })) |
| 142 | + ) |
| 143 | + .forEach(({ toolDir, beforeFile, afterFile, testName }) => { |
| 144 | + test(`${toolDir}/${testName} suggestions should match snapshot`, async (t) => { |
| 145 | + const diffContent = await generateDiff(beforeFile, afterFile) |
| 146 | + const parsed = parseGitDiff(diffContent) |
| 147 | + // For clarity in snapshots we want the path to reference the BEFORE file. |
| 148 | + // The diff we generate is from before -> after (so parseGitDiff reports the "after" path), |
| 149 | + // but suggestions conceptually apply to the before state to reach the after state in these fixtures. |
| 150 | + const suggestions = generateReviewComments(parsed).map((s) => ({ |
| 151 | + ...s, |
| 152 | + path: beforeFile, |
| 153 | + })) |
| 154 | + t.assert.snapshot(suggestions) |
| 155 | + }) |
| 156 | + }) |
| 157 | + }) |
| 158 | + |
| 159 | + describe('Suggestion Application', () => { |
| 160 | + // Generate tests for all tool/testcase combinations |
| 161 | + // These tests verify that applying the generated suggestions to the "before" state |
| 162 | + // produces the "after" state, ensuring the suggestions are correct and complete. |
| 163 | + toolDirs |
| 164 | + .flatMap((toolDir) => |
| 165 | + findBeforeAfterPairs(join(fixtureDir, toolDir)).map((pair) => ({ |
| 166 | + toolDir, |
| 167 | + ...pair, |
| 168 | + })) |
| 169 | + ) |
| 170 | + .forEach(({ toolDir, beforeFile, afterFile, testName }) => { |
| 171 | + test(`${toolDir}/${testName} applying suggestions should transform before → after`, async () => { |
| 172 | + // Read the before and after files with normalized line endings |
| 173 | + const beforeContent = normalizeLineEndings(readFileSync(beforeFile, 'utf8')) |
| 174 | + const afterContent = normalizeLineEndings(readFileSync(afterFile, 'utf8')) |
| 175 | + |
| 176 | + // Generate suggestions |
| 177 | + const diffContent = await generateDiff(beforeFile, afterFile) |
| 178 | + const parsed = parseGitDiff(diffContent) |
| 179 | + const suggestions = generateReviewComments(parsed) |
| 180 | + |
| 181 | + // Apply suggestions to the before content |
| 182 | + const result = applySuggestions(beforeContent, suggestions) |
| 183 | + |
| 184 | + // Verify that applying suggestions transforms before → after |
| 185 | + assert.strictEqual( |
| 186 | + result, |
| 187 | + afterContent, |
| 188 | + `Applying suggestions should transform ${beforeFile} to match ${afterFile}` |
| 189 | + ) |
| 190 | + }) |
| 191 | + }) |
| 192 | + }) |
| 193 | +}) |
0 commit comments