Skip to content

Commit 5a40159

Browse files
committed
Only test removal for local queries for now
Paired with @robertbrignull on debugging why having all types of query history items isn't playing nicely when we try to remove an item. We've tracked down the issue it the handleRemoveHistoryItem method not correctly setting the `current` item after a deletion. However, it's unclear whether the test setup is to blame or this is a real bug. I'm going to leave the tests for `handleRemoveHistoryItem` to test just local queries for now (as they were originally) and will come back to this in a different PR.
1 parent a1daa91 commit 5a40159

File tree

1 file changed

+15
-41
lines changed

1 file changed

+15
-41
lines changed

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

Lines changed: 15 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -263,14 +263,14 @@ describe('query-history', () => {
263263

264264
describe('handleRemoveHistoryItem', () => {
265265
it('should remove an item and not select a new one', async () => {
266-
queryHistoryManager = await createMockQueryHistory(allHistory);
266+
queryHistoryManager = await createMockQueryHistory(localQueryHistory);
267267
// initialize the selection
268-
await queryHistoryManager.treeView.reveal(allHistory[0], { select: true });
268+
await queryHistoryManager.treeView.reveal(localQueryHistory[0], { select: true });
269269

270270
// deleting the first item when a different item is selected
271271
// will not change the selection
272-
const toDelete = allHistory[1];
273-
const selected = allHistory[3];
272+
const toDelete = localQueryHistory[1];
273+
const selected = localQueryHistory[3];
274274

275275
// select the item we want
276276
await queryHistoryManager.treeView.reveal(selected, { select: true });
@@ -281,58 +281,32 @@ describe('query-history', () => {
281281
// remove an item
282282
await queryHistoryManager.handleRemoveHistoryItem(toDelete, [toDelete]);
283283

284-
if (toDelete.t == 'local') {
285-
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
286-
} else if (toDelete.t == 'remote') {
287-
expect(remoteQueriesManagerStub.removeRemoteQuery).to.have.been.calledOnceWith((toDelete as RemoteQueryHistoryItem).queryId);
288-
} else if (toDelete.t == 'variant-analysis') {
289-
expect(variantAnalysisManagerStub.removeVariantAnalysis).to.have.been.calledOnceWith((toDelete as VariantAnalysisHistoryItem).variantAnalysis.id);
290-
}
291-
292-
// the same item should be selected
293-
if (selected.t == 'local') {
294-
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(selected);
295-
} else if (toDelete.t == 'remote') {
296-
expect(remoteQueriesManagerStub.openRemoteQueryResults).to.have.been.calledOnceWith((selected as RemoteQueryHistoryItem).queryId);
297-
} else if (toDelete.t == 'variant-analysis') {
298-
expect(variantAnalysisManagerStub.showView).to.have.been.calledOnceWith((selected as VariantAnalysisHistoryItem).variantAnalysis.id);
299-
}
300-
284+
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
301285
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.deep.eq(selected);
302286
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
287+
288+
// the same item should be selected
289+
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(selected);
303290
});
304291

305292
it('should remove an item and select a new one', async () => {
306-
queryHistoryManager = await createMockQueryHistory(allHistory);
293+
queryHistoryManager = await createMockQueryHistory(localQueryHistory);
307294

308295
// deleting the selected item automatically selects next item
309-
const toDelete = allHistory[1];
310-
const newSelected = allHistory[2];
296+
const toDelete = localQueryHistory[1];
297+
const newSelected = localQueryHistory[2];
311298
// avoid triggering the callback by setting the field directly
312299

313300
// select the item we want
314301
await queryHistoryManager.treeView.reveal(toDelete, { select: true });
315302
await queryHistoryManager.handleRemoveHistoryItem(toDelete, [toDelete]);
316303

317-
if (toDelete.t == 'local') {
318-
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
319-
} else if (toDelete.t == 'remote') {
320-
expect(remoteQueriesManagerStub.removeRemoteQuery).to.have.been.calledOnceWith((toDelete as RemoteQueryHistoryItem).queryId);
321-
} else if (toDelete.t == 'variant-analysis') {
322-
expect(variantAnalysisManagerStub.removeVariantAnalysis).to.have.been.calledOnceWith((toDelete as VariantAnalysisHistoryItem).variantAnalysis.id);
323-
}
324-
325-
// the current item should have been selected
326-
if (newSelected.t == 'local') {
327-
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(newSelected);
328-
} else if (toDelete.t == 'remote') {
329-
expect(remoteQueriesManagerStub.openRemoteQueryResults).to.have.been.calledOnceWith((newSelected as RemoteQueryHistoryItem).queryId);
330-
} else if (toDelete.t == 'variant-analysis') {
331-
expect(variantAnalysisManagerStub.showView).to.have.been.calledOnceWith((newSelected as VariantAnalysisHistoryItem).variantAnalysis.id);
332-
}
333-
304+
expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
334305
expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(newSelected);
335306
expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
307+
308+
// the current item should have been selected
309+
expect(localQueriesResultsViewStub.showResults).to.have.been.calledOnceWith(newSelected);
336310
});
337311
});
338312

0 commit comments

Comments
 (0)