diff --git a/__tests__/functional.test.js b/__tests__/functional.test.js index 855d800c..53db9e1d 100644 --- a/__tests__/functional.test.js +++ b/__tests__/functional.test.js @@ -1,11 +1,11 @@ 'use strict' +const fs = require('fs') +const child_process = require('child_process') const app = require('../src/main') -const { GIT_FOLDER } = require('../src/utils/gitConstants') +const { COMMIT_REF_TYPE, GIT_FOLDER } = require('../src/utils/gitConstants') jest.mock('fs') jest.mock('fs-extra') jest.mock('child_process') -const fs = require('fs') -const child_process = require('child_process') const lines = [ 'D force-app/main/default/objects/Account/fields/deleted.field-meta.xml', @@ -38,13 +38,21 @@ describe(`test if the appli`, () => { }) }) test('can execute with rich parameters and big diff', async () => { - child_process.__setOutput([lines, [], ['firstSHA']]) + child_process.__setOutput([ + lines, + [], + [], + [COMMIT_REF_TYPE], + [COMMIT_REF_TYPE], + [], + ]) expect( await app({ output: 'output', repo: '.', source: '', to: 'test', + from: 'main', apiVersion: '46', }) ).toHaveProperty('warnings', []) @@ -52,12 +60,20 @@ describe(`test if the appli`, () => { test('catch internal warnings', async () => { fs.errorMode = true - child_process.__setOutput([lines, [], ['firstSHA']]) + child_process.__setOutput([ + lines, + [], + [], + [], + [COMMIT_REF_TYPE], + [COMMIT_REF_TYPE], + ]) const work = await app({ output: 'output', repo: '', source: '', to: 'HEAD', + from: 'main', apiVersion: '46', generateDelta: true, }) @@ -65,12 +81,20 @@ describe(`test if the appli`, () => { }) test('do not generate destructiveChanges.xml and package.xml with same element', async () => { - child_process.__setOutput([lines, [], ['firstSHA']]) + child_process.__setOutput([ + lines, + [], + [], + [], + [COMMIT_REF_TYPE], + [COMMIT_REF_TYPE], + ]) const work = await app({ output: 'output', repo: '', source: '', to: 'HEAD', + from: 'main', apiVersion: '46', generateDelta: true, }) diff --git a/__tests__/integration/main.test.js b/__tests__/integration/main.test.js index 8d07f092..9ea3d879 100644 --- a/__tests__/integration/main.test.js +++ b/__tests__/integration/main.test.js @@ -1,6 +1,6 @@ 'use strict' const app = require('../../src/main') -const { GIT_FOLDER } = require('../../src/utils/gitConstants') +const { COMMIT_REF_TYPE, GIT_FOLDER } = require('../../src/utils/gitConstants') jest.mock('child_process') jest.mock('fs-extra') @@ -14,6 +14,7 @@ const testConfig = { repo: '', source: '', to: 'test', + from: 'main', apiVersion: '46', } @@ -32,7 +33,7 @@ describe(`test if the appli`, () => { }) beforeEach(() => { - child_process.__setOutput([]) + child_process.__setOutput([[], [COMMIT_REF_TYPE], [COMMIT_REF_TYPE], []]) child_process.__setError(false) }) @@ -93,16 +94,4 @@ describe(`test if the appli`, () => { }) ).toHaveProperty('warnings', []) }) - - test('catch and reject big issues', async () => { - fsMocked.errorMode = true - fseMocked.errorMode = true - fseMocked.outputFileError = true - child_process.__setError(true) - try { - await app(testConfig) - } catch (error) { - expect(error).toBeDefined() - } - }) }) diff --git a/__tests__/unit/lib/utils/cliHelper.test.js b/__tests__/unit/lib/utils/cliHelper.test.js index 136f07f0..f9b5a195 100644 --- a/__tests__/unit/lib/utils/cliHelper.test.js +++ b/__tests__/unit/lib/utils/cliHelper.test.js @@ -1,18 +1,16 @@ 'use strict' - -jest.mock('../../../../src/utils/repoSetup') -const RepoSetup = require('../../../../src/utils/repoSetup') -RepoSetup.mockImplementation(() => { - return { - isToEqualHead: jest.fn(), - repoConfiguration: jest.fn(), - computeFromRef: jest.fn(), - } -}) -jest.mock('fs') - +const child_process = require('child_process') const fs = require('fs') +const { COMMIT_REF_TYPE } = require('../../../../src/utils/gitConstants') +const RepoSetup = require('../../../../src/utils/repoSetup') const CLIHelper = require('../../../../src/utils/cliHelper') +jest.mock('fs') +jest.mock('child_process') +jest.mock('../../../../src/utils/repoSetup') +RepoSetup.mockImplementation(() => ({ + isToEqualHead: jest.fn(), + repoConfiguration: jest.fn(), +})) const testConfig = { output: 'output', @@ -32,30 +30,16 @@ describe(`test if the application`, () => { }) }) - beforeEach(() => { - jest.resetAllMocks() - }) - test('throws errors when to parameter is not filled', async () => { let cliHelper = new CLIHelper({ ...testConfig, to: undefined }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when apiVersion parameter is NaN', async () => { let cliHelper = new CLIHelper({ ...testConfig, apiVersion: 'NotANumber' }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when fs.stat throw error', async () => { @@ -65,13 +49,8 @@ describe(`test if the application`, () => { to: undefined, output: 'stat/error', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when output folder does not exist', async () => { @@ -80,24 +59,14 @@ describe(`test if the application`, () => { to: undefined, output: 'not/exist/folder', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when output is not a folder', async () => { let cliHelper = new CLIHelper({ ...testConfig, output: 'file' }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when repo is not git repository', async () => { @@ -105,13 +74,8 @@ describe(`test if the application`, () => { ...testConfig, repo: 'not/git/folder', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when file is not found for --ignore', async () => { @@ -119,13 +83,8 @@ describe(`test if the application`, () => { ...testConfig, ignore: 'not-a-file', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when file is not found for --ignore-destructive', async () => { @@ -133,13 +92,8 @@ describe(`test if the application`, () => { ...testConfig, ignoreDestructive: 'not-a-file', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when file is not found for --include', async () => { @@ -147,13 +101,8 @@ describe(`test if the application`, () => { ...testConfig, include: 'not-a-file', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when file is not found for --include-destructive', async () => { @@ -161,13 +110,8 @@ describe(`test if the application`, () => { ...testConfig, includeDestructive: 'not-a-file', }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() }) test('throws errors when "-t" and "-d" are set', async () => { @@ -177,12 +121,61 @@ describe(`test if the application`, () => { to: notHeadSHA, generateDelta: true, }) - try { - await cliHelper.validateConfig() - } catch (e) { - expect(e).toBeDefined() - return - } - expect(true).toBe(false) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow() + }) + + test('throws errors when "-t" is not a git expression', async () => { + const emptyString = '' + let cliHelper = new CLIHelper({ + ...testConfig, + to: emptyString, + generateDelta: false, + }) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow( + `--to is blank: '${emptyString}'` + ) + }) + + test('throws errors when "-f" is not a git expression', async () => { + const emptyString = '' + let cliHelper = new CLIHelper({ + ...testConfig, + from: emptyString, + generateDelta: false, + }) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow( + `--from is blank: '${emptyString}'` + ) + }) + + test('throws errors when "-t" is not a valid sha pointer', async () => { + child_process.__setOutput([[COMMIT_REF_TYPE], ['not a valid sha pointer']]) + const notHeadSHA = 'test' + let cliHelper = new CLIHelper({ + ...testConfig, + to: notHeadSHA, + generateDelta: false, + }) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow( + `--to is not a valid sha pointer: '${notHeadSHA}'` + ) + }) + + test('throws errors when "-f" is not a valid sha pointer', async () => { + child_process.__setOutput([['not a valid sha pointer'], [COMMIT_REF_TYPE]]) + const notHeadSHA = 'test' + let cliHelper = new CLIHelper({ + ...testConfig, + from: notHeadSHA, + generateDelta: false, + }) + expect.assertions(1) + await expect(cliHelper.validateConfig()).rejects.toThrow( + `--from is not a valid sha pointer: '${notHeadSHA}'` + ) }) }) diff --git a/__tests__/unit/lib/utils/repoSetup.test.js b/__tests__/unit/lib/utils/repoSetup.test.js index 49123e29..f40aa5e5 100644 --- a/__tests__/unit/lib/utils/repoSetup.test.js +++ b/__tests__/unit/lib/utils/repoSetup.test.js @@ -21,24 +21,6 @@ describe(`test if repoSetup`, () => { expect(toEqualHead).toBe(false) }) - test('can set config.from if not defined', async () => { - const config = { repo: '.' } - child_process.__setOutput([['firstSha']]) - const repoSetup = new RepoSetup(config) - const firsSha = await repoSetup.computeFromRef() - - expect(firsSha).not.toBeUndefined() - }) - - test('can set config.from if defined', async () => { - const config = { repo: '.', from: 'HEAD~1' } - child_process.__setOutput([['firstSha']]) - const repoSetup = new RepoSetup(config) - const firsSha = await repoSetup.computeFromRef() - - expect(firsSha).not.toBeUndefined() - }) - test('can set core.quotepath to off', async () => { const config = { repo: '.', from: 'HEAD~1' } child_process.__setOutput([['']]) diff --git a/src/utils/childProcessUtils.js b/src/utils/childProcessUtils.js index 0c9789df..5a213125 100644 --- a/src/utils/childProcessUtils.js +++ b/src/utils/childProcessUtils.js @@ -22,6 +22,16 @@ async function* linify(stream) { } } +const getStreamContent = async stream => { + const content = [] + for await (const chunk of stream.stdout) { + content.push(chunk) + } + + return content.join('') +} + +module.exports.getStreamContent = getStreamContent +module.exports.linify = linify module.exports.treatPathSep = treatPathSep module.exports.sanitizePath = sanitizePath -module.exports.linify = linify diff --git a/src/utils/cliHelper.js b/src/utils/cliHelper.js index ff1e549f..685385fd 100644 --- a/src/utils/cliHelper.js +++ b/src/utils/cliHelper.js @@ -1,9 +1,9 @@ 'use strict' const asyncFilter = require('./asyncFilter') const RepoSetup = require('./repoSetup') -const { sanitizePath } = require('./childProcessUtils') -const { GIT_FOLDER } = require('./gitConstants') - +const { sanitizePath, getStreamContent } = require('./childProcessUtils') +const { COMMIT_REF_TYPE, GIT_FOLDER } = require('./gitConstants') +const { spawn } = require('child_process') const { stat } = require('fs').promises const { join } = require('path') @@ -24,18 +24,46 @@ const isGit = async dir => { return await dirExists(join(dir, GIT_FOLDER)) } +const commitCheckParams = ['cat-file', '-t'] + +const isBlank = str => !str || /^\s*$/.test(str) + class CLIHelper { constructor(config) { this.config = config this.repoSetup = new RepoSetup(config) } + async _validateGitSha() { + const errors = [] + await Promise.all( + ['to', 'from'] + .filter( + field => + !isBlank(this.config[field]) || + errors.push(`--${field} is blank: '${this.config[field]}'`) + ) + .map(async field => { + const refType = await getStreamContent( + spawn('git', [...commitCheckParams, this.config[field]], { + cwd: this.config.repo, + }) + ) + if (refType?.replace(/\s/g, '') !== COMMIT_REF_TYPE) { + errors.push( + `--${field} is not a valid sha pointer: '${this.config[field]}'` + ) + } + }) + ) + + return errors + } + async validateConfig() { - await this._sanitizeConfig() + this._sanitizeConfig() const errors = [] - if (typeof this.config.to !== 'string') { - errors.push(`to ${this.config.to} is not a sha`) - } + if (isNaN(this.config.apiVersion)) { errors.push(`api-version ${this.config.apiVersion} is not a number`) } @@ -56,6 +84,9 @@ class CLIHelper { errors.push(`${this.config.repo} is not a git repository`) } + const gitErrors = await this._validateGitSha() + errors.push(...gitErrors) + const isToEqualHead = await isToEqualHeadPromise if (!isToEqualHead && this.config.generateDelta) { errors.push( @@ -95,14 +126,13 @@ class CLIHelper { ) } - async _sanitizeConfig() { + _sanitizeConfig() { this.config.apiVersion = parseInt(this.config.apiVersion) this.config.repo = sanitizePath(this.config.repo) this.config.source = sanitizePath(this.config.source) this.config.output = sanitizePath(this.config.output) this.config.ignore = sanitizePath(this.config.ignore) this.config.ignoreDestructive = sanitizePath(this.config.ignoreDestructive) - this.config.from = await this.repoSetup.computeFromRef() this.config.include = sanitizePath(this.config.include) this.config.includeDestructive = sanitizePath( this.config.includeDestructive diff --git a/src/utils/gitConstants.js b/src/utils/gitConstants.js index f4a2323a..0f685370 100644 --- a/src/utils/gitConstants.js +++ b/src/utils/gitConstants.js @@ -2,6 +2,7 @@ module.exports.ADDITION = 'A' module.exports.DELETION = 'D' module.exports.MODIFICATION = 'M' +module.exports.COMMIT_REF_TYPE = 'commit' module.exports.GIT_DIFF_TYPE_REGEX = /^.\s+/u module.exports.GIT_FOLDER = '.git' module.exports.MINUS = '-' diff --git a/src/utils/repoSetup.js b/src/utils/repoSetup.js index 7122a5a8..78e5ccd9 100644 --- a/src/utils/repoSetup.js +++ b/src/utils/repoSetup.js @@ -1,20 +1,12 @@ 'use strict' +const { getStreamContent } = require('./childProcessUtils') const { spawn } = require('child_process') const HEAD = 'HEAD' const revparseParams = ['rev-parse'] -const revlistParams = ['rev-list', '--max-parents=0', HEAD] const gitConfig = ['config', 'core.quotepath', 'off'] -const _getStreamContent = async stream => { - const content = [] - for await (const chunk of stream) { - content.push(chunk) - } - return content.join('') -} - class RepoSetup { constructor(config) { this.config = config @@ -28,29 +20,19 @@ class RepoSetup { return true } - const headSHA = await _getStreamContent( - spawn('git', [...revparseParams, HEAD], this.spawnConfig).stdout + const headSHA = await getStreamContent( + spawn('git', [...revparseParams, HEAD], this.spawnConfig) ) - const toSHA = await _getStreamContent( - spawn('git', [...revparseParams, this.config.to], this.spawnConfig).stdout + const toSHA = await getStreamContent( + spawn('git', [...revparseParams, this.config.to], this.spawnConfig) ) return toSHA === headSHA } async repoConfiguration() { - await _getStreamContent(spawn('git', gitConfig, this.spawnConfig).stdout) - } - - async computeFromRef() { - let firstCommitSHA = this.config.from - if (!firstCommitSHA) { - firstCommitSHA = await _getStreamContent( - spawn('git', revlistParams, this.spawnConfig).stdout - ) - } - return firstCommitSHA + await getStreamContent(spawn('git', gitConfig, this.spawnConfig)) } }