Skip to content

Commit 90edc1f

Browse files
matejchalkmatejchalk
committed
Test and fix git diff and issues matching logic
1 parent 1bf04eb commit 90edc1f

8 files changed

Lines changed: 275 additions & 8 deletions

File tree

‎__tests__/git.test.ts‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import { mkdir, rename, rm, writeFile } from 'node:fs/promises'
2+
import { join } from 'node:path'
3+
import { simpleGit, type SimpleGit } from 'simple-git'
4+
import { listChangedFiles, type ChangedFiles } from '../src/git'
5+
6+
describe('git diff', () => {
7+
const workDir = join('tmp', 'git-utils-test')
8+
9+
let git: SimpleGit
10+
11+
beforeAll(async () => {
12+
await rm(workDir, { recursive: true, force: true })
13+
await mkdir(workDir, { recursive: true })
14+
15+
git = simpleGit(workDir)
16+
await git.init()
17+
await git.addConfig('user.name', 'John Doe')
18+
await git.addConfig('user.email', 'john.doe@example.com')
19+
await git.branch(['-M', 'main'])
20+
21+
await writeFile(join(workDir, 'LICENSE'), 'MIT License\n\n...')
22+
await writeFile(
23+
join(workDir, 'index.js'),
24+
'export const sum = values => values.reduce((acc, val) => acc + val, 0)\n'
25+
)
26+
await writeFile(
27+
join(workDir, 'package.json'),
28+
JSON.stringify({ name: 'sum', type: 'module', main: 'index.js' }, null, 2)
29+
)
30+
await git.add('.')
31+
await git.commit('Initial commit')
32+
33+
await git.checkoutLocalBranch('testing')
34+
await mkdir(join(workDir, 'src'))
35+
await mkdir(join(workDir, 'test'))
36+
await rename(join(workDir, 'index.js'), join(workDir, 'src/index.js'))
37+
await writeFile(
38+
join(workDir, 'test/index.test.js'),
39+
[
40+
"import assert from 'node:assert'",
41+
"import test from 'node:test'",
42+
"import { sum } from '../src/index.js'",
43+
'',
44+
"test('should sum all numbers', () => {",
45+
' assert.strictEqual(sum([1, 2, 3, 4]), 10)',
46+
'})'
47+
]
48+
.map(line => `${line}\n`)
49+
.join('')
50+
)
51+
await writeFile(
52+
join(workDir, 'package.json'),
53+
JSON.stringify(
54+
{
55+
name: 'sum',
56+
type: 'module',
57+
main: 'src/index.js',
58+
scripts: { test: 'node --test' }
59+
},
60+
null,
61+
2
62+
)
63+
)
64+
await git.add('.')
65+
await git.commit('Unit test')
66+
})
67+
68+
afterAll(async () => {
69+
await rm(workDir, { recursive: true, force: true })
70+
})
71+
72+
it('should list added, modified and renamed files', async () => {
73+
await expect(
74+
listChangedFiles({ base: 'main', head: 'testing' }, git)
75+
).resolves.toEqual<ChangedFiles>({
76+
'package.json': {
77+
lineChanges: [
78+
{ prev: { line: 4, count: 1 }, curr: { line: 4, count: 4 } }
79+
]
80+
},
81+
'src/index.js': {
82+
originalFile: 'index.js',
83+
lineChanges: []
84+
},
85+
'test/index.test.js': {
86+
lineChanges: [
87+
{ prev: { line: 0, count: 0 }, curr: { line: 1, count: 7 } }
88+
]
89+
}
90+
})
91+
})
92+
})

‎__tests__/issues.test.ts‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,165 @@
1+
import { issuesMatch } from '../src/issues'
2+
3+
describe('issues comparison', () => {
4+
it('should match issues with exact same metadata', () => {
5+
expect(
6+
issuesMatch(
7+
{
8+
plugin: { slug: 'coverage', title: 'Code coverage' },
9+
audit: { slug: 'function-coverage', title: 'Function coverage' },
10+
message: 'Function formatDate is not called in any test case.',
11+
severity: 'error',
12+
source: { file: 'src/utils.ts', position: { startLine: 100 } }
13+
},
14+
{
15+
plugin: { slug: 'coverage', title: 'Code coverage' },
16+
audit: { slug: 'function-coverage', title: 'Function coverage' },
17+
message: 'Function formatDate is not called in any test case.',
18+
severity: 'error',
19+
source: { file: 'src/utils.ts', position: { startLine: 100 } }
20+
},
21+
{
22+
'src/utils.ts': {
23+
lineChanges: [
24+
{ prev: { line: 200, count: 0 }, curr: { line: 200, count: 3 } }
25+
]
26+
}
27+
}
28+
)
29+
).toBe(true)
30+
})
31+
32+
it('should not match issues from different audits', () => {
33+
expect(
34+
issuesMatch(
35+
{
36+
plugin: { slug: 'coverage', title: 'Code coverage' },
37+
audit: { slug: 'function-coverage', title: 'Function coverage' },
38+
message: 'Function formatDate is not called in any test case.',
39+
severity: 'error',
40+
source: { file: 'src/utils.ts', position: { startLine: 100 } }
41+
},
42+
{
43+
plugin: { slug: 'eslint', title: 'ESLint' },
44+
audit: {
45+
slug: 'typescript-eslint-explicit-function-return-type',
46+
title:
47+
'Require explicit return types on functions and class methods.'
48+
},
49+
message: 'Missing return type on function.',
50+
severity: 'error',
51+
source: { file: 'src/utils.ts', position: { startLine: 100 } }
52+
},
53+
{
54+
'src/utils.ts': {
55+
lineChanges: [
56+
{ prev: { line: 200, count: 0 }, curr: { line: 200, count: 3 } }
57+
]
58+
}
59+
}
60+
)
61+
).toBe(false)
62+
})
63+
64+
it('should match issues based on adjusted line', () => {
65+
expect(
66+
issuesMatch(
67+
{
68+
plugin: { slug: 'coverage', title: 'Code coverage' },
69+
audit: { slug: 'line-coverage', title: 'Line coverage' },
70+
message: 'Lines 100-103 are not covered in any test case.',
71+
severity: 'error',
72+
source: {
73+
file: 'src/utils.ts',
74+
position: { startLine: 100, endLine: 103 }
75+
}
76+
},
77+
{
78+
plugin: { slug: 'coverage', title: 'Code coverage' },
79+
audit: { slug: 'line-coverage', title: 'Line coverage' },
80+
message: 'Lines 102-105 are not covered in any test case.',
81+
severity: 'error',
82+
source: {
83+
file: 'src/utils.ts',
84+
position: { startLine: 102, endLine: 105 }
85+
}
86+
},
87+
{
88+
'src/utils.ts': {
89+
lineChanges: [
90+
{ prev: { line: 42, count: 1 }, curr: { line: 42, count: 3 } }
91+
]
92+
}
93+
}
94+
)
95+
).toBe(true)
96+
})
97+
98+
it('should match issues from renamed files', () => {
99+
expect(
100+
issuesMatch(
101+
{
102+
plugin: { slug: 'coverage', title: 'Code coverage' },
103+
audit: { slug: 'function-coverage', title: 'Function coverage' },
104+
message: 'Function formatDate is not called in any test case.',
105+
severity: 'error',
106+
source: { file: 'src/utils.ts', position: { startLine: 100 } }
107+
},
108+
{
109+
plugin: { slug: 'coverage', title: 'Code coverage' },
110+
audit: { slug: 'function-coverage', title: 'Function coverage' },
111+
message: 'Function formatDate is not called in any test case.',
112+
severity: 'error',
113+
source: { file: 'src/utils/format.ts', position: { startLine: 100 } }
114+
},
115+
{
116+
'src/utils/format.ts': {
117+
originalFile: 'src/utils.ts',
118+
lineChanges: []
119+
}
120+
}
121+
)
122+
).toBe(true)
123+
})
124+
125+
it('should match issues based on adjusted line range', () => {
126+
expect(
127+
issuesMatch(
128+
{
129+
plugin: { slug: 'eslint', title: 'ESLint' },
130+
audit: {
131+
slug: 'max-lines',
132+
title: 'Enforce a maximum number of lines per file'
133+
},
134+
message: 'File has too many lines (420). Maximum allowed is 300.',
135+
severity: 'warning',
136+
source: {
137+
file: 'src/app.component.ts',
138+
position: { startLine: 300, endLine: 420 }
139+
}
140+
},
141+
{
142+
plugin: { slug: 'eslint', title: 'ESLint' },
143+
audit: {
144+
slug: 'max-lines',
145+
title: 'Enforce a maximum number of lines per file'
146+
},
147+
message: 'File has too many lines (450). Maximum allowed is 300.',
148+
severity: 'warning',
149+
source: {
150+
file: 'src/app.component.ts',
151+
position: { startLine: 300, endLine: 450 }
152+
}
153+
},
154+
{
155+
'src/app.component.ts': {
156+
lineChanges: [
157+
{ prev: { line: 12, count: 0 }, curr: { line: 12, count: 50 } },
158+
{ prev: { line: 123, count: 25 }, curr: { line: 173, count: 5 } }
159+
]
160+
}
161+
}
162+
)
163+
).toBe(true)
164+
})
165+
})

‎__tests__/main.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ describe('code-pushup action', () => {
130130
jest.spyOn(git, 'diffSummary').mockResolvedValue({
131131
files: [{ file: 'index.ts', binary: false }]
132132
} as DiffResult)
133+
jest.spyOn(git, 'diff').mockResolvedValue('')
133134

134135
await git.init()
135136
await git.addConfig('user.name', 'John Doe')

‎badges/coverage.svg‎

Lines changed: 1 addition & 1 deletion
Loading

‎dist/index.js‎

Lines changed: 5 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎dist/index.js.map‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎src/git.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ export async function listChangedFiles(
4646
.map(async ({ file, originalFile }) => {
4747
const diff = await git.diff([
4848
'--unified=0',
49+
refs.base,
50+
refs.head,
4951
'--',
5052
file,
5153
...(originalFile ? [originalFile] : [])
@@ -112,7 +114,11 @@ export function adjustFileName(
112114
changedFiles: ChangedFiles,
113115
file: string
114116
): string {
115-
return changedFiles[file]?.originalFile ?? file
117+
return (
118+
Object.entries(changedFiles).find(
119+
([, { originalFile }]) => originalFile === file
120+
)?.[0] ?? file
121+
)
116122
}
117123

118124
export function adjustLine(

‎src/issues.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ import type {
1616
export type SourceFileIssue = Required<Issue> & IssueContext
1717

1818
type IssueContext = {
19-
audit: Audit
20-
plugin: PluginMeta
19+
audit: Pick<Audit, 'slug' | 'title'>
20+
plugin: Pick<PluginMeta, 'slug' | 'title'>
2121
}
2222

2323
export function filterRelevantIssues({
@@ -69,7 +69,7 @@ function getAuditIssues(
6969
)
7070
}
7171

72-
function issuesMatch(
72+
export function issuesMatch(
7373
prev: SourceFileIssue,
7474
curr: SourceFileIssue,
7575
changedFiles: ChangedFiles

0 commit comments

Comments
 (0)