Skip to content

Commit d5cda86

Browse files
scolladonscolladon
authored
fix: use numstat to filter whitespace-only changes with -W flag (#1217)
1 parent f3fce5a commit d5cda86

File tree

12 files changed

+370
-157
lines changed

12 files changed

+370
-157
lines changed

‎__tests__/functional/delta.nut.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe('sgd source delta NUTS', () => {
5858
it('run `e2e` tests', async () => {
5959
// Act
6060
const result = execCmd(
61-
'sgd source delta --from "origin/e2e/base" --to "origin/e2e/head" --output e2e/expected --generate-delta --repo e2e --include e2e/.sgdinclude --include-destructive e2e/.sgdincludeDestructive --ignore e2e/.sgdignore --ignore-destructive e2e/.sgdignoreDestructive --json',
61+
'sgd source delta --from "origin/e2e/base" --to "origin/e2e/head" --output e2e/expected --generate-delta --repo e2e --include e2e/.sgdinclude --include-destructive e2e/.sgdincludeDestructive --ignore e2e/.sgdignore --ignore-destructive e2e/.sgdignoreDestructive --ignore-whitespace --json',
6262
{
6363
ensureExitCode: 0,
6464
}

‎__tests__/unit/lib/adapter/GitAdapter.test.ts‎

Lines changed: 98 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -611,45 +611,125 @@ describe('GitAdapter', () => {
611611
})
612612

613613
describe('getDiffLines', () => {
614-
it('calls diff name-status', async () => {
614+
it('Given diff output, When getDiffLines, Then calls numstat for each change type and transforms output', async () => {
615615
// Arrange
616616
const gitAdapter = GitAdapter.getInstance(config)
617-
mockedRaw.mockResolvedValue(`A\ttest\nM\tfile\nD\tanotherfile` as never)
617+
mockedRaw
618+
.mockResolvedValueOnce('8\t0\ttest' as never)
619+
.mockResolvedValueOnce('3\t2\tfile' as never)
620+
.mockResolvedValueOnce('0\t5\tanotherfile' as never)
618621

619622
// Act
620623
const result = await gitAdapter.getDiffLines()
621624

622625
// Assert
623626
expect(result).toEqual(['A\ttest', 'M\tfile', 'D\tanotherfile'])
624-
expect(mockedRaw).toHaveBeenCalledTimes(1)
625-
expect(mockedRaw).toHaveBeenCalledWith(
626-
expect.arrayContaining(['diff', '--name-status', '--no-renames'])
627+
expect(mockedRaw).toHaveBeenCalledTimes(3)
628+
expect(mockedRaw).toHaveBeenNthCalledWith(
629+
1,
630+
expect.arrayContaining([
631+
'diff',
632+
'--numstat',
633+
'--no-renames',
634+
'--diff-filter=A',
635+
])
636+
)
637+
expect(mockedRaw).toHaveBeenNthCalledWith(
638+
2,
639+
expect.arrayContaining([
640+
'diff',
641+
'--numstat',
642+
'--no-renames',
643+
'--diff-filter=M',
644+
])
645+
)
646+
expect(mockedRaw).toHaveBeenNthCalledWith(
647+
3,
648+
expect.arrayContaining([
649+
'diff',
650+
'--numstat',
651+
'--no-renames',
652+
'--diff-filter=D',
653+
])
627654
)
628655
})
629656

630-
describe('when called with ignore white space', () => {
631-
it('add ignore white space params', async () => {
657+
it('Given empty diff output, When getDiffLines, Then returns empty array', async () => {
658+
// Arrange
659+
const gitAdapter = GitAdapter.getInstance(config)
660+
mockedRaw
661+
.mockResolvedValueOnce('' as never)
662+
.mockResolvedValueOnce('' as never)
663+
.mockResolvedValueOnce('' as never)
664+
665+
// Act
666+
const result = await gitAdapter.getDiffLines()
667+
668+
// Assert
669+
expect(result).toEqual([])
670+
expect(mockedRaw).toHaveBeenCalledTimes(3)
671+
})
672+
673+
it('Given multiple files per change type, When getDiffLines, Then concatenates all results', async () => {
674+
// Arrange
675+
const gitAdapter = GitAdapter.getInstance(config)
676+
mockedRaw
677+
.mockResolvedValueOnce('10\t0\tnewFile1\n5\t0\tnewFile2' as never)
678+
.mockResolvedValueOnce('3\t2\tmodFile1' as never)
679+
.mockResolvedValueOnce('0\t8\tdelFile1\n0\t3\tdelFile2' as never)
680+
681+
// Act
682+
const result = await gitAdapter.getDiffLines()
683+
684+
// Assert
685+
expect(result).toEqual([
686+
'A\tnewFile1',
687+
'A\tnewFile2',
688+
'M\tmodFile1',
689+
'D\tdelFile1',
690+
'D\tdelFile2',
691+
])
692+
})
693+
694+
describe('Given ignoreWhitespace is enabled', () => {
695+
it('When getDiffLines, Then adds whitespace params to each call', async () => {
632696
// Arrange
633697
config.ignoreWhitespace = true
634698
const gitAdapter = GitAdapter.getInstance(config)
635-
mockedRaw.mockResolvedValue(`A\ttest\nM\tfile\nD\tanotherfile` as never)
699+
mockedRaw
700+
.mockResolvedValueOnce('8\t0\ttest' as never)
701+
.mockResolvedValueOnce('3\t2\tfile' as never)
702+
.mockResolvedValueOnce('' as never)
636703

637704
// Act
638705
const result = await gitAdapter.getDiffLines()
639706

640707
// Assert
641-
expect(result).toEqual(['A\ttest', 'M\tfile', 'D\tanotherfile'])
642-
expect(mockedRaw).toHaveBeenCalledTimes(1)
643-
expect(mockedRaw).toHaveBeenCalledWith(
644-
expect.arrayContaining([
645-
'diff',
646-
'--name-status',
647-
'--no-renames',
648-
...IGNORE_WHITESPACE_PARAMS,
649-
])
650-
)
708+
expect(result).toEqual(['A\ttest', 'M\tfile'])
709+
expect(mockedRaw).toHaveBeenCalledTimes(3)
710+
for (let i = 1; i <= 3; i++) {
711+
expect(mockedRaw).toHaveBeenNthCalledWith(
712+
i,
713+
expect.arrayContaining([...IGNORE_WHITESPACE_PARAMS])
714+
)
715+
}
651716
})
652717
})
718+
719+
it('Given binary files in diff, When getDiffLines, Then handles dash stats correctly', async () => {
720+
// Arrange
721+
const gitAdapter = GitAdapter.getInstance(config)
722+
mockedRaw
723+
.mockResolvedValueOnce('-\t-\tbinaryFile.png' as never)
724+
.mockResolvedValueOnce('' as never)
725+
.mockResolvedValueOnce('' as never)
726+
727+
// Act
728+
const result = await gitAdapter.getDiffLines()
729+
730+
// Assert
731+
expect(result).toEqual(['A\tbinaryFile.png'])
732+
})
653733
})
654734

655735
describe('listDirAtRevision', () => {

‎__tests__/unit/lib/utils/arrayUtils.test.ts‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
'use strict'
2+
import { pushAll } from '../../../../src/utils/arrayUtils'
3+
4+
describe('arrayUtils', () => {
5+
describe('pushAll', () => {
6+
describe('given an empty source array', () => {
7+
it('should not modify target array', () => {
8+
// Arrange
9+
const target = [1, 2, 3]
10+
const source: number[] = []
11+
12+
// Act
13+
pushAll(target, source)
14+
15+
// Assert
16+
expect(target).toEqual([1, 2, 3])
17+
})
18+
})
19+
20+
describe('given a non-empty source array', () => {
21+
it('should append all elements to target array', () => {
22+
// Arrange
23+
const target = [1, 2]
24+
const source = [3, 4, 5]
25+
26+
// Act
27+
pushAll(target, source)
28+
29+
// Assert
30+
expect(target).toEqual([1, 2, 3, 4, 5])
31+
})
32+
})
33+
34+
describe('given an empty target array', () => {
35+
it('should populate target with source elements', () => {
36+
// Arrange
37+
const target: string[] = []
38+
const source = ['a', 'b', 'c']
39+
40+
// Act
41+
pushAll(target, source)
42+
43+
// Assert
44+
expect(target).toEqual(['a', 'b', 'c'])
45+
})
46+
})
47+
48+
describe('given multiple source arrays', () => {
49+
it('should append all elements from all sources in order', () => {
50+
// Arrange
51+
const target = ['a']
52+
const source1 = ['b', 'c']
53+
const source2 = ['d']
54+
const source3 = ['e', 'f']
55+
56+
// Act
57+
pushAll(target, source1, source2, source3)
58+
59+
// Assert
60+
expect(target).toEqual(['a', 'b', 'c', 'd', 'e', 'f'])
61+
})
62+
})
63+
64+
describe('given mixed empty and non-empty source arrays', () => {
65+
it('should handle empty arrays in the middle', () => {
66+
// Arrange
67+
const target = [1]
68+
const source1 = [2]
69+
const source2: number[] = []
70+
const source3 = [3]
71+
72+
// Act
73+
pushAll(target, source1, source2, source3)
74+
75+
// Assert
76+
expect(target).toEqual([1, 2, 3])
77+
})
78+
})
79+
})
80+
})

‎knip.config.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,5 @@ export default {
1414
'sinon',
1515
'ts-jest-mock-import-meta',
1616
],
17-
ignore: ['tooling/**'],
1817
ignoreBinaries: ['npm-check-updates'],
1918
}

0 commit comments

Comments
 (0)