fix: deploy errors are reported properly

src/formatters/deployResultFormatter.ts

@@ -51,7 +51,7 @@ export class DeployResultFormatter extends ResultFormatter {
   }
 
   /**
-   * Displays deploy results in human format.  Output can vary based on:
+   * Displays deploy results in human readable format.  Output can vary based on:
    *
    * 1. Verbose option
    * 3. Checkonly deploy (checkonly=true)
@@ -85,19 +85,6 @@ export class DeployResultFormatter extends ResultFormatter {
     return getString(this.result, 'response.status') === status;
   }
 
-  // Returns true if the components returned in the server response
-  // were mapped to local source in the ComponentSet.
-  protected hasMappedComponents(): boolean {
-    return getNumber(this.result, 'components.size', 0) > 0;
-  }
-
-  // Returns true if the server response contained components.
-  protected hasComponents(): boolean {
-    const successes = getNumber(this.result, 'response.details.componentSuccesses.length', 0) > 0;
-    const failures = getNumber(this.result, 'response.details.componentFailures.length', 0) > 0;
-    return successes || failures;
-  }
-
   protected isRunTestsEnabled(): boolean {
     return getBoolean(this.result, 'response.runTestsEnabled', false);
   }
@@ -115,13 +102,17 @@ export class DeployResultFormatter extends ResultFormatter {
   }
 
   protected displaySuccesses(): void {
-    if (this.isSuccess() && this.hasComponents()) {
-      this.sortFileResponses(this.fileResponses);
-      this.asRelativePaths(this.fileResponses);
+    if (this.isSuccess() && this.fileResponses?.length) {
+      const successes = this.fileResponses.filter((f) => f.state !== 'Failed');
+      if (!successes.length) {
+        return;
+      }
+      this.sortFileResponses(successes);
+      this.asRelativePaths(successes);
 
       this.ux.log('');
       this.ux.styledHeader(chalk.blue('Deployed Source'));
-      this.ux.table(this.fileResponses, {
+      this.ux.table(successes, {
         columns: [
           { key: 'fullName', label: 'FULL NAME' },
           { key: 'type', label: 'TYPE' },
@@ -132,15 +123,13 @@ export class DeployResultFormatter extends ResultFormatter {
   }
 
   protected displayFailures(): void {
-    if (this.hasStatus(RequestStatus.Failed) && this.hasComponents()) {
+    if (this.hasStatus(RequestStatus.Failed) && this.fileResponses?.length) {
       const failures = this.fileResponses.filter((f) => f.state === 'Failed');
       this.sortFileResponses(failures);
       this.asRelativePaths(failures);
 
       this.ux.log('');
       this.ux.styledHeader(chalk.red(`Component Failures [${failures.length}]`));
-      // TODO: do we really need the project path or file path in the table?
-      // Seems like we can just provide the full name and devs will know.
       this.ux.table(failures, {
         columns: [
           { key: 'problemType', label: 'Type' },

test/commands/source/deployResponses.ts

@@ -6,7 +6,12 @@
  */
 
 import { DeployResult } from '@salesforce/source-deploy-retrieve';
-import { MetadataApiDeployStatus, RequestStatus } from '@salesforce/source-deploy-retrieve/lib/src/client/types';
+import {
+  DeployMessage,
+  MetadataApiDeployStatus,
+  RequestStatus,
+} from '@salesforce/source-deploy-retrieve/lib/src/client/types';
+import { cloneJson } from '@salesforce/kit';
 
 const baseDeployResponse = {
   checkOnly: false,
@@ -73,15 +78,26 @@ export const getDeployResponse = (
   type: DeployResponseType,
   overrides?: Partial<MetadataApiDeployStatus>
 ): MetadataApiDeployStatus => {
-  const response = { ...baseDeployResponse, ...overrides };
+  const response = JSON.parse(JSON.stringify({ ...baseDeployResponse, ...overrides })) as MetadataApiDeployStatus;
 
   if (type === 'canceled') {
     response.canceledBy = '0051h000006BHOq';
     response.canceledByName = 'Canceling User';
     response.status = RequestStatus.Canceled;
   }
 
-  return response as MetadataApiDeployStatus;
+  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';
+    delete response.details.componentFailures.id;
+    response.details.componentFailures.problemType = 'Error';
+    response.details.componentFailures.problem = 'This component has some problems';
+  }
+
+  return response;
 };
 
 export const getDeployResult = (
@@ -93,14 +109,30 @@ export const getDeployResult = (
   return {
     response,
     getFileResponses() {
-      let successes = response.details.componentSuccesses;
-      successes = Array.isArray(successes) ? successes : [successes];
-      return successes.map((comp) => ({
-        fullName: comp.fullName,
-        filePath: comp.fileName,
-        state: 'Changed',
-        type: comp.componentType,
-      }));
+      let fileProps: DeployMessage[] = [];
+      if (type === 'failed') {
+        const failures = response.details.componentFailures || [];
+        fileProps = Array.isArray(failures) ? failures : [failures];
+        return fileProps.map((comp) => ({
+          fullName: comp.fullName,
+          filePath: comp.fileName,
+          state: 'Failed',
+          type: comp.componentType,
+          error: comp.problem,
+          problemType: comp.problemType,
+        }));
+      } else {
+        const successes = response.details.componentSuccesses;
+        fileProps = Array.isArray(successes) ? successes : [successes];
+        return fileProps
+          .filter((p) => p.fileName !== 'package.xml')
+          .map((comp) => ({
+            fullName: comp.fullName,
+            filePath: comp.fileName,
+            state: 'Changed',
+            type: comp.componentType,
+          }));
+      }
     },
   } as DeployResult;
 };

test/formatters/deployResultFormatter.test.ts

@@ -0,0 +1,90 @@
+/*
+ * 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 * as sinon from 'sinon';
+import { expect } from 'chai';
+import { Logger } from '@salesforce/core';
+import { UX } from '@salesforce/command';
+import { stubInterface } from '@salesforce/ts-sinon';
+import { getDeployResult } from '../commands/source/deployResponses';
+import { DeployCommandResult, DeployResultFormatter } from '../../src/formatters/deployResultFormatter';
+
+describe('DeployResultFormatter', () => {
+  const sandbox = sinon.createSandbox();
+
+  const deployResultSuccess = getDeployResult('successSync');
+  const deployResultFailure = getDeployResult('failed');
+
+  const logger = Logger.childFromRoot('deployTestLogger').useMemoryLogging();
+  let ux;
+  let logStub: sinon.SinonStub;
+  let styledHeaderStub: sinon.SinonStub;
+  let tableStub: sinon.SinonStub;
+
+  beforeEach(() => {
+    logStub = sandbox.stub();
+    styledHeaderStub = sandbox.stub();
+    tableStub = sandbox.stub();
+    ux = stubInterface<UX>(sandbox, {
+      log: logStub,
+      styledHeader: styledHeaderStub,
+      table: tableStub,
+    });
+  });
+
+  afterEach(() => {
+    sandbox.restore();
+  });
+
+  describe('getJson', () => {
+    it('should return expected json for a success', async () => {
+      const deployResponse = JSON.parse(JSON.stringify(deployResultSuccess.response)) as DeployCommandResult;
+      const expectedSuccessResults = deployResultSuccess.response as DeployCommandResult;
+      const formatter = new DeployResultFormatter(logger, ux, {}, deployResultSuccess);
+      const json = formatter.getJson();
+
+      expectedSuccessResults.deployedSource = deployResultSuccess.getFileResponses();
+      expectedSuccessResults.outboundFiles = [];
+      expectedSuccessResults.deploys = [deployResponse];
+      expect(json).to.deep.equal(expectedSuccessResults);
+    });
+
+    it('should return expected json for a failure', async () => {
+      const deployResponse = JSON.parse(JSON.stringify(deployResultFailure.response)) as DeployCommandResult;
+      const expectedFailureResults = deployResultFailure.response as DeployCommandResult;
+      expectedFailureResults.deployedSource = deployResultFailure.getFileResponses();
+      expectedFailureResults.outboundFiles = [];
+      expectedFailureResults.deploys = [deployResponse];
+      const formatter = new DeployResultFormatter(logger, ux, {}, deployResultFailure);
+      expect(formatter.getJson()).to.deep.equal(expectedFailureResults);
+    });
+  });
+
+  describe('display', () => {
+    it('should output as expected for a success', async () => {
+      const formatter = new DeployResultFormatter(logger, ux, {}, deployResultSuccess);
+      formatter.display();
+      expect(styledHeaderStub.calledOnce).to.equal(true);
+      expect(logStub.calledOnce).to.equal(true);
+      expect(tableStub.called).to.equal(true);
+      expect(styledHeaderStub.firstCall.args[0]).to.contain('Deployed Source');
+      const fileResponses = deployResultSuccess.getFileResponses();
+      expect(tableStub.firstCall.args[0]).to.deep.equal(fileResponses);
+    });
+
+    it('should output as expected for a failure', async () => {
+      const formatter = new DeployResultFormatter(logger, ux, {}, deployResultFailure);
+      formatter.display();
+      expect(styledHeaderStub.calledOnce).to.equal(true);
+      expect(logStub.calledTwice).to.equal(true);
+      expect(tableStub.called).to.equal(true);
+      expect(styledHeaderStub.firstCall.args[0]).to.contain('Component Failures [1]');
+      const fileResponses = deployResultFailure.getFileResponses();
+      expect(tableStub.firstCall.args[0]).to.deep.equal(fileResponses);
+    });
+  });
+});