Skip to content

Commit

Permalink
fix: binary filtering for git diff --numstat parsing and document met…
Browse files Browse the repository at this point in the history
…a type file regression (#271)
  • Loading branch information
scolladon authored Mar 22, 2022
1 parent 9e61b3f commit 91c582c
Show file tree
Hide file tree
Showing 13 changed files with 215 additions and 134 deletions.
5 changes: 3 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,5 +216,6 @@ To test SGD as a Salesforce CLI plugin from a pending pull request:
1. uninstall the previous version you may have `sfdx plugins:uninstall sfdx-git-delta`
2. clone the repository
3. checkout the branch to test
4. run `yarn pack`, followed by `sfdx plugins:link`, from that local repository
5. test the plugin!
4. install the dependencies `yarn install`
5. run `yarn pack`, followed by `sfdx plugins:link`, from that local repository
6. test the plugin!
161 changes: 115 additions & 46 deletions __tests__/unit/lib/utils/repoGitDiff.test.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
'use strict'
const RepoGitDiff = require('../../../../src/utils/repoGitDiff')
const metadataManager = require('../../../../src/metadata/metadataManager')
const {
ADDITION,
DELETION,
MODIFICATION,
} = require('../../../../src/utils/gitConstants')
jest.mock('child_process')
const child_process = require('child_process')

const FORCEIGNORE_MOCK_PATH = '__mocks__/.forceignore'
const FORCEINCLUDE_MOCK_PATH = '__mocks__/.forceinclude'

const TAB = '\t'

describe(`test if repoGitDiff`, () => {
let globalMetadata
beforeAll(async () => {
Expand Down Expand Up @@ -44,48 +51,87 @@ describe(`test if repoGitDiff`, () => {
expect(work).toStrictEqual(output)
})

test('can resolve deletion', async () => {
test('can resolve file deletion', async () => {
const output = [
'D force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([output])
child_process.__setOutput([[], output.map(x => `1${TAB}1${TAB}${x}`), []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignore: FORCEIGNORE_MOCK_PATH },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toMatchObject(output)
expect(work).toStrictEqual(output.map(x => `${DELETION}${TAB}${x}`))
})

test('can resolve binary file deletion', async () => {
const output = [
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([[], output.map(x => `1${TAB}1${TAB}${x}`), []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toMatchObject(output.map(x => `${DELETION}${TAB}${x}`))
})

test('can resolve file copy when new file is added', async () => {
const output = [
'A force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([output])
child_process.__setOutput([[], [], output.map(x => `1${TAB}1${TAB}${x}`)])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toStrictEqual(output)
expect(work).toMatchObject(output.map(x => `${ADDITION}${TAB}${x}`))
})

test('can resolve file copy when new binary file is added', async () => {
const output = [
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([[], [], output.map(x => `-${TAB}-${TAB}${x}`)])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toMatchObject(output.map(x => `${ADDITION}${TAB}${x}`))
})

test('can resolve file copy when file is modified', async () => {
const output = [
'M force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([output])
child_process.__setOutput([output.map(x => `-${TAB}-${TAB}${x}`), [], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toStrictEqual(output)
expect(work).toMatchObject(output.map(x => `${MODIFICATION}${TAB}${x}`))
})

test('can resolve file copy when binary file is modified', async () => {
const output = [
'force-app/main/default/objects/Account/fields/awesome.field-meta.xml',
]
child_process.__setOutput([output.map(x => `-${TAB}-${TAB}${x}`), [], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toMatchObject(output.map(x => `${MODIFICATION}${TAB}${x}`))
})

test('can filter ignored files', async () => {
const output = ['M force-app/main/default/lwc/jsconfig.json']
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([[`1${TAB}1${TAB}${output}`], [], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignore: FORCEIGNORE_MOCK_PATH },
globalMetadata
Expand All @@ -97,8 +143,8 @@ describe(`test if repoGitDiff`, () => {
})

test('can filter ignored destructive files', async () => {
const output = ['D force-app/main/default/lwc/jsconfig.json']
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([[], [`1${TAB}1${TAB}${output}`], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignoreDestructive: FORCEIGNORE_MOCK_PATH },
globalMetadata
Expand All @@ -110,11 +156,12 @@ describe(`test if repoGitDiff`, () => {
})

test('can filter ignored and ignored destructive files', async () => {
const output = [
'M force-app/main/default/lwc/jsconfig.json',
'D force-app/main/default/lwc/jsconfig.json',
]
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([
[],
[`1${TAB}1${TAB}${output}`],
[`1${TAB}1${TAB}${output}`],
])
const repoGitDiff = new RepoGitDiff(
{
output: '',
Expand All @@ -131,8 +178,8 @@ describe(`test if repoGitDiff`, () => {
})

test('can filter deletion if only ignored is specified files', async () => {
const output = ['D force-app/main/default/lwc/jsconfig.json']
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([[], [`1${TAB}1${TAB}${output}`], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignore: FORCEIGNORE_MOCK_PATH },
globalMetadata
Expand All @@ -144,19 +191,20 @@ describe(`test if repoGitDiff`, () => {
})

test('cannot filter non deletion if only ignored destructive is specified files', async () => {
const output = ['A force-app/main/default/lwc/jsconfig.json']
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([[], [], [`1${TAB}1${TAB}${output}`]])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignoreDestructive: FORCEIGNORE_MOCK_PATH },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toStrictEqual(output)
const expected = [`${ADDITION}${TAB}${output}`]
expect(work).toStrictEqual(expected)
})

test('can filter sub folders', async () => {
const output = ['M force-app/main/default/pages/Account.page']
child_process.__setOutput([output])
const output = 'force-app/main/default/lwc/jsconfig.json'
child_process.__setOutput([[`1${TAB}1${TAB}${output}`], [], []])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '', ignore: FORCEIGNORE_MOCK_PATH },
globalMetadata
Expand All @@ -169,60 +217,84 @@ describe(`test if repoGitDiff`, () => {

test('can filter moved files', async () => {
const output = [
'D force-app/main/default/classes/Account.cls',
'A force-app/account/domain/classes/Account.cls',
'force-app/main/default/classes/Account.cls',
'force-app/account/domain/classes/Account.cls',
]
child_process.__setOutput([output])
child_process.__setOutput([
[],
[`1${TAB}1${TAB}${output[0]}`],
[`1${TAB}1${TAB}${output[1]}`],
])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
const expected = [output[1]]
const expected = [`${ADDITION}${TAB}${output[1]}`]
expect(work).toStrictEqual(expected)
})

test('can filter case changed files', async () => {
const output = [
'D force-app/main/default/objects/Account/fields/TEST__c.field-meta.xml',
'A force-app/main/default/objects/Account/fields/Test__c.field-meta.xml',
'force-app/main/default/objects/Account/fields/TEST__c.field-meta.xml',
'force-app/main/default/objects/Account/fields/Test__c.field-meta.xml',
]
child_process.__setOutput([output])
child_process.__setOutput([
[],
[`1${TAB}1${TAB}${output[0]}`],
[`1${TAB}1${TAB}${output[1]}`],
])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
const expected = [output[1]]
const expected = [`${ADDITION}${TAB}${output[1]}`]
expect(work).toStrictEqual(expected)
})

test('cannot filter renamed files', async () => {
const output = [
'D force-app/main/default/classes/Account.cls',
'A force-app/main/default/classes/RenamedAccount.cls',
'force-app/main/default/classes/Account.cls',
'force-app/main/default/classes/RenamedAccount.cls',
]
child_process.__setOutput([output])
child_process.__setOutput([
[],
[`1${TAB}1${TAB}${output[0]}`],
[`1${TAB}1${TAB}${output[1]}`],
])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toStrictEqual(output)
const expected = [
`${ADDITION}${TAB}${output[1]}`,
`${DELETION}${TAB}${output[0]}`,
]
expect(work).toStrictEqual(expected)
})

test('cannot filter same name file with different metadata', async () => {
const output = [
'D force-app/main/default/objects/Account/fields/CustomField__c.field-meta.xml',
'A force-app/main/default/objects/Opportunity/fields/CustomField__c.field-meta.xml',
'force-app/main/default/objects/Account/fields/CustomField__c.field-meta.xml',
'force-app/main/default/objects/Opportunity/fields/CustomField__c.field-meta.xml',
]
child_process.__setOutput([output])
child_process.__setOutput([
[],
[`1${TAB}1${TAB}${output[0]}`],
[`1${TAB}1${TAB}${output[1]}`],
])
const repoGitDiff = new RepoGitDiff(
{ output: '', repo: '' },
globalMetadata
)
const work = await repoGitDiff.getLines()
expect(work).toStrictEqual(output)
const expected = [
`${ADDITION}${TAB}${output[1]}`,
`${DELETION}${TAB}${output[0]}`,
]
expect(work).toStrictEqual(expected)
})

test('can explicitly include files', async () => {
Expand All @@ -237,7 +309,6 @@ describe(`test if repoGitDiff`, () => {
globalMetadata
)
const work = await repoGitDiff.getLines()
//should be empty
const expected = ['A force-app/main/default/lwc/jsconfig.json']
expect(work).toStrictEqual(expected)
})
Expand All @@ -254,7 +325,6 @@ describe(`test if repoGitDiff`, () => {
globalMetadata
)
const work = await repoGitDiff.getLines()
//should be empty
const expected = ['D force-app/main/default/lwc/jsconfig.json']
expect(work).toStrictEqual(expected)
})
Expand Down Expand Up @@ -297,9 +367,8 @@ describe(`test if repoGitDiff`, () => {
globalMetadata
)
const work = await repoGitDiff.getLines()
//should be empty
const expected = [
'D force-app/main/default/lwc/jsconfig.json',
`D force-app/main/default/lwc/jsconfig.json`,
'D force-app/main/default/staticresources/jsconfig.json',
]
expect(work).toStrictEqual(expected)
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"test:debug": "node --inspect node_modules/.bin/jest",
"test:debug:break": "node --inspect-brk node_modules/.bin/jest",
"postpack": "rm -f oclif.manifest.json ; prettier --write README.md",
"postrelease": "npm run release:tags && npm run release:github",
"postrelease": "yarn release:tags && yarn release:github",
"prepack": "rm -rf lib && tsc -b && oclif-dev manifest && oclif-dev readme",
"prepare": "husky install",
"release": "standard-version --no-verify --commit-all",
Expand Down Expand Up @@ -111,4 +111,4 @@
"sfdc": {
"latestApiVersion": "54"
}
}
}
1 change: 1 addition & 0 deletions src/metadata/v51.json
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@
"directoryName": "documents",
"inFolder": true,
"metaFile": true,
"suffix": "document",
"xmlName": "Document"
},
{
Expand Down
1 change: 1 addition & 0 deletions src/metadata/v52.json
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@
"directoryName": "documents",
"inFolder": true,
"metaFile": true,
"suffix": "document",
"xmlName": "Document"
},
{
Expand Down
1 change: 1 addition & 0 deletions src/metadata/v53.json
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@
"directoryName": "documents",
"inFolder": true,
"metaFile": true,
"suffix": "document",
"xmlName": "Document"
},
{
Expand Down
1 change: 1 addition & 0 deletions src/metadata/v54.json
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@
"directoryName": "documents",
"inFolder": true,
"metaFile": true,
"suffix": "document",
"xmlName": "Document"
},
{
Expand Down
Loading

0 comments on commit 91c582c

Please sign in to comment.