Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add more structured output for tests #626

Merged
merged 1 commit into from
Nov 9, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions extensions/ql-vscode/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand Down
4 changes: 3 additions & 1 deletion extensions/ql-vscode/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}

/**
Expand Down
20 changes: 18 additions & 2 deletions extensions/ql-vscode/src/test-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,26 @@ export class QLTestAdapter extends DisposableObject implements TestAdapter {
cancellationToken: cancellationToken,
logger: testLogger
})) {
const state = event.pass
? 'passed'
: event.messages?.length
? 'errored'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would failed due to errors or failed to run be more meaningful for the user? I had to think about the difference between 'errored' and 'failed' for a moment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That won't work. state is a property of the TestEvent interface and only allows 'running' | 'passed' | 'failed' | 'skipped' | 'errored'. The hope is that the messages will be more explicit as to what happened.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh fair enough. We can consider having more detail in the way message is constructed below, but no need to block this PR.

: '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
}))
});
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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: []
});
})()
);
}
});