diff --git a/src/commands/force/source/beta/pull.ts b/src/commands/force/source/beta/pull.ts index 92e15cc8e..50267f4c9 100644 --- a/src/commands/force/source/beta/pull.ts +++ b/src/commands/force/source/beta/pull.ts @@ -47,7 +47,7 @@ export default class Pull extends SourceCommand { protected retrieveResult: RetrieveResult; protected deleteFileResponses: FileResponse[]; - public async run(): Promise { + public async run(): Promise { await this.preChecks(); await this.retrieve(); // do not parallelize delete and retrieve...we only get to delete IF retrieve was successful @@ -139,7 +139,7 @@ export default class Pull extends SourceCommand { } } - protected formatResult(): PullResponse[] { + protected formatResult(): PullResponse { const formatterOptions = { verbose: this.getFlag('verbose', false), }; diff --git a/src/formatters/source/pullFormatter.ts b/src/formatters/source/pullFormatter.ts index a78d2f1d7..aeddec0d1 100644 --- a/src/formatters/source/pullFormatter.ts +++ b/src/formatters/source/pullFormatter.ts @@ -21,7 +21,7 @@ import { ResultFormatter, ResultFormatterOptions, toArray } from '../resultForma Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/plugin-source', 'pull'); -export type PullResponse = Pick; +export type PullResponse = { pulledSource: Array> }; export class PullResultFormatter extends ResultFormatter { protected result: RetrieveResult; @@ -53,8 +53,22 @@ export class PullResultFormatter extends ResultFormatter { * * @returns RetrieveCommandResult */ - public getJson(): PullResponse[] { - return this.fileResponses.map(({ state, fullName, type, filePath }) => ({ state, fullName, type, filePath })); + public getJson(): PullResponse { + if (!this.isSuccess()) { + const error = new SfdxError('Pull failed.', 'PullFailed', [], process.exitCode); + error.setData( + this.fileResponses.map(({ state, fullName, type, filePath }) => ({ state, fullName, type, filePath })) + ); + throw error; + } + return { + pulledSource: this.fileResponses.map(({ state, fullName, type, filePath }) => ({ + state, + fullName, + type, + filePath, + })), + }; } /** diff --git a/src/formatters/source/pushResultFormatter.ts b/src/formatters/source/pushResultFormatter.ts index cb5f151a2..60082a633 100644 --- a/src/formatters/source/pushResultFormatter.ts +++ b/src/formatters/source/pushResultFormatter.ts @@ -46,6 +46,20 @@ export class PushResultFormatter extends ResultFormatter { * @returns a JSON formatted result matching the provided type. */ public getJson(): PushResponse { + // throws a particular json structure. commandName property will be appended by sfdxCommand when this throws + if (process.exitCode !== 0) { + const error = new SfdxError(messages.getMessage('sourcepushFailed'), 'DeployFailed', [], process.exitCode); + const errorData = this.fileResponses.filter((fileResponse) => fileResponse.state === ComponentStatus.Failed); + error.setData(errorData); + error['result'] = errorData; + // partial success + if (process.exitCode === 69) { + error['partialSuccess'] = this.fileResponses.filter( + (fileResponse) => fileResponse.state !== ComponentStatus.Failed + ); + } + throw error; + } // quiet returns only failures const toReturn = this.isQuiet() ? this.fileResponses.filter((fileResponse) => fileResponse.state === ComponentStatus.Failed) diff --git a/src/trackingFunctions.ts b/src/trackingFunctions.ts index 090a017d3..774634f73 100644 --- a/src/trackingFunctions.ts +++ b/src/trackingFunctions.ts @@ -4,6 +4,7 @@ * Licensed under the BSD 3-Clause license. * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause */ +import * as path from 'path'; import { UX } from '@salesforce/command'; import { ChangeResult, @@ -21,7 +22,6 @@ import { FileResponse, RetrieveResult, } from '@salesforce/source-deploy-retrieve'; - Messages.importMessagesDirectory(__dirname); const messages = Messages.loadMessages('@salesforce/plugin-source', 'tracking'); @@ -42,6 +42,12 @@ interface TrackingUpdateRequest { fileResponses?: FileResponse[]; } +interface ConflictResponse { + state: 'Conflict'; + fullName: string; + type: string; + filePath: string; +} /** * Check if any conflicts exist in a specific component set. * If conflicts exist, this will output the table and throw @@ -141,18 +147,15 @@ export const updateTracking = async ({ tracking, result, ux, fileResponses }: Tr ux.stopSpinner(); }; -const writeConflictTable = (conflicts: ChangeResult[], ux: UX): void => { - ux.table( - conflicts.map((conflict) => ({ ...conflict, state: 'Conflict' })), - { - columns: [ - { label: 'STATE', key: 'state' }, - { label: 'FULL NAME', key: 'name' }, - { label: 'TYPE', key: 'type' }, - { label: 'PROJECT PATH', key: 'filenames' }, - ], - } - ); +const writeConflictTable = (conflicts: ConflictResponse[], ux: UX): void => { + ux.table(conflicts, { + columns: [ + { label: 'STATE', key: 'state' }, + { label: 'FULL NAME', key: 'name' }, + { label: 'TYPE', key: 'type' }, + { label: 'PROJECT PATH', key: 'filenames' }, + ], + }); }; /** @@ -166,8 +169,16 @@ const processConflicts = (conflicts: ChangeResult[], ux: UX, message: string): v if (conflicts.length === 0) { return; } - writeConflictTable(conflicts, ux); - const err = new SfdxError(message); - err.setData(conflicts); + const reformattedConflicts: ConflictResponse[] = conflicts.flatMap((conflict) => + conflict.filenames.map((f) => ({ + state: 'Conflict', + fullName: conflict.name, + type: conflict.type, + filePath: path.resolve(f), + })) + ); + writeConflictTable(reformattedConflicts, ux); + const err = new SfdxError(message, 'sourceConflictDetected'); + err.setData(reformattedConflicts); throw err; }; diff --git a/test/commands/source/deployResponses.ts b/test/commands/source/deployResponses.ts index 7d077d299..36b669e43 100644 --- a/test/commands/source/deployResponses.ts +++ b/test/commands/source/deployResponses.ts @@ -102,12 +102,16 @@ export const getDeployResponse = ( if (type === 'failed') { response.status = RequestStatus.Failed; response.success = false; - response.details.componentFailures = cloneJson(baseDeployResponse.details.componentSuccesses[1]) as DeployMessage; response.details.componentSuccesses = cloneJson(baseDeployResponse.details.componentSuccesses[0]) as DeployMessage; - response.details.componentFailures.success = 'false'; + response.details.componentFailures = { + ...(cloneJson(baseDeployResponse.details.componentSuccesses[1]) as DeployMessage), + success: false, + problemType: 'Error', + problem: 'This component has some problems', + lineNumber: '27', + columnNumber: '18', + }; delete response.details.componentFailures.id; - response.details.componentFailures.problemType = 'Error'; - response.details.componentFailures.problem = 'This component has some problems'; } if (type === 'failedTest') { @@ -305,6 +309,8 @@ export const getDeployResult = ( type: comp.componentType, error: comp.problem, problemType: comp.problemType, + lineNumber: comp.lineNumber, + columnNumber: comp.columnNumber, })); } else { const successes = response.details.componentSuccesses; diff --git a/test/formatters/pullFormatter.test.ts b/test/formatters/pullFormatter.test.ts new file mode 100644 index 000000000..9a60b5a94 --- /dev/null +++ b/test/formatters/pullFormatter.test.ts @@ -0,0 +1,152 @@ +/* + * Copyright (c) 2020, salesforce.com, inc. + * All rights reserved. + * Licensed under the BSD 3-Clause license. + * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause + */ + +import { relative } from 'path'; +import * as sinon from 'sinon'; +import { expect } from 'chai'; +import { Logger } from '@salesforce/core'; +import { UX } from '@salesforce/command'; +import { FileResponse } from '@salesforce/source-deploy-retrieve'; +// import { cloneJson } from '@salesforce/kit'; +import { stubInterface } from '@salesforce/ts-sinon'; +import { getRetrieveResult } from '../commands/source/retrieveResponses'; +import { PullResponse, PullResultFormatter } from '../../src/formatters/source/pullFormatter'; +import { toArray } from '../../src/formatters/resultFormatter'; + +describe('PullFormatter', () => { + const sandbox = sinon.createSandbox(); + + const retrieveResultSuccess = getRetrieveResult('success'); + const retrieveResultFailure = getRetrieveResult('failed'); + const retrieveResultInProgress = getRetrieveResult('inProgress'); + const retrieveResultEmpty = getRetrieveResult('empty'); + const retrieveResultWarnings = getRetrieveResult('warnings'); + + const logger = Logger.childFromRoot('retrieveTestLogger').useMemoryLogging(); + let ux; + let logStub: sinon.SinonStub; + let styledHeaderStub: sinon.SinonStub; + let tableStub: sinon.SinonStub; + + const resolveExpectedPaths = (fileResponses: FileResponse[]): void => { + fileResponses.forEach((file) => { + if (file.filePath) { + file.filePath = relative(process.cwd(), file.filePath); + } + }); + }; + + beforeEach(() => { + logStub = sandbox.stub(); + styledHeaderStub = sandbox.stub(); + tableStub = sandbox.stub(); + ux = stubInterface(sandbox, { + log: logStub, + styledHeader: styledHeaderStub, + table: tableStub, + }); + }); + + afterEach(() => { + sandbox.restore(); + process.exitCode = undefined; + }); + + describe('getJson', () => { + it('should return expected json for a success', async () => { + process.exitCode = 0; + const expectedSuccessResults: PullResponse['pulledSource'] = retrieveResultSuccess.getFileResponses(); + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultSuccess); + expect(formatter.getJson().pulledSource).to.deep.equal(expectedSuccessResults); + }); + + it('should return expected json for a failure', async () => { + process.exitCode = 1; + const expectedFailureResults: PullResponse['pulledSource'] = retrieveResultFailure.getFileResponses(); + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultFailure); + try { + formatter.getJson().pulledSource; + throw new Error('should have thrown'); + } catch (error) { + expect(error).to.have.property('message', 'Pull failed.'); + expect(error).to.have.property('data').deep.equal(expectedFailureResults); + expect(error).to.have.property('stack').includes('PullFailed:'); + expect(error).to.have.property('name', 'PullFailed'); + } + }); + + it('should return expected json for an InProgress', async () => { + const expectedInProgressResults: PullResponse['pulledSource'] = retrieveResultInProgress.getFileResponses(); + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultInProgress); + expect(formatter.getJson().pulledSource).to.deep.equal(expectedInProgressResults); + }); + + describe('display', () => { + it('should output as expected for a success', async () => { + process.exitCode = 0; + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultSuccess); + formatter.display(); + expect(styledHeaderStub.called).to.equal(true); + expect(logStub.called).to.equal(false); + expect(tableStub.called).to.equal(true); + expect(styledHeaderStub.firstCall.args[0]).to.contain('Retrieved Source'); + const fileResponses = retrieveResultSuccess.getFileResponses(); + resolveExpectedPaths(fileResponses); + expect(tableStub.firstCall.args[0]).to.deep.equal(fileResponses); + }); + + it('should output as expected for an InProgress', async () => { + process.exitCode = 68; + const options = { waitTime: 33 }; + const formatter = new PullResultFormatter(logger, ux as UX, options, retrieveResultInProgress); + formatter.display(); + expect(styledHeaderStub.called).to.equal(false); + expect(logStub.called).to.equal(true); + expect(tableStub.called).to.equal(false); + expect(logStub.firstCall.args[0]) + .to.contain('Your retrieve request did not complete') + .and.contain(`${options.waitTime} minutes`); + }); + + it('should output as expected for a Failure', async () => { + process.exitCode = 1; + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultFailure); + sandbox.stub(formatter, 'isSuccess').returns(false); + + formatter.display(); + expect(logStub.called).to.equal(true); + expect(tableStub.called).to.equal(false); + expect(logStub.firstCall.args[0]).to.contain('Retrieve Failed due to:'); + }); + + it('should output as expected for warnings', async () => { + process.exitCode = 0; + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultWarnings); + formatter.display(); + // Should call styledHeader for warnings and the standard "Retrieved Source" header + expect(styledHeaderStub.calledTwice).to.equal(true); + expect(logStub.called).to.equal(true); + expect(tableStub.calledOnce).to.equal(true); + expect(styledHeaderStub.secondCall.args[0]).to.contain('Retrieved Source Warnings'); + const warnMessages = retrieveResultWarnings.response.messages; + const warnings = toArray(warnMessages); + expect(tableStub.firstCall.args[0]).to.deep.equal(warnings); + }); + + it('should output a message when no results were returned', async () => { + process.exitCode = 0; + const formatter = new PullResultFormatter(logger, ux as UX, {}, retrieveResultEmpty); + formatter.display(); + expect(styledHeaderStub.called).to.equal(true); + expect(logStub.called).to.equal(true); + expect(tableStub.called).to.equal(false); + expect(styledHeaderStub.firstCall.args[0]).to.contain('Retrieved Source'); + expect(logStub.firstCall.args[0]).to.contain('No results found'); + }); + }); + }); +}); diff --git a/test/formatters/pushResultFormatter.test.ts b/test/formatters/pushResultFormatter.test.ts index 385e5a796..07cd34a47 100644 --- a/test/formatters/pushResultFormatter.test.ts +++ b/test/formatters/pushResultFormatter.test.ts @@ -37,7 +37,18 @@ describe('PushResultFormatter', () => { }); describe('json', () => { + const expectedFail = { + filePath: 'classes/ProductController.cls', + fullName: 'ProductController', + state: 'Failed', + type: 'ApexClass', + lineNumber: '27', + columnNumber: '18', + problemType: 'Error', + error: 'This component has some problems', + }; it('returns expected json for success', () => { + process.exitCode = 0; const formatter = new PushResultFormatter(logger, new UX(logger), {}, deployResultSuccess); expect(formatter.getJson().pushedSource).to.deep.equal([ { @@ -48,14 +59,37 @@ describe('PushResultFormatter', () => { }, ]); }); + it('returns expected json for failure', () => { + const formatter = new PushResultFormatter(logger, new UX(logger), {}, deployResultFailure); + process.exitCode = 1; + + try { + formatter.getJson(); + throw new Error('should have thrown'); + } catch (error) { + expect(error).to.have.property('message', 'Push failed.'); + expect(error).to.have.property('name', 'DeployFailed'); + expect(error).to.have.property('stack').includes('DeployFailed:'); + expect(error).to.have.property('actions').deep.equal([]); + expect(error).to.have.property('data').deep.equal([expectedFail]); + expect(error).to.have.property('result').deep.equal([expectedFail]); + } + }); describe('json with quiet', () => { it('honors quiet flag for json successes', () => { + process.exitCode = 0; const formatter = new PushResultFormatter(logger, new UX(logger), { quiet: true }, deployResultSuccess); expect(formatter.getJson().pushedSource).to.deep.equal([]); }); - it('honors quiet flag for json successes', () => { + it('honors quiet flag for json failure', () => { const formatter = new PushResultFormatter(logger, new UX(logger), { quiet: true }, deployResultFailure); - expect(formatter.getJson().pushedSource).to.have.length(1); + try { + formatter.getJson(); + throw new Error('should have thrown'); + } catch (error) { + expect(error).to.have.property('message', 'Push failed.'); + expect(error).to.have.property('result').deep.equal([expectedFail]); + } }); }); }); diff --git a/test/nuts/trackingCommands/basics.nut.ts b/test/nuts/trackingCommands/basics.nut.ts index 9c53ff0b0..7ccffe731 100644 --- a/test/nuts/trackingCommands/basics.nut.ts +++ b/test/nuts/trackingCommands/basics.nut.ts @@ -10,7 +10,7 @@ import * as fs from 'fs'; import { expect } from 'chai'; import * as shelljs from 'shelljs'; import { execCmd, TestSession } from '@salesforce/cli-plugins-testkit'; -import { ComponentStatus } from '@salesforce/source-deploy-retrieve'; +import { ComponentStatus, FileResponse } from '@salesforce/source-deploy-retrieve'; import { replaceRenamedCommands } from '@salesforce/source-tracking'; import { PushResponse } from '../../../src/formatters/source/pushResultFormatter'; import { StatusResult } from '../../../src/formatters/source/statusFormatter'; @@ -70,11 +70,11 @@ describe('end-to-end-test for tracking with an org (single packageDir)', () => { }); it('can pull the remote profile', () => { - const pullResult = execCmd(replaceRenamedCommands('force:source:pull --json'), { + const pullResult = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0, }).jsonOutput.result; expect( - pullResult.some((item) => item.type === 'Profile'), + pullResult.pulledSource.some((item) => item.type === 'Profile'), JSON.stringify(pullResult) ).to.equal(true); }); @@ -172,10 +172,22 @@ describe('end-to-end-test for tracking with an org (single packageDir)', () => { ]); }); it('fails to push', () => { - const result = execCmd(replaceRenamedCommands('force:source:push --json'), { + const failure = execCmd(replaceRenamedCommands('force:source:push --json'), { ensureExitCode: 1, - }).jsonOutput.result.pushedSource; - expect(result.every((r) => r.type === 'ApexClass' && r.state === 'Failed')).to.equal(true); + }).jsonOutput as unknown as { name: string; exitCode: number; result: FileResponse[]; data: FileResponse[] }; + expect(failure).to.have.property('exitCode', 1); + expect(failure).to.have.property('commandName', 'Push'); + expect( + failure.result.every((r) => r.type === 'ApexClass' && r.state === 'Failed' && r.problemType === 'Error') + ).to.equal(true); + failure.result.forEach((f) => { + if (f.state === 'Failed') { + expect(f.lineNumber).to.exist; + expect(f.columnNumber).to.exist; + expect(f.error).to.be.a('string'); + } + }); + expect(failure.result).to.deep.equal(failure.data); }); it('classes that failed to deploy are still in local status', () => { it('sees no local changes', () => { diff --git a/test/nuts/trackingCommands/forceIgnore.nut.ts b/test/nuts/trackingCommands/forceIgnore.nut.ts index be8eedd61..aaaec836d 100644 --- a/test/nuts/trackingCommands/forceIgnore.nut.ts +++ b/test/nuts/trackingCommands/forceIgnore.nut.ts @@ -142,10 +142,10 @@ describe('forceignore changes', () => { expect(statusOutput.some((result) => result.fullName === 'CreatedClass')).to.equal(true); // pull doesn't retrieve that change - const pullOutput = execCmd(replaceRenamedCommands('force:source:pull --json'), { + const pullOutput = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0, }).jsonOutput.result; - expect(pullOutput.some((result) => result.fullName === 'CreatedClass')).to.equal(false); + expect(pullOutput.pulledSource.some((result) => result.fullName === 'CreatedClass')).to.equal(false); }); }); }); diff --git a/test/nuts/trackingCommands/remoteChanges.nut.ts b/test/nuts/trackingCommands/remoteChanges.nut.ts index 22743f602..6e4e38a77 100644 --- a/test/nuts/trackingCommands/remoteChanges.nut.ts +++ b/test/nuts/trackingCommands/remoteChanges.nut.ts @@ -97,12 +97,14 @@ describe('remote changes', () => { expect(result).to.deep.equal([]); }); it('can pull the delete', () => { - const result = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0 }) + const result = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0 }) .jsonOutput.result; // the 2 files for the apexClass, and possibly one for the Profile (depending on whether it got created in time) - expect(result, JSON.stringify(result)).to.have.length.greaterThanOrEqual(2); - expect(result, JSON.stringify(result)).to.have.length.lessThanOrEqual(4); - result.filter((r) => r.fullName === 'TestOrderController').map((r) => expect(r.state).to.equal('Deleted')); + expect(result.pulledSource, JSON.stringify(result)).to.have.length.greaterThanOrEqual(2); + expect(result.pulledSource, JSON.stringify(result)).to.have.length.lessThanOrEqual(4); + result.pulledSource + .filter((r) => r.fullName === 'TestOrderController') + .map((r) => expect(r.state).to.equal('Deleted')); }); it('local file was deleted', () => { expect( @@ -151,10 +153,12 @@ describe('remote changes', () => { ).to.equal(true); }); it('can pull the add', () => { - const result = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0 }) + const result = execCmd(replaceRenamedCommands('force:source:pull --json'), { ensureExitCode: 0 }) .jsonOutput.result; // SDR marks all retrieves as 'Changed' even if it creates new local files. This is different from toolbelt, which marked those as 'Created' - result.filter((r) => r.fullName === className).map((r) => expect(r.state, JSON.stringify(r)).to.equal('Created')); + result.pulledSource + .filter((r) => r.fullName === className) + .map((r) => expect(r.state, JSON.stringify(r)).to.equal('Created')); }); it('sees correct local and remote status', () => { const remoteResult = execCmd(replaceRenamedCommands('force:source:status --json --remote'), {