Add tests for sort order and selection

Author: aeisenberg

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

@@ -70,7 +70,7 @@ const FAILED_QUERY_HISTORY_ITEM_ICON = 'media/red-x.svg';
  */
 const LOCAL_SUCCESS_QUERY_HISTORY_ITEM_ICON = 'media/drive.svg';
 
-enum SortOrder {
+export enum SortOrder {
   NameAsc = 'NameAsc',
   NameDesc = 'NameDesc',
   DateAsc = 'DateAsc',
@@ -820,6 +820,9 @@ the file in the file explorer and dragging it into the workspace.`
 
   /**
    * If no items are selected, attempt to grab the selection from the treeview.
+   * However, often the treeview itself does not have any selection. In this case,
+   * grab the selection from the `treeDataProvider` current item.
+   *
    * We need to use this method because when clicking on commands from the view title
    * bar, the selections are not passed in.
    *
@@ -845,6 +848,13 @@ the file in the file explorer and dragging it into the workspace.`
         };
       }
     }
+
+    // ensure we do not return undefined
+    if (singleItem && !multiSelect?.[0]) {
+      multiSelect = [singleItem];
+    } else if (!singleItem && multiSelect?.[0]) {
+      singleItem = multiSelect[0];
+    }
     return {
       finalSingleItem: singleItem,
       finalMultiSelect: multiSelect

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

@@ -5,7 +5,7 @@ import * as vscode from 'vscode';
 import * as sinon from 'sinon';
 import * as chaiAsPromised from 'chai-as-promised';
 import { logger } from '../../logging';
-import { QueryHistoryManager, HistoryTreeDataProvider } from '../../query-history';
+import { QueryHistoryManager, HistoryTreeDataProvider, SortOrder } from '../../query-history';
 import { QueryEvaluationInfo, QueryWithResults } from '../../run-queries';
 import { QueryHistoryConfigListener } from '../../config';
 import * as messages from '../../pure/messages';
@@ -217,13 +217,14 @@ describe('query-history', () => {
     // will not change the selection
     const toDelete = allHistory[1];
     const selected = allHistory[3];
-    // avoid triggering the callback by setting the field directly
-    (queryHistoryManager.treeDataProvider as any).current = selected;
+
+    // select the item we want
+    await queryHistoryManager.treeView.reveal(selected, { select: true });
     await queryHistoryManager.handleRemoveHistoryItem(toDelete, [toDelete]);
 
     expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
-    expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(selected);
-    expect(allHistory).not.to.contain(toDelete);
+    expect(queryHistoryManager.treeDataProvider.getCurrent()).to.deep.eq(selected);
+    expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
 
     // the current item should have been re-selected
     expect(selectedCallback).to.have.been.calledOnceWith(selected);
@@ -236,12 +237,14 @@ describe('query-history', () => {
     const toDelete = allHistory[1];
     const newSelected = allHistory[2];
     // avoid triggering the callback by setting the field directly
-    (queryHistoryManager.treeDataProvider as any).current = toDelete;
+
+    // select the item we want
+    await queryHistoryManager.treeView.reveal(toDelete, { select: true });
     await queryHistoryManager.handleRemoveHistoryItem(toDelete, [toDelete]);
 
     expect(toDelete.completedQuery!.dispose).to.have.been.calledOnce;
     expect(queryHistoryManager.treeDataProvider.getCurrent()).to.eq(newSelected);
-    expect(allHistory).not.to.contain(toDelete);
+    expect(queryHistoryManager.treeDataProvider.allHistory).not.to.contain(toDelete);
 
     // the current item should have been selected
     expect(selectedCallback).to.have.been.calledOnceWith(newSelected);
@@ -356,6 +359,155 @@ describe('query-history', () => {
     });
   });
 
+  describe('determineSelection', () => {
+    const singleItem = 'a';
+    const multipleItems = ['b', 'c', 'd'];
+
+    it('should get the selection from parameters', async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      const selection = (queryHistoryManager as any).determineSelection(singleItem, multipleItems);
+      expect(selection).to.deep.eq({
+        finalSingleItem: singleItem,
+        finalMultiSelect: multipleItems
+      });
+    });
+
+    it('should get the selection when single selection is empty', async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      const selection = (queryHistoryManager as any).determineSelection(undefined, multipleItems);
+      expect(selection).to.deep.eq({
+        finalSingleItem: multipleItems[0],
+        finalMultiSelect: multipleItems
+      });
+    });
+
+    it('should get the selection when multi-selection is empty', async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      const selection = (queryHistoryManager as any).determineSelection(singleItem, undefined);
+      expect(selection).to.deep.eq({
+        finalSingleItem: singleItem,
+        finalMultiSelect: [singleItem]
+      });
+    });
+
+    it('should get the selection from the treeView when both selections are empty', async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      await queryHistoryManager.treeView.reveal(allHistory[1], { select: true });
+      const selection = (queryHistoryManager as any).determineSelection(undefined, undefined);
+      expect(selection).to.deep.eq({
+        finalSingleItem: allHistory[1],
+        finalMultiSelect: [allHistory[1]]
+      });
+    });
+
+    it('should get the selection from the treeDataProvider when both selections and the treeView are empty', async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      await queryHistoryManager.treeView.reveal(allHistory[1], { select: true });
+      const selection = (queryHistoryManager as any).determineSelection(undefined, undefined);
+      expect(selection).to.deep.eq({
+        finalSingleItem: allHistory[1],
+        finalMultiSelect: [allHistory[1]]
+      });
+    });
+  });
+
+  describe('getChildren', () => {
+    const history = [
+      item('a', 10, 20),
+      item('b', 5, 30),
+      item('c', 1, 25),
+    ];
+    let treeDataProvider: HistoryTreeDataProvider;
+
+    beforeEach(async () => {
+      queryHistoryManager = await createMockQueryHistory(allHistory);
+      (queryHistoryManager.treeDataProvider as any).history = [...history];
+      treeDataProvider = queryHistoryManager.treeDataProvider;
+    });
+
+    it('should get children for name ascending', async () => {
+      const expected = [...history];
+      treeDataProvider.sortOrder = SortOrder.NameAsc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for name descending', async () => {
+      const expected = [...history].reverse();
+      treeDataProvider.sortOrder = SortOrder.NameDesc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for date ascending', async () => {
+      const expected = [history[2], history[1], history[0]];
+      treeDataProvider.sortOrder = SortOrder.DateAsc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for date descending', async () => {
+      const expected = [history[0], history[1], history[2]];
+      treeDataProvider.sortOrder = SortOrder.DateDesc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for result count ascending', async () => {
+      const expected = [history[0], history[2], history[1]];
+      treeDataProvider.sortOrder = SortOrder.CountAsc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for result count descending', async () => {
+      const expected = [history[1], history[2], history[0]];
+      treeDataProvider.sortOrder = SortOrder.CountDesc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for result count ascending when there are no results', async () => {
+      // fall back to name
+      const thisHistory = [item('a', 10), item('b', 50), item('c', 1)];
+      (queryHistoryManager!.treeDataProvider as any).history = [...thisHistory];
+      const expected = [...thisHistory];
+      treeDataProvider.sortOrder = SortOrder.CountAsc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    it('should get children for result count descending when there are no results', async () => {
+      // fall back to name
+      const thisHistory = [item('a', 10), item('b', 50), item('c', 1)];
+      (queryHistoryManager!.treeDataProvider as any).history = [...thisHistory];
+      const expected = [...thisHistory].reverse();
+      treeDataProvider.sortOrder = SortOrder.CountDesc;
+
+      const children = await treeDataProvider.getChildren();
+      expect(children).to.deep.eq(expected);
+    });
+
+    function item(label: string, start: number, resultCount?: number) {
+      return {
+        label,
+        initialInfo: {
+          start: new Date(start),
+        },
+        completedQuery: {
+          resultCount,
+        }
+      };
+    }
+  });
+
   function createMockFullQueryInfo(dbName = 'a', queryWitbResults?: QueryWithResults, isFail = false): FullQueryInfo {
     const fqi = new FullQueryInfo(
       {
@@ -397,9 +549,9 @@ describe('query-history', () => {
       selectedCallback,
       doCompareCallback
     );
-    (qhm.treeDataProvider as any).history = allHistory;
+    (qhm.treeDataProvider as any).history = [...allHistory];
     await vscode.workspace.saveAll();
-
+    qhm.refreshTreeView();
     return qhm;
   }
 });