Restructure the tree in the Test Explorer View

With this change we display the tree based on the file system not based
on ql-packs. We also merge test folders whose only child is another
test folder.

Resolves #595
This commit is contained in:
Andrew Eisenberg
2020-10-19 15:04:53 -07:00
parent afd0694111
commit d553f6c069
4 changed files with 119 additions and 98 deletions

View File

@@ -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'
);
}
/**

View File

@@ -1,7 +1,7 @@
import * as path from 'path';
import { QLPackDiscovery, QLPack } from './qlpack-discovery';
import { QLPackDiscovery } 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 +29,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 +54,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 +108,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,7 +125,7 @@ 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,
@@ -128,12 +141,17 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
/**
* 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; }
public get testDirectory(): QLTestDirectory | undefined {
return this._testDirectory;
}
private handleDidChangeQLPacks(): void {
this.refresh();
@@ -144,72 +162,49 @@ export class QLTestDiscovery extends Discovery<QLTestDiscoveryResults> {
this.refresh();
}
}
static cnt = 0;
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 timer = 'testDirectory-' + this.workspaceFolder.uri.fsPath + '-' + QLTestDiscovery.cnt++;
console.time(timer);
const testDirectory = await this.discoverTests();
console.timeEnd(timer);
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;
}
/**

View File

@@ -99,6 +99,7 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
this.qlPackDiscovery = this.push(new QLPackDiscovery(workspaceFolder, cliServer));
this.qlTestDiscovery = this.push(new QLTestDiscovery(this.qlPackDiscovery, workspaceFolder, cliServer));
// TODO: Don't run test discovery until pack discovery finishes
this.qlPackDiscovery.refresh();
this.qlTestDiscovery.refresh();
@@ -160,22 +161,22 @@ 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);
}
}
public async run(tests: string[]): Promise<void> {
if (this.runningTask !== undefined) {

View File

@@ -1,40 +1,61 @@
import 'vscode-test';
import 'mocha';
import * as path from 'path';
import { Uri, workspace } from 'vscode';
import { Uri, WorkspaceFolder } 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(
describe('discoverTests', () => {
it('should run discovery', async () => {
const baseUri = Uri.parse('file:/a/b');
const baseDir = baseUri.fsPath;
const cDir = Uri.parse('file:/a/b/c').fsPath;
const dFile = Uri.parse('file:/a/b/c/d.ql').fsPath;
const eFile = Uri.parse('file:/a/b/c/e.ql').fsPath;
const hDir = Uri.parse('file:/a/b/c/f/g/h').fsPath;
const iFile = Uri.parse('file:/a/b/c/f/g/h/i.ql').fsPath;
const qlTestDiscover = new QLTestDiscovery(
{ onDidChangeQLPacks: () => ({}) } as any,
{ index: 0 } as any,
{} as any
{
uri: baseUri,
name: 'My tests'
} as unknown as WorkspaceFolder,
{
resolveTests() {
return [
Uri.parse('file:/a/b/c/d.ql').fsPath,
Uri.parse('file:/a/b/c/e.ql').fsPath,
Uri.parse('file:/a/b/c/f/g/h/i.ql').fsPath
];
}
} as any
);
const uri = workspace.workspaceFolders![0].uri;
expect(qlTestDiscover.isRelevantQlPack({
name: '-hucairz',
uri
})).to.be.false;
const result = await (qlTestDiscover as any).discover();
expect(result.watchPath).to.eq(baseDir);
expect(result.testDirectory.path).to.eq(baseDir);
expect(result.testDirectory.name).to.eq('My tests');
expect(qlTestDiscover.isRelevantQlPack({
name: '-tests',
uri: Uri.file('/a/b/')
})).to.be.false;
let children = result.testDirectory.children;
expect(children[0].path).to.eq(cDir);
expect(children[0].name).to.eq('c');
expect(children.length).to.eq(1);
expect(qlTestDiscover.isRelevantQlPack({
name: '-tests',
uri
})).to.be.true;
children = children[0].children;
expect(children[0].path).to.eq(dFile);
expect(children[0].name).to.eq('d.ql');
expect(children[1].path).to.eq(eFile);
expect(children[1].name).to.eq('e.ql');
expect(qlTestDiscover.isRelevantQlPack({
name: '-tests',
uri: Uri.file(path.join(uri.fsPath, 'other'))
})).to.be.true;
// A merged foler
expect(children[2].path).to.eq(hDir);
expect(children[2].name).to.eq('f / g / h');
expect(children.length).to.eq(3);
children = children[2].children;
expect(children[0].path).to.eq(iFile);
expect(children[0].name).to.eq('i.ql');
});
});
});