Skip to content

Commit 4b9db6a

Browse files
committed
Make query history tests work with remote / variant analysis queries
We're adding both remote query history items and variant analysis history items to the query history. We've introduced a little method to shuffle the query history list before we run our tests so that we don't accidentally write tests that depend on a fixed order. The query history now has increased test coverage for: - handling an item being clicked - removing and selecting the next item in query history - handling single / multi selection - showing the item results While we're here we're also: 1. Adding a factory to generate variant analysis history items 2. Providing all fields for remote query history items and ordering them according to their type definition order. At least one field (`queryId`) was missing from our factory, which we will need to make the tests work with remote queries.
1 parent 6289411 commit 4b9db6a

4 files changed

Lines changed: 132 additions & 22 deletions

File tree

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,39 @@
11
import { RemoteQueryHistoryItem } from '../../../remote-queries/remote-query-history-item';
2+
import { QueryStatus } from '../../../query-status';
23

34
export function createMockRemoteQueryHistoryItem({
45
date = new Date('2022-01-01T00:00:00.000Z'),
6+
failureReason = undefined,
57
resultCount = 16,
6-
userSpecifiedLabel = undefined,
78
repositoryCount = 0,
9+
userSpecifiedLabel = undefined,
810
}: {
911
date?: Date;
12+
failureReason?: string;
1013
resultCount?: number;
11-
userSpecifiedLabel?: string;
1214
repositoryCount?: number;
15+
userSpecifiedLabel?: string;
1316
}): RemoteQueryHistoryItem {
1417
return ({
1518
t: 'remote',
16-
userSpecifiedLabel,
19+
failureReason,
20+
resultCount,
21+
status: QueryStatus.InProgress,
22+
completed: false,
23+
queryId: 'queryId',
1724
remoteQuery: {
18-
executionStartTime: date.getTime(),
1925
queryName: 'query-name',
2026
queryFilePath: 'query-file.ql',
27+
queryText: 'select 1',
28+
language: 'javascript',
2129
controllerRepository: {
2230
owner: 'github',
2331
name: 'vscode-codeql-integration-tests',
2432
},
25-
language: 'javascript',
33+
executionStartTime: date.getTime(),
34+
actionsWorkflowRunId: 1,
2635
repositoryCount,
2736
},
28-
status: 'in progress',
29-
resultCount,
37+
userSpecifiedLabel,
3038
} as unknown) as RemoteQueryHistoryItem;
3139
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { faker } from '@faker-js/faker';
2+
import { VariantAnalysisHistoryItem } from '../../../remote-queries/variant-analysis-history-item';
3+
import { QueryStatus } from '../../../query-status';
4+
import { VariantAnalysisStatus } from '../../../remote-queries/shared/variant-analysis';
5+
6+
export function createMockVariantAnalysisHistoryItem(
7+
historyItemStatus: QueryStatus = QueryStatus.InProgress,
8+
variantAnalysisStatus: VariantAnalysisStatus = VariantAnalysisStatus.Succeeded,
9+
failureReason?: string,
10+
resultCount?: number,
11+
userSpecifiedLabel?: string
12+
): VariantAnalysisHistoryItem {
13+
return ({
14+
t: 'variant-analysis',
15+
failureReason,
16+
resultCount,
17+
status: historyItemStatus,
18+
completed: false,
19+
variantAnalysis: {
20+
'id': faker.datatype.number(),
21+
'controllerRepoId': faker.datatype.number(),
22+
'query': {
23+
'name': 'Variant Analysis Query History Item',
24+
'filePath': 'PLACEHOLDER/q2.ql',
25+
'language': 'ruby',
26+
'text': '/**\n * @name Variant Analysis Query History Item\n * @kind problem\n * @problem.severity warning\n * @id ruby/example/empty-block\n */\nimport ruby\n\nfrom Block b\nwhere b.getNumberOfStatements() = 0\nselect b, \'This is an empty block.\'\n'
27+
},
28+
'databases': {
29+
'repositories': ['92384123', '1230871']
30+
},
31+
'createdAt': faker.date.recent().toISOString(),
32+
'updatedAt': faker.date.recent().toISOString(),
33+
'executionStartTime': faker.date.recent().toISOString(),
34+
'status': variantAnalysisStatus,
35+
'actionsWorkflowRunId': faker.datatype.number()
36+
},
37+
userSpecifiedLabel,
38+
} as unknown) as VariantAnalysisHistoryItem;
39+
}
40+

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

Lines changed: 72 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ import { QueryRunner } from '../../queryRunner';
2222
import { VariantAnalysisManager } from '../../remote-queries/variant-analysis-manager';
2323
import { QueryHistoryInfo } from '../../query-history-info';
2424
import { createMockLocalQuery, createMockQueryWithResults } from '../factories/local-queries/local-query-history-item';
25+
import { createMockRemoteQueryHistoryItem } from '../factories/remote-queries/remote-query-history-item';
26+
import { RemoteQueryHistoryItem } from '../../remote-queries/remote-query-history-item';
27+
import { shuffleHistoryItems } from '../utils/query-history-helpers';
28+
import { createMockVariantAnalysisHistoryItem } from '../factories/remote-queries/variant-analysis-history-item';
29+
import { VariantAnalysisHistoryItem } from '../../remote-queries/variant-analysis-history-item';
2530

2631
describe('query-history', () => {
2732
const mockExtensionLocation = path.join(tmpDir.name, 'mock-extension-location');
@@ -63,13 +68,17 @@ describe('query-history', () => {
6368
remoteQueriesManagerStub = {
6469
onRemoteQueryAdded: sandbox.stub(),
6570
onRemoteQueryRemoved: sandbox.stub(),
66-
onRemoteQueryStatusUpdate: sandbox.stub()
71+
onRemoteQueryStatusUpdate: sandbox.stub(),
72+
removeRemoteQuery: sandbox.stub(),
73+
openRemoteQueryResults: sandbox.stub(),
6774
} as any as RemoteQueriesManager;
6875

6976
variantAnalysisManagerStub = {
7077
onVariantAnalysisAdded: sandbox.stub(),
7178
onVariantAnalysisStatusUpdated: sandbox.stub(),
72-
onVariantAnalysisRemoved: sandbox.stub()
79+
onVariantAnalysisRemoved: sandbox.stub(),
80+
removeVariantAnalysis: sandbox.stub(),
81+
showView: sandbox.stub(),
7382
} as any as VariantAnalysisManager;
7483
});
7584

@@ -124,6 +133,8 @@ describe('query-history', () => {
124133

125134
let allHistory: QueryHistoryInfo[];
126135
let localQueryHistory: LocalQueryInfo[];
136+
let remoteQueryHistory: RemoteQueryHistoryItem[];
137+
let variantAnalysisHistory: VariantAnalysisHistoryItem[];
127138

128139
beforeEach(() => {
129140
localQueryHistory = [
@@ -132,7 +143,19 @@ describe('query-history', () => {
132143
createMockLocalQuery('a', createMockQueryWithResults(sandbox, false)),
133144
createMockLocalQuery('a', createMockQueryWithResults(sandbox, true)),
134145
];
135-
allHistory = [...localQueryHistory];
146+
remoteQueryHistory = [
147+
createMockRemoteQueryHistoryItem({}),
148+
createMockRemoteQueryHistoryItem({}),
149+
createMockRemoteQueryHistoryItem({}),
150+
createMockRemoteQueryHistoryItem({})
151+
];
152+
variantAnalysisHistory = [
153+
createMockVariantAnalysisHistoryItem(),
154+
createMockVariantAnalysisHistoryItem(),
155+
createMockVariantAnalysisHistoryItem(),
156+
createMockVariantAnalysisHistoryItem()
157+
];
158+
allHistory = shuffleHistoryItems([...localQueryHistory, ...remoteQueryHistory, ...variantAnalysisHistory]);
136159
});
137160

138161
describe('Local Queries', () => {
@@ -264,19 +287,30 @@ describe('query-history', () => {
264287
describe('handleItemClicked', () => {
265288
it('should call the selectedCallback when an item is clicked', async () => {
266289
queryHistoryManager = await createMockQueryHistory(allHistory);
290+
const itemClicked = allHistory[0];
291+
await queryHistoryManager.handleItemClicked(itemClicked, [itemClicked]);
292+
293+
if (itemClicked.t == 'local') {
294+
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(itemClicked);
295+
} else if (itemClicked.t == 'remote') {
296+
expect(remoteQueriesManagerStub.openRemoteQueryResults).to.have.been.calledOnceWith(itemClicked.queryId);
297+
} else if (itemClicked.t == 'variant-analysis') {
298+
expect(variantAnalysisManagerStub.showView).to.have.been.calledOnceWith(itemClicked.variantAnalysis.id);
299+
}
267300

268-
await queryHistoryManager.handleItemClicked(allHistory[0], [allHistory[0]]);
269-
270-
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(allHistory[0]);
271-
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(allHistory[0]);
301+
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(itemClicked);
272302
});
273303

274304
it('should do nothing if there is a multi-selection', async () => {
275305
queryHistoryManager = await createMockQueryHistory(allHistory);
306+
const itemClicked = allHistory[0];
307+
const secondItemClicked = allHistory[1];
276308

277-
await queryHistoryManager.handleItemClicked(allHistory[0], [allHistory[0], allHistory[1]]);
309+
await queryHistoryManager.handleItemClicked(itemClicked, [itemClicked, secondItemClicked]);
278310

279311
expect(localQueriesResultsViewStub.showResults).not.to.have.been.called;
312+
expect(remoteQueriesManagerStub.openRemoteQueryResults).not.to.have.been.called;
313+
expect(variantAnalysisManagerStub.showView).not.to.have.been.called;
280314
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.be.undefined;
281315
});
282316

@@ -286,6 +320,8 @@ describe('query-history', () => {
286320
await queryHistoryManager.handleItemClicked(undefined!, []);
287321

288322
expect(localQueriesResultsViewStub.showResults).not.to.have.been.called;
323+
expect(remoteQueriesManagerStub.openRemoteQueryResults).not.to.have.been.called;
324+
expect(variantAnalysisManagerStub.showView).not.to.have.been.called;
289325
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.be.undefined;
290326
});
291327
});
@@ -311,12 +347,23 @@ describe('query-history', () => {
311347

312348
if (toDelete.t == 'local') {
313349
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
350+
} else if (toDelete.t == 'remote') {
351+
expect(remoteQueriesManagerStub.removeRemoteQuery).to.have.been.calledOnceWith((toDelete as RemoteQueryHistoryItem).queryId);
352+
} else if (toDelete.t == 'variant-analysis') {
353+
expect(variantAnalysisManagerStub.removeVariantAnalysis).to.have.been.calledOnceWith((toDelete as VariantAnalysisHistoryItem).variantAnalysis.id);
314354
}
315-
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.deep.eq(selected);
316-
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
317355

318356
// the same item should be selected
319-
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(selected);
357+
if (selected.t == 'local') {
358+
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(selected);
359+
} else if (toDelete.t == 'remote') {
360+
expect(remoteQueriesManagerStub.openRemoteQueryResults).to.have.been.calledOnceWith((selected as RemoteQueryHistoryItem).queryId);
361+
} else if (toDelete.t == 'variant-analysis') {
362+
expect(variantAnalysisManagerStub.showView).to.have.been.calledOnceWith((selected as VariantAnalysisHistoryItem).variantAnalysis.id);
363+
}
364+
365+
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.deep.eq(selected);
366+
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
320367
});
321368

322369
it('should remove an item and select a new one', async () => {
@@ -333,12 +380,23 @@ describe('query-history', () => {
333380

334381
if (toDelete.t == 'local') {
335382
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
383+
} else if (toDelete.t == 'remote') {
384+
expect(remoteQueriesManagerStub.removeRemoteQuery).to.have.been.calledOnceWith((toDelete as RemoteQueryHistoryItem).queryId);
385+
} else if (toDelete.t == 'variant-analysis') {
386+
expect(variantAnalysisManagerStub.removeVariantAnalysis).to.have.been.calledOnceWith((toDelete as VariantAnalysisHistoryItem).variantAnalysis.id);
336387
}
337-
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(newSelected);
338-
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
339388

340389
// the current item should have been selected
341-
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(newSelected);
390+
if (newSelected.t == 'local') {
391+
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(newSelected);
392+
} else if (toDelete.t == 'remote') {
393+
expect(remoteQueriesManagerStub.openRemoteQueryResults).to.have.been.calledOnceWith((newSelected as RemoteQueryHistoryItem).queryId);
394+
} else if (toDelete.t == 'variant-analysis') {
395+
expect(variantAnalysisManagerStub.showView).to.have.been.calledOnceWith((newSelected as VariantAnalysisHistoryItem).variantAnalysis.id);
396+
}
397+
398+
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(newSelected);
399+
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
342400
});
343401

344402
describe('HistoryTreeDataProvider', () => {
@@ -355,7 +413,6 @@ describe('query-history', () => {
355413
historyTreeDataProvider.dispose();
356414
});
357415

358-
359416
it('should get a tree item with raw results', async () => {
360417
const mockQuery = createMockLocalQuery('a', createMockQueryWithResults(sandbox, true, /* raw results */ false));
361418
const treeItem = await historyTreeDataProvider.getTreeItem(mockQuery);
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { QueryHistoryInfo } from '../../query-history-info';
2+
3+
export function shuffleHistoryItems(history: QueryHistoryInfo[]) {
4+
return history.sort(() => Math.random() - 0.5);
5+
}

0 commit comments

Comments
 (0)