From 11517eee4d0d30e599a9b69460c7b4536962a6ec Mon Sep 17 00:00:00 2001 From: Andrew Eisenberg Date: Tue, 20 Oct 2020 11:15:36 -0700 Subject: [PATCH] Add more structured output for tests The diff and the errors were always available, but they were not being sent to the output. Additionally, make sure to send output to both the test explorer log and the codeql test log. --- extensions/ql-vscode/CHANGELOG.md | 4 + extensions/ql-vscode/src/cli.ts | 4 +- extensions/ql-vscode/src/test-adapter.ts | 20 +++- .../no-workspace/databases.test.ts | 5 +- .../no-workspace/test-adapter.test.ts | 106 ++++++++++++++++++ 5 files changed, 134 insertions(+), 5 deletions(-) create mode 100644 extensions/ql-vscode/src/vscode-tests/no-workspace/test-adapter.test.ts diff --git a/extensions/ql-vscode/CHANGELOG.md b/extensions/ql-vscode/CHANGELOG.md index dd395101f35..ec64803ab3f 100644 --- a/extensions/ql-vscode/CHANGELOG.md +++ b/extensions/ql-vscode/CHANGELOG.md @@ -4,6 +4,9 @@ - Editors opened by navigating from the results view are no longer opened in _preview mode_. Now they are opened as a persistent editor. [#630](https://github.com/github/vscode-codeql/pull/630) - When comparing the results of a failed QL test run and the `.expected` file does not exist, an empty `.expected` file is created and compared against the `.actual` file. [#669](https://github.com/github/vscode-codeql/pull/669) +- Alter structure of the _Test Explorer_ tree. It now follows the structure of the filesystem instead of the QL Packs. [#624](https://github.com/github/vscode-codeql/pull/624) +- Alter structure of the _Test Explorer_ tree. It now follows the structure of the filesystem instead of the QL Packs. [#624](https://github.com/github/vscode-codeql/pull/624) +- Add more structured output for tests. [#626](https://github.com/github/vscode-codeql/pull/626) ## 1.3.6 - 4 November 2020 @@ -33,6 +36,7 @@ - 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. +- Ensure output of CodeQL test runs includes compilation error messages and test failure messages. ## 1.3.3 - 16 September 2020 diff --git a/extensions/ql-vscode/src/cli.ts b/extensions/ql-vscode/src/cli.ts index 48423de9d1c..153574384fc 100644 --- a/extensions/ql-vscode/src/cli.ts +++ b/extensions/ql-vscode/src/cli.ts @@ -17,6 +17,7 @@ import { DistributionProvider, FindDistributionResultKind } from './distribution import { assertNever } from './pure/helpers-pure'; import { QueryMetadata, SortDirection } from './pure/interface-types'; import { Logger, ProgressReporter } from './logging'; +import { CompilationMessage } from './pure/messages'; /** * The version of the SARIF format that we are using. @@ -90,10 +91,11 @@ export interface TestRunOptions { export interface TestCompleted { test: string; pass: boolean; - messages: string[]; + messages: CompilationMessage[]; compilationMs: number; evaluationMs: number; expected: string; + diff: string[] | undefined; } /** diff --git a/extensions/ql-vscode/src/test-adapter.ts b/extensions/ql-vscode/src/test-adapter.ts index e6dac4156aa..017bb3a45b2 100644 --- a/extensions/ql-vscode/src/test-adapter.ts +++ b/extensions/ql-vscode/src/test-adapter.ts @@ -217,10 +217,26 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter { cancellationToken: cancellationToken, logger: testLogger })) { + const state = event.pass + ? 'passed' + : event.messages?.length + ? 'errored' + : 'failed'; + let message: string | undefined; + if (event.diff?.length) { + message = ['', `${state}: ${event.test}`, ...event.diff, ''].join('\n'); + testLogger.log(message); + } + (event.diff || []).join('\n'); this._testStates.fire({ type: 'test', - state: event.pass ? 'passed' : 'failed', - test: event.test + state, + test: event.test, + message, + decorations: event.messages?.map(msg => ({ + line: msg.position.line, + message: msg.message + })) }); } } diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/databases.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/databases.test.ts index 127d806741f..07b8f49aba7 100644 --- a/extensions/ql-vscode/src/vscode-tests/no-workspace/databases.test.ts +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/databases.test.ts @@ -24,8 +24,9 @@ describe('databases', () => { databaseManager = new DatabaseManager( { workspaceState: { - update: updateSpy - } + update: updateSpy, + get: sinon.stub() + }, } as unknown as ExtensionContext, {} as QueryServerConfig, {} as Logger, diff --git a/extensions/ql-vscode/src/vscode-tests/no-workspace/test-adapter.test.ts b/extensions/ql-vscode/src/vscode-tests/no-workspace/test-adapter.test.ts new file mode 100644 index 00000000000..2070201779d --- /dev/null +++ b/extensions/ql-vscode/src/vscode-tests/no-workspace/test-adapter.test.ts @@ -0,0 +1,106 @@ +import 'vscode-test'; +import 'mocha'; +import * as sinon from 'sinon'; +import { Uri, WorkspaceFolder } from 'vscode'; +import { expect } from 'chai'; + +import { QLTestAdapter } from '../../test-adapter'; +import { CodeQLCliServer } from '../../cli'; + +describe('test-adapter', () => { + let adapter: QLTestAdapter; + let runTestsSpy: sinon.SinonStub; + let resolveTestsSpy: sinon.SinonStub; + let resolveQlpacksSpy: sinon.SinonStub; + let sandox: sinon.SinonSandbox; + + beforeEach(() => { + sandox = sinon.createSandbox(); + mockRunTests(); + resolveQlpacksSpy = sandox.stub().resolves({}); + resolveTestsSpy = sandox.stub().resolves([]); + adapter = new QLTestAdapter({ + name: 'ABC', + uri: Uri.parse('file:/ab/c') + } as WorkspaceFolder, { + runTests: runTestsSpy, + resolveQlpacks: resolveQlpacksSpy, + resolveTests: resolveTestsSpy + } as unknown as CodeQLCliServer); + }); + + afterEach(() => { + sandox.restore(); + }); + + it('should run some tests', async () => { + + const listenerSpy = sandox.spy(); + adapter.testStates(listenerSpy); + const testsPath = Uri.parse('file:/ab/c').fsPath; + const dPath = Uri.parse('file:/ab/c/d.ql').fsPath; + const gPath = Uri.parse('file:/ab/c/e/f/g.ql').fsPath; + const hPath = Uri.parse('file:/ab/c/e/f/h.ql').fsPath; + + await adapter.run([testsPath]); + + expect(listenerSpy.getCall(0).args).to.deep.eq([ + { type: 'started', tests: [testsPath] } + ]); + expect(listenerSpy.getCall(1).args).to.deep.eq([{ + type: 'test', + state: 'passed', + test: dPath, + message: undefined, + decorations: [] + }]); + expect(listenerSpy.getCall(2).args).to.deep.eq([{ + type: 'test', + state: 'errored', + test: gPath, + message: `\nerrored: ${gPath}\npqr\nxyz\n`, + decorations: [ + { line: 1, message: 'abc' } + ] + }]); + expect(listenerSpy.getCall(3).args).to.deep.eq([{ + type: 'test', + state: 'failed', + test: hPath, + message: `\nfailed: ${hPath}\njkh\ntuv\n`, + decorations: [] + }]); + expect(listenerSpy.getCall(4).args).to.deep.eq([{ type: 'finished' }]); + expect(listenerSpy).to.have.callCount(5); + }); + + function mockRunTests() { + // runTests is an async generator function. This is not directly supported in sinon + // However, we can pretend the same thing by just returning an async array. + runTestsSpy = sandox.stub(); + runTestsSpy.returns( + (async function*() { + yield Promise.resolve({ + test: Uri.parse('file:/ab/c/d.ql').fsPath, + pass: true, + messages: [] + }); + yield Promise.resolve({ + test: Uri.parse('file:/ab/c/e/f/g.ql').fsPath, + pass: false, + diff: ['pqr', 'xyz'], + // a compile error + messages: [ + { position: { line: 1 }, message: 'abc' } + ] + }); + yield Promise.resolve({ + test: Uri.parse('file:/ab/c/e/f/h.ql').fsPath, + pass: false, + diff: ['jkh', 'tuv'], + messages: [] + }); + })() + ); + } +});