Restructure the tree in the Test Explorer View

extensions/ql-vscode/CHANGELOG.md

@@ -28,6 +28,7 @@
 - Add a `View DIL` command on query history items. This opens a text editor containing the Datalog Intermediary Language representation of the compiled query.
 - Remove feature flag for the AST Viewer. For more information on how to use the AST Viewer, [see the documentation](https://help.semmle.com/codeql/codeql-for-vscode/procedures/exploring-the-structure-of-your-source-code.html).
 - The `codeQL.runningTests.numberOfThreads` setting is now used correctly when running tests.
+- Alter structure of the _Test Explorer_ tree. It now follows the structure of the filesystem instead of the qlpacks.
 
 ## 1.3.3 - 16 September 2020
 

extensions/ql-vscode/src/cli.ts

@@ -470,7 +470,11 @@ export class CodeQLCliServer implements Disposable {
     const subcommandArgs = [
       testPath
     ];
-    return await this.runJsonCodeQlCliCommand<ResolvedTests>(['resolve', 'tests'], subcommandArgs, 'Resolving tests');
+    return await this.runJsonCodeQlCliCommand<ResolvedTests>(
+      ['resolve', 'tests', '--strict-test-discovery'],
+      subcommandArgs,
+      'Resolving tests'
+    );
   }
 
   /**

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

@@ -1,7 +1,6 @@
 import * as path from 'path';
-import { QLPackDiscovery, QLPack } from './qlpack-discovery';
 import { Discovery } from './discovery';
-import { EventEmitter, Event, Uri, RelativePattern, WorkspaceFolder, env, workspace } from 'vscode';
+import { EventEmitter, Event, Uri, RelativePattern, WorkspaceFolder, env } from 'vscode';
 import { MultiFileSystemWatcher } from './vscode-utils/multi-file-system-watcher';
 import { CodeQLCliServer } from './cli';
 
@@ -29,9 +28,8 @@ export abstract class QLTestNode {
  * A directory containing one or more QL tests or other test directories.
  */
 export class QLTestDirectory extends QLTestNode {
-  private _children: QLTestNode[] = [];
 
-  constructor(_path: string, _name: string) {
+  constructor(_path: string, _name: string, private _children: QLTestNode[] = []) {
     super(_path, _name);
   }
 
@@ -55,10 +53,23 @@ export class QLTestDirectory extends QLTestNode {
   }
 
   public finish(): void {
+    // remove empty directories
+    this._children.filter(child =>
+      child instanceof QLTestFile || child.children.length > 0
+    );
     this._children.sort((a, b) => a.name.localeCompare(b.name, env.language));
-    for (const child of this._children) {
+    this._children.forEach((child, i) => {
       child.finish();
-    }
+      if (child.children?.length === 1 && child.children[0] instanceof QLTestDirectory) {
+        // collapse children
+        const replacement = new QLTestDirectory(
+          child.children[0].path,
+          child.name + ' / ' + child.children[0].name,
+          Array.from(child.children[0].children)
+        );
+        this._children[i] = replacement;
+      }
+    });
   }
 
   private createChildDirectory(name: string): QLTestDirectory {
@@ -96,14 +107,15 @@ export class QLTestFile extends QLTestNode {
  */
 interface QLTestDiscoveryResults {
   /**
-   * The root test directory for each QL pack that contains tests.
+   * A directory that contains one or more QL Tests, or other QLTestDirectories.
    */
-  testDirectories: QLTestDirectory[];
+  testDirectory: QLTestDirectory | undefined;
+
   /**
-   * The list of file system paths to watch. If any of these paths changes, the discovery results
-   * may be out of date.
+   * The file system path to a directory to watch. If any ql or qlref file changes in
+   * this directory, then this signifies a change in tests.
    */
-  watchPaths: string[];
+  watchPath: string;
 }
 
 /**
@@ -112,104 +124,76 @@ interface QLTestDiscoveryResults {
 export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
   private readonly _onDidChangeTests = this.push(new EventEmitter<void>());
   private readonly watcher: MultiFileSystemWatcher = this.push(new MultiFileSystemWatcher());
-  private _testDirectories: QLTestDirectory[] = [];
+  private _testDirectory: QLTestDirectory | undefined;
 
   constructor(
-    private readonly qlPackDiscovery: QLPackDiscovery,
     private readonly workspaceFolder: WorkspaceFolder,
     private readonly cliServer: CodeQLCliServer
   ) {
     super('QL Test Discovery');
 
-    this.push(this.qlPackDiscovery.onDidChangeQLPacks(this.handleDidChangeQLPacks, this));
     this.push(this.watcher.onDidChange(this.handleDidChange, this));
   }
 
   /**
    * Event to be fired when the set of discovered tests may have changed.
    */
-  public get onDidChangeTests(): Event<void> { return this._onDidChangeTests.event; }
+  public get onDidChangeTests(): Event<void> {
+    return this._onDidChangeTests.event;
+  }
 
   /**
-   * The root test directory for each QL pack that contains tests.
+   * The root directory. There is at least one test in this directory, or
+   * in a subdirectory of this.
    */
-  public get testDirectories(): QLTestDirectory[] { return this._testDirectories; }
-
-  private handleDidChangeQLPacks(): void {
-    this.refresh();
+  public get testDirectory(): QLTestDirectory | undefined {
+    return this._testDirectory;
   }
 
   private handleDidChange(uri: Uri): void {
     if (!QLTestDiscovery.ignoreTestPath(uri.fsPath)) {
       this.refresh();
     }
   }
-
   protected async discover(): Promise<QLTestDiscoveryResults> {
-    const testDirectories: QLTestDirectory[] = [];
-    const watchPaths: string[] = [];
-    const qlPacks = this.qlPackDiscovery.qlPacks;
-    for (const qlPack of qlPacks) {
-      //HACK: Assume that only QL packs whose name ends with '-tests' contain tests.
-      if (this.isRelevantQlPack(qlPack)) {
-        watchPaths.push(qlPack.uri.fsPath);
-        const testPackage = await this.discoverTests(qlPack.uri.fsPath, qlPack.name);
-        if (testPackage !== undefined) {
-          testDirectories.push(testPackage);
-        }
-      }
-    }
-
-    return { testDirectories, watchPaths };
+    const testDirectory = await this.discoverTests();
+    return {
+      testDirectory,
+      watchPath: this.workspaceFolder.uri.fsPath
+    };
   }
 
   protected update(results: QLTestDiscoveryResults): void {
-    this._testDirectories = results.testDirectories;
+    this._testDirectory = results.testDirectory;
 
     // Watch for changes to any `.ql` or `.qlref` file in any of the QL packs that contain tests.
     this.watcher.clear();
-    results.watchPaths.forEach(watchPath => {
-      this.watcher.addWatch(new RelativePattern(watchPath, '**/*.{ql,qlref}'));
-    });
+    this.watcher.addWatch(new RelativePattern(results.watchPath, '**/*.{ql,qlref}'));
     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')
-      && workspace.getWorkspaceFolder(qlPack.uri)?.index === this.workspaceFolder.index;
-  }
-
   /**
    * Discover all QL tests in the specified directory and its subdirectories.
-   * @param fullPath The full path of the test directory.
-   * @param name The display name to use for the returned `TestDirectory` object.
    * @returns A `QLTestDirectory` object describing the contents of the directory, or `undefined` if
    *   no tests were found.
    */
-  private async discoverTests(fullPath: string, name: string): Promise<QLTestDirectory | undefined> {
+  private async discoverTests(): Promise<QLTestDirectory> {
+    const fullPath = this.workspaceFolder.uri.fsPath;
+    const name = this.workspaceFolder.name;
     const resolvedTests = (await this.cliServer.resolveTests(fullPath))
       .filter((testPath) => !QLTestDiscovery.ignoreTestPath(testPath));
 
-    if (resolvedTests.length === 0) {
-      return undefined;
+    const rootDirectory = new QLTestDirectory(fullPath, name);
+    for (const testPath of resolvedTests) {
+      const relativePath = path.normalize(path.relative(fullPath, testPath));
+      const dirName = path.dirname(relativePath);
+      const parentDirectory = rootDirectory.createDirectory(dirName);
+      parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
     }
-    else {
-      const rootDirectory = new QLTestDirectory(fullPath, name);
-      for (const testPath of resolvedTests) {
-        const relativePath = path.normalize(path.relative(fullPath, testPath));
-        const dirName = path.dirname(relativePath);
-        const parentDirectory = rootDirectory.createDirectory(dirName);
-        parentDirectory.addChild(new QLTestFile(testPath, path.basename(testPath)));
-      }
 
-      rootDirectory.finish();
+    rootDirectory.finish();
 
-      return rootDirectory;
-    }
+    return rootDirectory;
   }
 
   /**

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

@@ -545,9 +545,7 @@ export class QueryHistoryManager extends DisposableObject {
   private async tryOpenExternalFile(fileLocation: string) {
     const uri = vscode.Uri.file(fileLocation);
     try {
-      await vscode.window.showTextDocument(uri, {
-        preview: false
-      });
+      await vscode.window.showTextDocument(uri, { preview: false });
     } catch (e) {
       if (
         e.message.includes(

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

@@ -16,7 +16,6 @@ import { TestAdapterRegistrar } from 'vscode-test-adapter-util';
 import { QLTestFile, QLTestNode, QLTestDirectory, QLTestDiscovery } from './qltest-discovery';
 import { Event, EventEmitter, CancellationTokenSource, CancellationToken } from 'vscode';
 import { DisposableObject } from './vscode-utils/disposable-object';
-import { QLPackDiscovery } from './qlpack-discovery';
 import { CodeQLCliServer } from './cli';
 import { getOnDiskWorkspaceFolders } from './helpers';
 import { testLogger } from './logging';
@@ -82,7 +81,6 @@ function changeExtension(p: string, ext: string): string {
  * Test adapter for QL tests.
  */
 export class QLTestAdapter extends DisposableObject implements TestAdapter {
-  private readonly qlPackDiscovery: QLPackDiscovery;
   private readonly qlTestDiscovery: QLTestDiscovery;
   private readonly _tests = this.push(
     new EventEmitter<TestLoadStartedEvent | TestLoadFinishedEvent>());
@@ -97,9 +95,7 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
   ) {
     super();
 
-    this.qlPackDiscovery = this.push(new QLPackDiscovery(workspaceFolder, cliServer));
-    this.qlTestDiscovery = this.push(new QLTestDiscovery(this.qlPackDiscovery, workspaceFolder, cliServer));
-    this.qlPackDiscovery.refresh();
+    this.qlTestDiscovery = this.push(new QLTestDiscovery(workspaceFolder, cliServer));
     this.qlTestDiscovery.refresh();
 
     this.push(this.qlTestDiscovery.onDidChangeTests(this.discoverTests, this));
@@ -160,20 +156,20 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
   private discoverTests(): void {
     this._tests.fire({ type: 'started' } as TestLoadStartedEvent);
 
-    const testDirectories = this.qlTestDiscovery.testDirectories;
-    const children = testDirectories.map(
-      testDirectory => QLTestAdapter.createTestSuiteInfo(testDirectory, testDirectory.name)
-    );
-    const testSuite: TestSuiteInfo = {
-      type: 'suite',
-      label: 'CodeQL',
-      id: '.',
-      children
-    };
-
+    const testDirectory = this.qlTestDiscovery.testDirectory;
+    let testSuite: TestSuiteInfo | undefined;
+    if (testDirectory?.children.length) {
+      const children = QLTestAdapter.createTestOrSuiteInfos(testDirectory.children);
+      testSuite = {
+        type: 'suite',
+        label: 'CodeQL',
+        id: testDirectory.path,
+        children
+      };
+    }
     this._tests.fire({
       type: 'finished',
-      suite: children.length > 0 ? testSuite : undefined
+      suite: testSuite
     } as TestLoadFinishedEvent);
   }