Add cancelling of variant analysis to view

This implements the "Stop query" button on the view. It moves some of
the logic of actually cancelling the variant analysis to the manager
instead of being in the query history to allow better re-use of the
code.
This commit is contained in:
Koen Vlaswinkel
2022-11-07 11:21:24 +01:00
parent 93054e14a2
commit 09bae13732
10 changed files with 134 additions and 25 deletions

View File

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

View File

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

View File

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

View File

@@ -22,8 +22,9 @@ import { VariantAnalysisResultsManager } from './variant-analysis-results-manage
import { getControllerRepo } from './run-remote-query'; import { getControllerRepo } from './run-remote-query';
import { processUpdatedVariantAnalysis, processVariantAnalysisRepositoryTask } from './variant-analysis-processor'; import { processUpdatedVariantAnalysis, processVariantAnalysisRepositoryTask } from './variant-analysis-processor';
import PQueue from 'p-queue'; import PQueue from 'p-queue';
import { createTimestampFile, showAndLogErrorMessage } from '../helpers'; import { createTimestampFile, showAndLogErrorMessage, showAndLogInformationMessage } from '../helpers';
import * as fs from 'fs-extra'; import * as fs from 'fs-extra';
import { cancelVariantAnalysis } from './gh-api/gh-actions-api-client';
export class VariantAnalysisManager extends DisposableObject implements VariantAnalysisViewManager<VariantAnalysisView> { export class VariantAnalysisManager extends DisposableObject implements VariantAnalysisViewManager<VariantAnalysisView> {
private static readonly REPO_STATES_FILENAME = 'repo_states.json'; 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 ${variantAnalysis.query.name}`);
}
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 { private getRepoStatesStoragePath(variantAnalysisId: number): string {
return path.join( return path.join(
this.getVariantAnalysisStorageLocation(variantAnalysisId), this.getVariantAnalysisStorageLocation(variantAnalysisId),

View File

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

View File

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

View File

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

View File

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

View File

@@ -5,6 +5,7 @@ import { CodeQLExtensionInterface } from '../../../extension';
import { logger } from '../../../logging'; import { logger } from '../../../logging';
import * as config from '../../../config'; import * as config from '../../../config';
import * as ghApiClient from '../../../remote-queries/gh-api/gh-api-client'; 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 { Credentials } from '../../../authentication';
import * as fs from 'fs-extra'; import * as fs from 'fs-extra';
import * as path from 'path'; 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 a-query-name');
}
});
it('should return cancel if valid', async () => {
await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id);
expect(mockCancelVariantAnalysis).to.have.been.calledWith(mockCredentials, variantAnalysis);
});
});
});
}); });

View File

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