fix: push-on-save working as expected, diagnostics cleared after successful push

packages/salesforcedx-vscode-core/src/commands/baseDeployRetrieve.ts

@@ -36,15 +36,19 @@ import { nls } from '../messages';
 import { componentSetUtils } from '../services/sdr/componentSetUtils';
 import { DeployQueue, sfdxCoreSettings } from '../settings';
 import { SfdxPackageDirectories } from '../sfdxProject';
-import { createComponentCount, formatException } from './util';
+import {
+  createComponentCount,
+  formatException,
+  SfdxCommandletExecutor
+} from './util';
 
 type DeployRetrieveResult = DeployResult | RetrieveResult;
 type DeployRetrieveOperation = MetadataApiDeploy | MetadataApiRetrieve;
 
 export abstract class DeployRetrieveExecutor<
   T
 > extends LibraryCommandletExecutor<T> {
-  protected static errorCollection = vscode.languages.createDiagnosticCollection(
+  public static errorCollection = vscode.languages.createDiagnosticCollection(
     'deploy-errors'
   );
   protected cancellable: boolean = true;
@@ -159,6 +163,7 @@ export abstract class DeployExecutor<T> extends DeployRetrieveExecutor<T> {
           );
         } else {
           DeployRetrieveExecutor.errorCollection.clear();
+          SfdxCommandletExecutor.errorCollection.clear();
         }
       }
     } finally {
@@ -277,6 +282,7 @@ export abstract class RetrieveExecutor<T> extends DeployRetrieveExecutor<T> {
   ): Promise<void> {
     if (result) {
       DeployRetrieveExecutor.errorCollection.clear();
+      SfdxCommandletExecutor.errorCollection.clear();
       const relativePackageDirs = await SfdxPackageDirectories.getPackageDirectoryPaths();
       const output = this.createOutput(result, relativePackageDirs);
       channelService.appendLine(output);

packages/salesforcedx-vscode-core/src/commands/forceSourcePush.ts

@@ -25,6 +25,7 @@ import { FORCE_SOURCE_PUSH_LOG_NAME } from '../constants';
 import { handleDiagnosticErrors } from '../diagnostics';
 import { nls } from '../messages';
 import { telemetryService } from '../telemetry';
+import { DeployRetrieveExecutor } from './baseDeployRetrieve';
 import {
   CommandParams,
   EmptyParametersGatherer,
@@ -50,9 +51,6 @@ export const pushCommand: CommandParams = {
 
 export class ForceSourcePushExecutor extends SfdxCommandletExecutor<{}> {
   private flag: string | undefined;
-  public errorCollection = vscode.languages.createDiagnosticCollection(
-    'push-errors'
-  );
   public constructor(
     flag?: string,
     public params: CommandParams = pushCommand
@@ -132,7 +130,8 @@ export class ForceSourcePushExecutor extends SfdxCommandletExecutor<{}> {
       const telemetry = new TelemetryBuilder();
       let success = false;
       try {
-        this.errorCollection.clear();
+        SfdxCommandletExecutor.errorCollection.clear();
+        DeployRetrieveExecutor.errorCollection.clear();
         if (stdOut) {
           const pushParser = new ForcePushResultParser(stdOut);
           const errors = pushParser.getErrors();
@@ -142,15 +141,16 @@ export class ForceSourcePushExecutor extends SfdxCommandletExecutor<{}> {
               errors,
               workspacePath,
               execFilePathOrPaths,
-              this.errorCollection
+              SfdxCommandletExecutor.errorCollection
             );
           } else {
             success = true;
           }
           this.outputResult(pushParser);
         }
       } catch (e) {
-        this.errorCollection.clear();
+        SfdxCommandletExecutor.errorCollection.clear();
+        DeployRetrieveExecutor.errorCollection.clear();
         if (e.name !== 'PushParserFail') {
           e.message =
             'Error while creating diagnostics for vscode problem view.';

packages/salesforcedx-vscode-core/src/commands/util/sfdxCommandletExecutor.ts

@@ -29,6 +29,9 @@ import { CommandletExecutor } from './commandletExecutor';
 
 export abstract class SfdxCommandletExecutor<T>
   implements CommandletExecutor<T> {
+  public static errorCollection = vscode.languages.createDiagnosticCollection(
+    'push-errors'
+  );
   protected showChannelOutput = true;
   protected executionCwd = workspaceUtils.getRootWorkspacePath();
   protected onDidFinishExecutionEventEmitter = new vscode.EventEmitter<

packages/salesforcedx-vscode-core/src/diagnostics/diagnostics.ts

@@ -12,6 +12,7 @@ import {
 } from '@salesforce/source-deploy-retrieve';
 import * as path from 'path';
 import * as vscode from 'vscode';
+import { SfdxCommandletExecutor } from '../commands/util';
 
 const notApplicable = 'N/A';
 
@@ -104,6 +105,7 @@ export function handleDeployDiagnostics(
   errorCollection: vscode.DiagnosticCollection
 ): vscode.DiagnosticCollection {
   errorCollection.clear();
+  SfdxCommandletExecutor.errorCollection.clear();
 
   const diagnosticMap: Map<string, vscode.Diagnostic[]> = new Map();
 

packages/salesforcedx-vscode-core/src/settings/pushOrDeployOnSave.ts

@@ -128,6 +128,7 @@ export class DeployQueue {
           default:
             displayError(e.message);
         }
+      } finally {
         this.locked = false;
       }
       this.deployWaitStart = undefined;

packages/salesforcedx-vscode-core/test/jest/commands/deployExecutor.test.ts

@@ -11,9 +11,17 @@ import {
 } from '@salesforce/salesforcedx-utils-vscode';
 import { ComponentSet } from '@salesforce/source-deploy-retrieve';
 import * as fs from 'fs';
-import { DeployExecutor } from '../../../src/commands/baseDeployRetrieve';
+import { channelService } from '../../../src/channels';
+import {
+  DeployExecutor,
+  DeployRetrieveExecutor
+} from '../../../src/commands/baseDeployRetrieve';
+import { SfdxCommandletExecutor } from '../../../src/commands/util';
+import { PersistentStorageService } from '../../../src/conflict';
 import { WorkspaceContext } from '../../../src/context/workspaceContext';
-import { sfdxCoreSettings } from '../../../src/settings';
+import * as diagnostics from '../../../src/diagnostics';
+import { DeployQueue, sfdxCoreSettings } from '../../../src/settings';
+import { SfdxPackageDirectories } from '../../../src/sfdxProject';
 
 jest.mock('@salesforce/source-deploy-retrieve', () => {
   return {
@@ -74,6 +82,11 @@ describe('Deploy Executor', () => {
       return new Promise(resolve => resolve(new ComponentSet()));
     }
   }
+  class MockErrorCollection {
+    public static clear(): void {
+      jest.fn();
+    }
+  }
 
   beforeEach(async () => {
     jest.spyOn(process, 'cwd').mockReturnValue(dummyProcessCwd);
@@ -94,7 +107,8 @@ describe('Deploy Executor', () => {
       .mockResolvedValue({ pollStatus: jest.fn() } as any);
     getEnableSourceTrackingForDeployAndRetrieveMock = jest.spyOn(
       sfdxCoreSettings,
-      'getEnableSourceTrackingForDeployAndRetrieve');
+      'getEnableSourceTrackingForDeployAndRetrieve'
+    );
   });
 
   it('should create Source Tracking and call ensureLocalTracking before deploying', async () => {
@@ -148,4 +162,140 @@ describe('Deploy Executor', () => {
     expect(ensureLocalTrackingSpy).not.toHaveBeenCalled();
     expect(deploySpy).toHaveBeenCalled();
   });
+
+  it('unsuccessfulOperationHandler', () => {
+    // Arrange
+    const mockDeployResult = {
+      response: {
+        status: 'Failed'
+      }
+    };
+    const handleDeployDiagnosticsSpy = jest
+      .spyOn(diagnostics, 'handleDeployDiagnostics')
+      .mockImplementation(jest.fn());
+    DeployRetrieveExecutor.errorCollection = MockErrorCollection as any;
+    const executor = new TestDeployExecutor(
+      'testDeploy',
+      'force_source_deploy_with_sourcepath_beta'
+    );
+
+    // Act
+    (executor as any).unsuccessfulOperationHandler(
+      mockDeployResult,
+      DeployRetrieveExecutor.errorCollection
+    );
+
+    expect(handleDeployDiagnosticsSpy).toHaveBeenCalledWith(
+      mockDeployResult,
+      DeployRetrieveExecutor.errorCollection
+    );
+  });
+
+  describe('postOperation', () => {
+    let mockUnlock: any;
+    let unlockSpy: any;
+    let setPropertiesForFilesDeployMock: any;
+    let getInstanceSpy: any;
+    let getPackageDirectoryPathsSpy: any;
+    let createOutputSpy: any;
+    let appendLineSpy: any;
+    beforeEach(() => {
+      setPropertiesForFilesDeployMock = jest.fn();
+      getInstanceSpy = jest
+        .spyOn(PersistentStorageService, 'getInstance')
+        .mockReturnValue({
+          setPropertiesForFilesDeploy: setPropertiesForFilesDeployMock
+        } as any);
+      getPackageDirectoryPathsSpy = jest
+        .spyOn(SfdxPackageDirectories, 'getPackageDirectoryPaths')
+        .mockResolvedValue('path/to/foo' as any);
+      createOutputSpy = jest
+        .spyOn(TestDeployExecutor.prototype as any, 'createOutput')
+        .mockReturnValue('path/to/foo' as any);
+      appendLineSpy = jest
+        .spyOn(channelService, 'appendLine')
+        .mockImplementation(jest.fn());
+      mockUnlock = jest.fn();
+      unlockSpy = jest
+        .spyOn(DeployQueue, 'get')
+        .mockReturnValue({ unlock: mockUnlock } as any);
+      DeployRetrieveExecutor.errorCollection = MockErrorCollection as any;
+    });
+
+    it('should clear errors on success', async () => {
+      // Arrange
+      const mockDeployResult = {
+        response: {
+          status: 'Succeeded'
+        }
+      };
+      const deployRetrieveExecutorClearSpy = jest.spyOn(
+        DeployRetrieveExecutor.errorCollection,
+        'clear'
+      );
+      SfdxCommandletExecutor.errorCollection = MockErrorCollection as any;
+      const sfdxCommandletExecutorClearSpy = jest.spyOn(
+        SfdxCommandletExecutor.errorCollection,
+        'clear'
+      );
+
+      const executor = new TestDeployExecutor(
+        'testDeploy',
+        'force_source_deploy_with_sourcepath_beta'
+      );
+
+      // Act
+      await (executor as any).postOperation(mockDeployResult);
+
+      // Assert
+      expect(getInstanceSpy).toHaveBeenCalled();
+      expect(getPackageDirectoryPathsSpy).toHaveBeenCalled();
+      expect(createOutputSpy).toHaveBeenCalled();
+      expect(appendLineSpy).toHaveBeenCalled();
+      expect(setPropertiesForFilesDeployMock).toHaveBeenCalledWith(
+        mockDeployResult
+      );
+      expect(deployRetrieveExecutorClearSpy).toHaveBeenCalled();
+      expect(sfdxCommandletExecutorClearSpy).toHaveBeenCalled();
+      expect(unlockSpy).toHaveBeenCalled();
+      expect(mockUnlock).toHaveBeenCalled();
+    });
+
+    it('should create diagnostics on failure', async () => {
+      // Arrange
+      const mockDeployResult = {
+        response: {
+          status: 'Failed'
+        }
+      };
+      const unsuccessfulOperationHandlerSpy = jest
+        .spyOn(
+          TestDeployExecutor.prototype as any,
+          'unsuccessfulOperationHandler'
+        )
+        .mockImplementation(jest.fn());
+      const executor = new TestDeployExecutor(
+        'testDeploy',
+        'force_source_deploy_with_sourcepath_beta'
+      );
+
+      // Act
+      await (executor as any).postOperation(mockDeployResult);
+
+      // Asserts
+      expect(getInstanceSpy).toHaveBeenCalled();
+      expect(getPackageDirectoryPathsSpy).toHaveBeenCalled();
+      expect(createOutputSpy).toHaveBeenCalled();
+      expect(appendLineSpy).toHaveBeenCalled();
+      expect(setPropertiesForFilesDeployMock).toHaveBeenCalledWith(
+        mockDeployResult
+      );
+      expect(unsuccessfulOperationHandlerSpy).toHaveBeenCalledWith(
+        mockDeployResult,
+        DeployRetrieveExecutor.errorCollection
+      );
+      expect(unlockSpy).toHaveBeenCalled();
+      expect(mockUnlock).toHaveBeenCalled();
+    });
+  });
 });

packages/salesforcedx-vscode-core/test/jest/commands/forceSourcePush.test.ts

@@ -8,13 +8,22 @@
 import { ChannelService } from '@salesforce/salesforcedx-utils-vscode';
 import { nls } from '@salesforce/salesforcedx-utils-vscode/src/messages';
 import { ForceSourcePushExecutor } from '../../../src/commands';
+import { DeployRetrieveExecutor } from '../../../src/commands/baseDeployRetrieve';
 import { DeployType } from '../../../src/commands/forceSourcePush';
-import { CommandParams } from '../../../src/commands/util';
+import {
+  CommandParams,
+  SfdxCommandletExecutor
+} from '../../../src/commands/util';
 import { PersistentStorageService } from '../../../src/conflict';
 import { dummyPushResult, dummyStdOut } from './data/testData';
 
 describe('ForceSourcePushExecutor', () => {
   describe('exitProcessHandlerPush', () => {
+    class MockErrorCollection {
+      public static clear(): void {
+        jest.fn();
+      }
+    }
     beforeEach(() => {
       jest.spyOn(ChannelService, 'getInstance').mockReturnValue({} as any);
       jest.spyOn(nls, 'localize').mockReturnValue('');
@@ -38,7 +47,16 @@ describe('ForceSourcePushExecutor', () => {
       const executor = new ForceSourcePushExecutor(flag, pushCommand);
       const updateCacheMock = jest.fn();
       const executorAsAny = executor as any;
-      executorAsAny.errorCollection = { clear: jest.fn() };
+      SfdxCommandletExecutor.errorCollection = MockErrorCollection as any;
+      DeployRetrieveExecutor.errorCollection = MockErrorCollection as any;
+      const deployRetrieveExecutorClearSpy = jest.spyOn(
+        DeployRetrieveExecutor.errorCollection,
+        'clear'
+      );
+      const sfdxCommandletExecutorClearSpy = jest.spyOn(
+        SfdxCommandletExecutor.errorCollection,
+        'clear'
+      );
       executorAsAny.updateCache = updateCacheMock;
       executorAsAny.getDeployType = jest.fn().mockReturnValue(DeployType.Push);
       executorAsAny.logMetric = jest.fn();
@@ -56,6 +74,8 @@ describe('ForceSourcePushExecutor', () => {
 
       // Assert
       expect(updateCacheMock).toHaveBeenCalled();
+      expect(sfdxCommandletExecutorClearSpy).toHaveBeenCalled();
+      expect(deployRetrieveExecutorClearSpy).toHaveBeenCalled();
     });
   });