Skip to content

Commit

Permalink
fix: --include parameter for metadata contained in file (#678)
Browse files Browse the repository at this point in the history
Co-authored-by: Mehdi Cherfaoui <[email protected]>
  • Loading branch information
scolladon and mehdicherf authored Aug 28, 2023
1 parent 2982f02 commit 281fa33
Show file tree
Hide file tree
Showing 22 changed files with 1,570 additions and 903 deletions.
3 changes: 3 additions & 0 deletions .github/linters/.cspell.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"destructiveignore",
"destructiveinclude",
"emailservices",
"firstsha",
"flexipage",
"flexipages",
"flowtest",
Expand Down Expand Up @@ -122,6 +123,8 @@
"scontrols",
"sfdx",
"sgdignore",
"sgdinclude",
"sgdincludedestructive",
"shellcheck",
"staticresources",
"stefanzweifel",
Expand Down
14 changes: 8 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,7 @@ OPTIONS
folder
-f, --from=from (required) commit sha from where the
diff is done [git rev-list
--max-parents=0 HEAD]
diff is done
-i, --ignore=ignore file listing paths to explicitly
ignore for any diff actions
Expand Down Expand Up @@ -212,6 +211,7 @@ sfdx sgd:source:delta --from "HEAD~1" # right git shortcut with windows because
In CI/CD pipelines, for most of the CI/CD providers, the checkout operation fetch only the last commit of the branch currently evaluated.
You need to fetch all the needed commits, as the plugin needs to have the branch to compare from as well,
Example for Github action checkout [here](https://github.com/actions/checkout#fetch-all-history-for-all-tags-and-branches).
If you use `-n` (`--include`) with metadata contained inside files you will need to have the full repo locally for the command to fully work.

In CI/CD pipelines, branches are not checked out locally when the repository is cloned, so you must specify the remote prefix.
If you do not specify the remote in CI context, the git pointer check will raise an error (as the branch is not created locally).
Expand Down Expand Up @@ -407,13 +407,16 @@ Note: when only using the `--ignore [-i]` parameter (and not `--ignore-destructi

### Explicitly including specific files for inclusion or destruction regardless of diff

The `--include [-n]` parameter allows you to specify a file based on [micromatch glob matching](https://github.com/micromatch/micromatch) to include specific files. Regardless whether they appears in the diff or not.
The `--include [-n]` parameter allows you to specify a file based on [gitignore glob matching](https://git-scm.com/docs/gitignore) to include specific files. Regardless whether they appears in the diff or not.
Like the `--ignore` flag, this file defines a list of glob file matchers to always include `git` aware files in the `package.xml` package.
SGD will include every line matching the pattern from the include file specified in the `--include [-n]`.
SGD will include every metadata from the repo at the `to` parameter state, matching the pattern from the include file specified in the `--include [-n]`.

As with `--ignore`, you may need different policies for the `package.xml` and `destructiveChanges.xml` files. This is where the `--include-destructive [-N]` option comes handy!

Use the `--include-destructive` parameter to specify a dedicated include file to handle deletions. Related metadata will appear in the `destructiveChanges.xml` output. Here, you will show which files should the `destructiveChanges.xml` should include .
Use the `--include-destructive` parameter to specify a dedicated include file to handle deletions. Related metadata will appear in the `destructiveChanges.xml` output.

/!\ In order to work properly with metadata contained inside files (Labels, Workflow, MatchingRules, etc) the local repo must have the full historic.

Consider the following:

- a repository containing many sub-folders (force-app/main,force-app/sample, etc)
Expand Down Expand Up @@ -521,7 +524,6 @@ console.log(JSON.stringify(work))
- [fs-extra](https://github.com/jprichardson/node-fs-extra) - Node.js: extra methods for the fs object like copy(), remove(), mkdirs().
- [ignore](https://github.com/kaelzhang/node-ignore#readme) - is a manager, filter and parser which implemented in pure JavaScript according to the .gitignore spec 2.22.1.
- [lodash](https://github.com/lodash/lodash) - A modern JavaScript utility library delivering modularity, performance & extras.
- [micromatch](https://github.com/micromatch/micromatch) - a file glob matcher utility
- [xmlbuilder2](https://github.com/oozcitak/xmlbuilder2) - An XML builder for node.js.
- [MegaLinter](https://megalinter.io) - Open-Source tool for CI/CD workflows that analyzes the consistency of your code, IAC, configuration, and scripts
Expand Down
8 changes: 6 additions & 2 deletions __tests__/functional/delta.nut.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('sgd:source:delta NUTS', () => {
it('run `e2e` tests', async () => {
// Act
const result = execCmd(
'sgd:source:delta --from "origin/e2e/base" --to "origin/e2e/head" --output e2e/expected --generate-delta --repo e2e --ignore e2e/.sgdignore',
'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',
{
ensureExitCode: 0,
}
Expand All @@ -57,7 +57,11 @@ describe('sgd:source:delta NUTS', () => {
const packageLineCount = await getFileLineNumber(
'e2e/expected/package/package.xml'
)
expect(packageLineCount).to.equal(209)
const destructiveChangesLineCount = await getFileLineNumber(
'e2e/expected/destructiveChanges/destructiveChanges.xml'
)
expect(packageLineCount).to.equal(211)
expect(destructiveChangesLineCount).to.equal(118)
expect(result).to.include('"error": null')
expect(result).to.include('"success": true')
})
Expand Down
2 changes: 1 addition & 1 deletion __tests__/perf/bench.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ const suite = new Benchmark.Suite()
suite
.add('e2e-test', () => {
execCmd(
'sgd:source:delta --from "origin/e2e/base" --to "origin/e2e/head" --output e2e/expected --generate-delta --repo e2e',
'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',
{
ensureExitCode: 0,
}
Expand Down
144 changes: 144 additions & 0 deletions __tests__/unit/lib/post-processor/includeProcessor.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
const IncludeProcessor = require('../../../../src/post-processor/includeProcessor')
const { buildIncludeHelper } = require('../../../../src/utils/ignoreHelper')

const mockProcess = jest.fn()
jest.mock('../../../../src/service/diffLineInterpreter', () => {
return jest.fn().mockImplementation(() => {
return {
process: mockProcess,
}
})
})

const mockGetAllFilesAsLineStream = jest.fn()
jest.mock('../../../../src/utils/repoSetup', () => {
return jest.fn().mockImplementation(() => {
return {
getAllFilesAsLineStream: mockGetAllFilesAsLineStream,
getFirstCommitRef: jest.fn(),
}
})
})

jest.mock('../../../../src/utils/ignoreHelper')

const mockKeep = jest.fn()
buildIncludeHelper.mockImplementation(() => ({
keep: mockKeep,
}))

describe('IncludeProcessor', () => {
let work
let metadata

beforeAll(async () => {
// eslint-disable-next-line no-undef
metadata = await getGlobalMetadata()
})

beforeEach(() => {
work = {
config: {},
diffs: { package: new Map(), destructiveChanges: new Map() },
warnings: [],
}
jest.clearAllMocks()
})

describe('when no include is configured', () => {
it('does not process include', async () => {
// Arrange
const sut = new IncludeProcessor(work, metadata)

// Act
await sut.process()

// Assert
expect(buildIncludeHelper).not.toBeCalled()
})
})

describe('when include is configured', () => {
beforeAll(() => {
mockGetAllFilesAsLineStream.mockImplementation(() => ['test'])
})

describe('when no file matches the patterns', () => {
beforeEach(() => {
mockKeep.mockReturnValue(true)
})
it('does not process include', async () => {
// Arrange
work.config.include = '.sgdinclude'
const sut = new IncludeProcessor(work, metadata)

// Act
await sut.process()

// Assert
expect(buildIncludeHelper).toBeCalled()
expect(mockProcess).not.toBeCalled()
})
})

describe('when file matches the patterns', () => {
beforeEach(() => {
mockKeep.mockReturnValue(false)
})
it('process include', async () => {
// Arrange
work.config.include = '.sgdinclude'
const sut = new IncludeProcessor(work, metadata)

// Act
await sut.process()

// Assert
expect(buildIncludeHelper).toBeCalled()
expect(mockProcess).toBeCalled()
})
})
})

describe('when includeDestructive is configured', () => {
beforeAll(() => {
mockGetAllFilesAsLineStream.mockImplementation(() => ['test'])
})
describe('when no file matches the patterns', () => {
beforeEach(() => {
mockKeep.mockReturnValue(true)
})
it('does not process include destructive', async () => {
// Arrange
work.config.includeDestructive = '.sgdincludedestructive'
const sut = new IncludeProcessor(work, metadata)

// Act
await sut.process()

// Assert
expect(buildIncludeHelper).toBeCalled()
expect(mockProcess).not.toBeCalled()
})
})

describe('when file matches the patterns', () => {
beforeEach(() => {
mockKeep.mockReturnValue(false)
})
it('process include destructive', async () => {
// Arrange
work.config.includeDestructive = '.sgdincludedestructive'
const sut = new IncludeProcessor(work, metadata)

// Act
await sut.process()

// Assert
expect(buildIncludeHelper).toBeCalled()
expect(mockProcess).toBeCalled()
})
})
})
})
it('test', () => {})
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ class TestProcessor extends BaseProcessor {
}

describe('postProcessorManager', () => {
const work = {}
const work = {
config: {},
}
describe('getPostProcessors', () => {
describe('when called', () => {
it('returns a post processor manager with a list of post processor', () => {
Expand Down
2 changes: 0 additions & 2 deletions __tests__/unit/lib/service/botHandler.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,4 @@ describe('BotHandler', () => {
})
})
})

// TODO getMetaTypeFilePath
})
62 changes: 62 additions & 0 deletions __tests__/unit/lib/service/diffLineInterpreter.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict'
const DiffLineInterpreter = require('../../../../src/service/diffLineInterpreter')

const mockHandle = jest.fn()
jest.mock('../../../../src/service/typeHandlerFactory', () => {
return jest.fn().mockImplementation(() => {
return {
getTypeHandler: jest.fn().mockImplementation(() => {
return { handle: mockHandle }
}),
}
})
})

let work
let sut
beforeEach(() => {
jest.clearAllMocks()
work = {
config: { output: '', repo: '', generateDelta: true },
diffs: { package: new Map(), destructiveChanges: new Map() },
}
sut = new DiffLineInterpreter()
})

describe('DiffLineInterpreter', () => {
let globalMetadata
beforeAll(async () => {
// eslint-disable-next-line no-undef
globalMetadata = await getGlobalMetadata()
})

beforeEach(() => {
sut = new DiffLineInterpreter(work, globalMetadata)
})

describe('when called with lines', () => {
it('process each lines', () => {
// Arrange
const lines = ['test']

// Act
sut.process(lines)

// Assert
expect(mockHandle).toBeCalledTimes(1)
})
})

describe('when called without lines', () => {
it('it does not process anything', () => {
// Arrange
const lines = []

// Act
sut.process(lines)

// Assert
expect(mockHandle).not.toBeCalled()
})
})
})
Loading

0 comments on commit 281fa33

Please sign in to comment.