Merge pull request #1720 from github/koesie10/stop-query-button

Add cancelling of variant analysis to view

Author: koesie10

extensions/ql-vscode/src/extension.ts

@@ -953,6 +953,14 @@ async function activateWithInstalledDistribution(
     })
   );
 
+  ctx.subscriptions.push(
+    commandRunner('codeQL.cancelVariantAnalysis', async (
+      variantAnalysisId: number,
+    ) => {
+      await variantAnalysisManager.cancelVariantAnalysis(variantAnalysisId);
+    })
+  );
+
   ctx.subscriptions.push(
     commandRunner('codeQL.openVariantAnalysis', async () => {
       await variantAnalysisManager.promptOpenVariantAnalysis();

extensions/ql-vscode/src/pure/interface-types.ts

@@ -445,11 +445,6 @@ export interface SetVariantAnalysisMessage {
   variantAnalysis: VariantAnalysis;
 }
 
-export type StopVariantAnalysisMessage = {
-  t: 'stopVariantAnalysis';
-  variantAnalysisId: number;
-}
-
 export type VariantAnalysisState = {
   variantAnalysisId: number;
 }
@@ -481,15 +476,19 @@ export interface OpenLogsMessage {
   t: 'openLogs';
 }
 
+export interface CancelVariantAnalysisMessage {
+  t: 'cancelVariantAnalysis';
+}
+
 export type ToVariantAnalysisMessage =
   | SetVariantAnalysisMessage
   | SetRepoResultsMessage
   | SetRepoStatesMessage;
 
 export type FromVariantAnalysisMessage =
   | ViewLoadedMsg
-  | StopVariantAnalysisMessage
   | RequestRepositoryResultsMessage
   | OpenQueryFileMessage
   | OpenQueryTextMessage
-  | OpenLogsMessage;
+  | OpenLogsMessage
+  | CancelVariantAnalysisMessage;

extensions/ql-vscode/src/query-history.ts

@@ -40,7 +40,7 @@ import * as fs from 'fs-extra';
 import { CliVersionConstraint } from './cli';
 import { HistoryItemLabelProvider } from './history-item-label-provider';
 import { Credentials } from './authentication';
-import { cancelRemoteQuery, cancelVariantAnalysis } from './remote-queries/gh-api/gh-actions-api-client';
+import { cancelRemoteQuery } from './remote-queries/gh-api/gh-actions-api-client';
 import { RemoteQueriesManager } from './remote-queries/remote-queries-manager';
 import { RemoteQueryHistoryItem } from './remote-queries/remote-query-history-item';
 import { ResultsView } from './interface';
@@ -1119,9 +1119,7 @@ export class QueryHistoryManager extends DisposableObject {
           const credentials = await this.getCredentials();
           await cancelRemoteQuery(credentials, item.remoteQuery);
         } else if (item.t === 'variant-analysis') {
-          void showAndLogInformationMessage('Cancelling variant analysis. This may take a while.');
-          const credentials = await this.getCredentials();
-          await cancelVariantAnalysis(credentials, item.variantAnalysis);
+          await commands.executeCommand('codeQL.cancelVariantAnalysis', item.variantAnalysis.id);
         }
       }
     });

extensions/ql-vscode/src/remote-queries/variant-analysis-manager.ts

@@ -22,8 +22,9 @@ import { VariantAnalysisResultsManager } from './variant-analysis-results-manage
 import { getControllerRepo } from './run-remote-query';
 import { processUpdatedVariantAnalysis, processVariantAnalysisRepositoryTask } from './variant-analysis-processor';
 import PQueue from 'p-queue';
-import { createTimestampFile, showAndLogErrorMessage } from '../helpers';
+import { createTimestampFile, showAndLogErrorMessage, showAndLogInformationMessage } from '../helpers';
 import * as fs from 'fs-extra';
+import { cancelVariantAnalysis } from './gh-api/gh-actions-api-client';
 
 export class VariantAnalysisManager extends DisposableObject implements VariantAnalysisViewManager<VariantAnalysisView> {
   private static readonly REPO_STATES_FILENAME = 'repo_states.json';
@@ -281,6 +282,25 @@ export class VariantAnalysisManager extends DisposableObject implements VariantA
     );
   }
 
+  public async cancelVariantAnalysis(variantAnalysisId: number) {
+    const variantAnalysis = this.variantAnalyses.get(variantAnalysisId);
+    if (!variantAnalysis) {
+      throw new Error(`No variant analysis with id: ${variantAnalysisId}`);
+    }
+
+    if (!variantAnalysis.actionsWorkflowRunId) {
+      throw new Error(`No workflow run id for variant analysis with id: ${variantAnalysis.id}`);
+    }
+
+    const credentials = await Credentials.initialize(this.ctx);
+    if (!credentials) {
+      throw Error('Error authenticating with GitHub');
+    }
+
+    void showAndLogInformationMessage('Cancelling variant analysis. This may take a while.');
+    await cancelVariantAnalysis(credentials, variantAnalysis);
+  }
+
   private getRepoStatesStoragePath(variantAnalysisId: number): string {
     return path.join(
       this.getVariantAnalysisStorageLocation(variantAnalysisId),

extensions/ql-vscode/src/remote-queries/variant-analysis-view.ts

@@ -91,8 +91,8 @@ export class VariantAnalysisView extends AbstractWebview<ToVariantAnalysisMessag
         await this.onWebViewLoaded();
 
         break;
-      case 'stopVariantAnalysis':
-        void logger.log(`Stop variant analysis: ${msg.variantAnalysisId}`);
+      case 'cancelVariantAnalysis':
+        void commands.executeCommand('codeQL.cancelVariantAnalysis', this.variantAnalysisId);
         break;
       case 'requestRepositoryResults':
         void commands.executeCommand('codeQL.loadVariantAnalysisRepoResults', this.variantAnalysisId, msg.repositoryFullName);

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx

@@ -30,6 +30,12 @@ const openQueryText = () => {
   });
 };
 
+const stopQuery = () => {
+  vscode.postMessage({
+    t: 'cancelVariantAnalysis',
+  });
+};
+
 const openLogs = () => {
   vscode.postMessage({
     t: 'openLogs',
@@ -88,7 +94,7 @@ export function VariantAnalysis({
         variantAnalysis={variantAnalysis}
         onOpenQueryFileClick={openQueryFile}
         onViewQueryTextClick={openQueryText}
-        onStopQueryClick={() => console.log('Stop query')}
+        onStopQueryClick={stopQuery}
         onCopyRepositoryListClick={() => console.log('Copy repository list')}
         onExportResultsClick={() => console.log('Export results')}
         onViewLogsClick={openLogs}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisActions.tsx

@@ -7,6 +7,7 @@ type Props = {
   variantAnalysisStatus: VariantAnalysisStatus;
 
   onStopQueryClick: () => void;
+  stopQueryDisabled?: boolean;
 
   onCopyRepositoryListClick: () => void;
   onExportResultsClick: () => void;
@@ -26,12 +27,13 @@ export const VariantAnalysisActions = ({
   variantAnalysisStatus,
   onStopQueryClick,
   onCopyRepositoryListClick,
-  onExportResultsClick
+  onExportResultsClick,
+  stopQueryDisabled,
 }: Props) => {
   return (
     <Container>
       {variantAnalysisStatus === VariantAnalysisStatus.InProgress && (
-        <Button appearance="secondary" onClick={onStopQueryClick}>
+        <Button appearance="secondary" onClick={onStopQueryClick} disabled={stopQueryDisabled}>
           Stop query
         </Button>
       )}

extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisHeader.tsx

@@ -73,6 +73,7 @@ export const VariantAnalysisHeader = ({
           onStopQueryClick={onStopQueryClick}
           onCopyRepositoryListClick={onCopyRepositoryListClick}
           onExportResultsClick={onExportResultsClick}
+          stopQueryDisabled={!variantAnalysis.actionsWorkflowRunId}
         />
       </Row>
       <VariantAnalysisStats

extensions/ql-vscode/src/vscode-tests/cli-integration/remote-queries/variant-analysis-manager.test.ts

@@ -5,6 +5,7 @@ import { CodeQLExtensionInterface } from '../../../extension';
 import { logger } from '../../../logging';
 import * as config from '../../../config';
 import * as ghApiClient from '../../../remote-queries/gh-api/gh-api-client';
+import * as ghActionsApiClient from '../../../remote-queries/gh-api/gh-actions-api-client';
 import { Credentials } from '../../../authentication';
 import * as fs from 'fs-extra';
 import * as path from 'path';
@@ -508,4 +509,80 @@ describe('Variant Analysis Manager', async function() {
       });
     });
   });
+
+  describe('cancelVariantAnalysis', async () => {
+    let variantAnalysis: VariantAnalysis;
+    let mockCancelVariantAnalysis: sinon.SinonStub;
+    let getOctokitStub: sinon.SinonStub;
+
+    let variantAnalysisStorageLocation: string;
+
+    beforeEach(async () => {
+      variantAnalysis = createMockVariantAnalysis({});
+
+      mockCancelVariantAnalysis = sandbox.stub(ghActionsApiClient, 'cancelVariantAnalysis');
+
+      variantAnalysisStorageLocation = variantAnalysisManager.getVariantAnalysisStorageLocation(variantAnalysis.id);
+      await createTimestampFile(variantAnalysisStorageLocation);
+      await variantAnalysisManager.rehydrateVariantAnalysis(variantAnalysis);
+    });
+
+    afterEach(() => {
+      fs.rmSync(variantAnalysisStorageLocation, { recursive: true });
+    });
+
+    describe('when the credentials are invalid', () => {
+      beforeEach(async () => {
+        sandbox.stub(Credentials, 'initialize').resolves(undefined);
+      });
+
+      it('should return early', async () => {
+        try {
+          await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
+        } catch (error: any) {
+          expect(error.message).to.equal('Error authenticating with GitHub');
+        }
+      });
+    });
+
+    describe('when the credentials are valid', () => {
+      let mockCredentials: Credentials;
+
+      beforeEach(async () => {
+        mockCredentials = {
+          getOctokit: () => Promise.resolve({
+            request: getOctokitStub
+          })
+        } as unknown as Credentials;
+        sandbox.stub(Credentials, 'initialize').resolves(mockCredentials);
+      });
+
+      it('should return early if the variant analysis is not found', async () => {
+        try {
+          await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id + 100);
+        } catch (error: any) {
+          expect(error.message).to.equal('No variant analysis with id: ' + (variantAnalysis.id + 100));
+        }
+      });
+
+      it('should return early if the variant analysis does not have an actions workflow run id', async () => {
+        await variantAnalysisManager.onVariantAnalysisUpdated({
+          ...variantAnalysis,
+          actionsWorkflowRunId: undefined,
+        });
+
+        try {
+          await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
+        } catch (error: any) {
+          expect(error.message).to.equal(`No workflow run id for variant analysis with id: ${variantAnalysis.id}`);
+        }
+      });
+
+      it('should return cancel if valid', async () => {
+        await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
+
+        expect(mockCancelVariantAnalysis).to.have.been.calledWith(mockCredentials, variantAnalysis);
+      });
+    });
+  });
 });

extensions/ql-vscode/src/vscode-tests/no-workspace/query-history.test.ts

@@ -338,7 +338,6 @@ describe('query-history', () => {
     describe('handleCancel', () => {
       let mockCredentials: Credentials;
       let mockCancelRemoteQuery: sinon.SinonStub;
-      let mockCancelVariantAnalysis: sinon.SinonStub;
       let getOctokitStub: sinon.SinonStub;
 
       beforeEach(async () => {
@@ -349,7 +348,6 @@ describe('query-history', () => {
         } as unknown as Credentials;
         sandbox.stub(Credentials, 'initialize').resolves(mockCredentials);
         mockCancelRemoteQuery = sandbox.stub(ghActionsApiClient, 'cancelRemoteQuery');
-        mockCancelVariantAnalysis = sandbox.stub(ghActionsApiClient, 'cancelVariantAnalysis');
       });
 
       describe('if the item is in progress', async () => {
@@ -408,7 +406,7 @@ describe('query-history', () => {
           const inProgress1 = variantAnalysisHistory[1];
 
           await queryHistoryManager.handleCancel(inProgress1, [inProgress1]);
-          expect(mockCancelVariantAnalysis).to.have.been.calledWith(mockCredentials, inProgress1.variantAnalysis);
+          expect(executeCommandSpy).to.have.been.calledWith('codeQL.cancelVariantAnalysis', inProgress1.variantAnalysis.id);
         });
 
         it('should cancel multiple variant analyses', async () => {
@@ -419,8 +417,8 @@ describe('query-history', () => {
           const inProgress2 = variantAnalysisHistory[3];
 
           await queryHistoryManager.handleCancel(inProgress1, [inProgress1, inProgress2]);
-          expect(mockCancelVariantAnalysis).to.have.been.calledWith(mockCredentials, inProgress1.variantAnalysis);
-          expect(mockCancelVariantAnalysis).to.have.been.calledWith(mockCredentials, inProgress2.variantAnalysis);
+          expect(executeCommandSpy).to.have.been.calledWith('codeQL.cancelVariantAnalysis', inProgress1.variantAnalysis.id);
+          expect(executeCommandSpy).to.have.been.calledWith('codeQL.cancelVariantAnalysis', inProgress2.variantAnalysis.id);
         });
       });
 
@@ -480,7 +478,7 @@ describe('query-history', () => {
           const completedVariantAnalysis = variantAnalysisHistory[0];
 
           await queryHistoryManager.handleCancel(completedVariantAnalysis, [completedVariantAnalysis]);
-          expect(mockCancelVariantAnalysis).to.not.have.been.calledWith(mockCredentials, completedVariantAnalysis.variantAnalysis);
+          expect(executeCommandSpy).to.not.have.been.calledWith('codeQL.cancelVariantAnalysis', completedVariantAnalysis.variantAnalysis);
         });
 
         it('should not cancel multiple variant analyses', async () => {
@@ -491,8 +489,8 @@ describe('query-history', () => {
           const failedVariantAnalysis = variantAnalysisHistory[2];
 
           await queryHistoryManager.handleCancel(completedVariantAnalysis, [completedVariantAnalysis, failedVariantAnalysis]);
-          expect(mockCancelVariantAnalysis).to.not.have.been.calledWith(mockCredentials, completedVariantAnalysis.variantAnalysis);
-          expect(mockCancelVariantAnalysis).to.not.have.been.calledWith(mockCredentials, failedVariantAnalysis.variantAnalysis);
+          expect(executeCommandSpy).to.not.have.been.calledWith('codeQL.cancelVariantAnalysis', completedVariantAnalysis.variantAnalysis.id);
+          expect(executeCommandSpy).to.not.have.been.calledWith('codeQL.cancelVariantAnalysis', failedVariantAnalysis.variantAnalysis.id);
         });
       });
     });