Filters qltest-discovery

qlpack tests that are not contained within the current workspace folder
will be filtered from the test runner view.

This also fixes a test that should have been failing but wasn't.

Author: aeisenberg

extensions/ql-vscode/src/qlpack-discovery.ts

@@ -16,18 +16,17 @@ export class QLPackDiscovery extends Discovery<QlpacksInfo> {
   private readonly watcher = this.push(new MultiFileSystemWatcher());
   private _qlPacks: readonly QLPack[] = [];
 
-  constructor(private readonly workspaceFolder: WorkspaceFolder,
-    private readonly cliServer: CodeQLCliServer) {
-
+  constructor(
+    private readonly workspaceFolder: WorkspaceFolder,
+    private readonly cliServer: CodeQLCliServer
+  ) {
     super();
 
     // Watch for any changes to `qlpack.yml` files in this workspace folder.
     // TODO: The CLI server should tell us what paths to watch for.
     this.watcher.addWatch(new RelativePattern(this.workspaceFolder, '**/qlpack.yml'));
     this.watcher.addWatch(new RelativePattern(this.workspaceFolder, '**/.codeqlmanifest.json'));
     this.push(this.watcher.onDidChange(this.handleQLPackFileChanged, this));
-
-    this.refresh();
   }
 
   public get onDidChangeQLPacks(): Event<void> { return this._onDidChangeQLPacks.event; }

extensions/ql-vscode/src/qltest-discovery.ts

@@ -1,7 +1,7 @@
 import * as path from 'path';
-import { QLPackDiscovery } from './qlpack-discovery';
+import { QLPackDiscovery, QLPack } from './qlpack-discovery';
 import { Discovery } from './discovery';
-import { EventEmitter, Event, Uri, RelativePattern, env } from 'vscode';
+import { EventEmitter, Event, Uri, RelativePattern, WorkspaceFolder, env } from 'vscode';
 import { MultiFileSystemWatcher } from './vscode-utils/multi-file-system-watcher';
 import { CodeQLCliServer } from './cli';
 
@@ -114,15 +114,15 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
   private readonly watcher: MultiFileSystemWatcher = this.push(new MultiFileSystemWatcher());
   private _testDirectories: QLTestDirectory[] = [];
 
-  constructor(private readonly qlPackDiscovery: QLPackDiscovery,
-    private readonly cliServer: CodeQLCliServer) {
-
+  constructor(
+    private readonly qlPackDiscovery: QLPackDiscovery,
+    private readonly workspaceFolder: WorkspaceFolder,
+    private readonly cliServer: CodeQLCliServer
+  ) {
     super();
 
     this.push(this.qlPackDiscovery.onDidChangeQLPacks(this.handleDidChangeQLPacks, this));
     this.push(this.watcher.onDidChange(this.handleDidChange, this));
-
-    this.refresh();
   }
 
   /**
@@ -151,7 +151,7 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
     const qlPacks = this.qlPackDiscovery.qlPacks;
     for (const qlPack of qlPacks) {
       //HACK: Assume that only QL packs whose name ends with '-tests' contain tests.
-      if (qlPack.name.endsWith('-tests')) {
+      if (this.isRelevantQlPack(qlPack)) {
         watchPaths.push(qlPack.uri.fsPath);
         const testPackage = await this.discoverTests(qlPack.uri.fsPath, qlPack.name);
         if (testPackage !== undefined) {
@@ -160,10 +160,7 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
       }
     }
 
-    return {
-      testDirectories: testDirectories,
-      watchPaths: watchPaths
-    };
+    return { testDirectories, watchPaths };
   }
 
   protected update(results: QLTestDiscoveryResults): void {
@@ -177,6 +174,15 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
     this._onDidChangeTests.fire();
   }
 
+  /**
+   * Only include qlpacks suffixed with '-tests' that are contained
+   * within the provided workspace folder.
+   */
+  private isRelevantQlPack(qlPack: QLPack): boolean {
+    return qlPack.name.endsWith('-tests')
+      && qlPack.uri.fsPath.startsWith(this.workspaceFolder.uri.fsPath);
+  }
+
   /**
    * Discover all QL tests in the specified directory and its subdirectories.
    * @param fullPath The full path of the test directory.

extensions/ql-vscode/src/test-adapter.ts

@@ -98,7 +98,9 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
     super();
 
     this.qlPackDiscovery = this.push(new QLPackDiscovery(workspaceFolder, cliServer));
-    this.qlTestDiscovery = this.push(new QLTestDiscovery(this.qlPackDiscovery, cliServer));
+    this.qlTestDiscovery = this.push(new QLTestDiscovery(this.qlPackDiscovery, workspaceFolder, cliServer));
+    this.qlPackDiscovery.refresh();
+    this.qlTestDiscovery.refresh();
 
     this.push(this.qlTestDiscovery.onDidChangeTests(this.discoverTests, this));
   }

extensions/ql-vscode/src/vscode-tests/no-workspace/contextual/queryResolver.test.ts

@@ -41,14 +41,20 @@ describe('queryResolver', () => {
       });
     });
 
-    it('should throw an error when there are no queries found', () => {
+    it('should throw an error when there are no queries found', async () => {
       mockCli.resolveQueriesInSuite.returns([]);
 
-      expect(module.resolveQueries(
-        mockCli, 'my-qlpack', KeyType.DefinitionQuery)
-      ).to.be.rejectedWith(
-        'Couldn\'t find any queries tagged ide-contextual-queries/local-definitions for qlpack my-qlpack'
-      );
+      // TODO: Figure out why chai-as-promised isn't failing the test on an
+      // unhandled rejection.
+      try {
+        await module.resolveQueries(mockCli, 'my-qlpack', KeyType.DefinitionQuery);
+        // should reject
+        expect(true).to.be.false;
+      } catch (e) {
+        expect(e.message).to.eq(
+          'Couldn\'t find any queries tagged ide-contextual-queries/local-definitions for qlpack my-qlpack'
+        );
+      }
     });
   });
 
@@ -78,7 +84,8 @@ describe('queryResolver', () => {
 
       '../helpers': {
         resolveDatasetFolder: resolveDatasetFolderSpy,
-        getOnDiskWorkspaceFolders: () => ({})
+        getOnDiskWorkspaceFolders: () => ({}),
+        showAndLogErrorMessage: () => ({})
       }
     });
   }

extensions/ql-vscode/src/vscode-tests/no-workspace/qltest-discovery.test.ts

@@ -0,0 +1,38 @@
+import 'vscode-test';
+import 'mocha';
+import { Uri } from 'vscode';
+import { expect } from 'chai';
+
+import { QLTestDiscovery } from '../../qltest-discovery';
+
+describe('qltest-discovery', () => {
+  describe('isRelevantQlPack', () => {
+    it('should check if a qlpack is relevant', () => {
+      const qlTestDiscover: any = new QLTestDiscovery(
+        { onDidChangeQLPacks: () => ({}) } as any,
+        { uri: Uri.parse('file:///a/b/c') } as any,
+        {} as any
+      );
+
+      expect(qlTestDiscover.isRelevantQlPack({
+        name: '-hucairz',
+        uri: Uri.parse('file:///a/b/c/d')
+      })).to.be.false;
+
+      expect(qlTestDiscover.isRelevantQlPack({
+        name: '-tests',
+        uri: Uri.parse('file:///a/b/')
+      })).to.be.false;
+
+      expect(qlTestDiscover.isRelevantQlPack({
+        name: '-tests',
+        uri: Uri.parse('file:///a/b/c')
+      })).to.be.true;
+
+      expect(qlTestDiscover.isRelevantQlPack({
+        name: '-tests',
+        uri: Uri.parse('file:///a/b/c/d')
+      })).to.be.true;
+    });
+  });
+});