From d14c7b4114e3eb7308eba7ef04940e765f701f20 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 28 Feb 2024 10:47:29 +0100 Subject: [PATCH 1/2] Show test results for tests with warnings --- extensions/ql-vscode/src/codeql-cli/cli.ts | 14 ++++-- .../src/query-testing/test-manager.ts | 48 ++++++++++++------- 2 files changed, 43 insertions(+), 19 deletions(-) diff --git a/extensions/ql-vscode/src/codeql-cli/cli.ts b/extensions/ql-vscode/src/codeql-cli/cli.ts index fd93fd79ae9..7c0b75ae706 100644 --- a/extensions/ql-vscode/src/codeql-cli/cli.ts +++ b/extensions/ql-vscode/src/codeql-cli/cli.ts @@ -156,9 +156,17 @@ type ResolvedQueries = string[]; type ResolvedTests = string[]; /** - * A compilation message for a test message (either an error or a warning) + * The severity of a compilation message for a test message. */ -interface CompilationMessage { +export enum CompilationMessageSeverity { + Error = "ERROR", + Warning = "WARNING", +} + +/** + * A compilation message for a test message (either an error or a warning). + */ +export interface CompilationMessage { /** * The text of the message */ @@ -170,7 +178,7 @@ interface CompilationMessage { /** * The severity of the message */ - severity: number; + severity: CompilationMessageSeverity; } /** diff --git a/extensions/ql-vscode/src/query-testing/test-manager.ts b/extensions/ql-vscode/src/query-testing/test-manager.ts index ff1a91d88e3..b7eb7139df7 100644 --- a/extensions/ql-vscode/src/query-testing/test-manager.ts +++ b/extensions/ql-vscode/src/query-testing/test-manager.ts @@ -21,7 +21,8 @@ import { } from "vscode"; import { DisposableObject } from "../common/disposable-object"; import { QLTestDiscovery } from "./qltest-discovery"; -import type { CodeQLCliServer } from "../codeql-cli/cli"; +import type { CodeQLCliServer, CompilationMessage } from "../codeql-cli/cli"; +import { CompilationMessageSeverity } from "../codeql-cli/cli"; import { getErrorMessage } from "../common/helpers-pure"; import type { BaseLogger, LogOptions } from "../common/logging"; import type { TestRunner } from "./test-runner"; @@ -66,6 +67,23 @@ function changeExtension(p: string, ext: string): string { return p.slice(0, -extname(p).length) + ext; } +function compilationMessageToTestMessage( + compilationMessage: CompilationMessage, +): TestMessage { + const location = new Location( + Uri.file(compilationMessage.position.fileName), + new Range( + compilationMessage.position.line - 1, + compilationMessage.position.column - 1, + compilationMessage.position.endLine - 1, + compilationMessage.position.endColumn - 1, + ), + ); + const testMessage = new TestMessage(compilationMessage.message); + testMessage.location = location; + return testMessage; +} + /** * Returns the complete text content of the specified file. If there is an error reading the file, * an error message is added to `testMessages` and this function returns undefined. @@ -398,23 +416,15 @@ export class TestManager extends DisposableObject { ); } } - if (event.messages?.length > 0) { + const errorMessages = event.messages.filter( + (m) => m.severity === CompilationMessageSeverity.Error, + ); + if (errorMessages.length > 0) { // The test didn't make it far enough to produce results. Transform any error messages // into `TestMessage`s and report the test as "errored". - const testMessages = event.messages.map((m) => { - const location = new Location( - Uri.file(m.position.fileName), - new Range( - m.position.line - 1, - m.position.column - 1, - m.position.endLine - 1, - m.position.endColumn - 1, - ), - ); - const testMessage = new TestMessage(m.message); - testMessage.location = location; - return testMessage; - }); + const testMessages = event.messages.map( + compilationMessageToTestMessage, + ); testRun.errored(testItem, testMessages, duration); } else { // Results didn't match expectations. Report the test as "failed". @@ -423,6 +433,12 @@ export class TestManager extends DisposableObject { // here. Any failed test needs at least one message. testMessages.push(new TestMessage("Test failed")); } + + // Add any warnings produced by the test to the test messages. + testMessages.push( + ...event.messages.map(compilationMessageToTestMessage), + ); + testRun.failed(testItem, testMessages, duration); } } From eaa432b9d7569c158f5f589df05b8bdbc64f0ed3 Mon Sep 17 00:00:00 2001 From: Koen Vlaswinkel Date: Wed, 28 Feb 2024 14:01:06 +0100 Subject: [PATCH 2/2] Add tests for new test warning behavior --- .../query-testing/test-adapter.test.ts | 28 +++++++++++++++++-- .../query-testing/test-runner-helpers.ts | 23 +++++++++++++++ .../query-testing/test-runner.test.ts | 23 ++++++++++++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-adapter.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-adapter.test.ts index ee7e6148c9f..0c8aa875b84 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-adapter.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-adapter.test.ts @@ -1,6 +1,7 @@ import type { TestItem, TestItemCollection, TestRun } from "vscode"; import { CancellationTokenSource, + Location, Range, TestRunRequest, Uri, @@ -75,6 +76,11 @@ describe("test-adapter", () => { id: `test ${mockTestsInfo.hPath}`, uri: Uri.file(mockTestsInfo.hPath), } as TestItem, + { + children: { size: 0 } as TestItemCollection, + id: `test ${mockTestsInfo.kPath}`, + uri: Uri.file(mockTestsInfo.kPath), + } as TestItem, ]; const childElements: IdTestItemPair[] = childItems.map((childItem) => [ childItem.id, @@ -87,7 +93,7 @@ describe("test-adapter", () => { id: `dir ${mockTestsInfo.testsPath}`, uri: Uri.file(mockTestsInfo.testsPath), children: { - size: 3, + size: 4, [Symbol.iterator]: childIteratorFunc, } as TestItemCollection, } as TestItem; @@ -95,7 +101,7 @@ describe("test-adapter", () => { const request = new TestRunRequest([rootItem]); await testManager.run(request, new CancellationTokenSource().token); - expect(enqueuedSpy).toHaveBeenCalledTimes(3); + expect(enqueuedSpy).toHaveBeenCalledTimes(4); expect(passedSpy).toHaveBeenCalledTimes(1); expect(passedSpy).toHaveBeenCalledWith(childItems[0], 3000); expect(erroredSpy).toHaveBeenCalledTimes(1); @@ -112,6 +118,7 @@ describe("test-adapter", () => { ], 4000, ); + expect(failedSpy).toHaveBeenCalledTimes(2); expect(failedSpy).toHaveBeenCalledWith( childItems[2], [ @@ -121,7 +128,22 @@ describe("test-adapter", () => { ], 11000, ); - expect(failedSpy).toHaveBeenCalledTimes(1); + expect(failedSpy).toHaveBeenCalledWith( + childItems[3], + [ + { + message: "Test failed", + }, + { + message: "abc", + location: new Location( + Uri.file(mockTestsInfo.kPath), + new Range(0, 0, 1, 1), + ), + }, + ], + 15000, + ); expect(endSpy).toHaveBeenCalledTimes(1); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner-helpers.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner-helpers.ts index afe5969cddc..8d52b9cb409 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner-helpers.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner-helpers.ts @@ -11,6 +11,7 @@ export const mockTestsInfo = { dPath: Uri.parse("file:/ab/c/d.ql").fsPath, gPath: Uri.parse("file:/ab/c/e/f/g.ql").fsPath, hPath: Uri.parse("file:/ab/c/e/f/h.ql").fsPath, + kPath: Uri.parse("file:/ab/c/e/f/k.ql").fsPath, }; /** @@ -89,6 +90,28 @@ function mockRunTests(): jest.Mock { evaluationMs: 6000, messages: [], }); + yield Promise.resolve({ + test: mockTestsInfo.kPath, + pass: false, + diff: ["jkh", "tuv"], + failureStage: "RESULT", + compilationMs: 7000, + evaluationMs: 8000, + // a warning in an otherwise successful test + messages: [ + { + position: { + fileName: mockTestsInfo.kPath, + line: 1, + column: 1, + endLine: 2, + endColumn: 2, + }, + message: "abc", + severity: "WARNING", + }, + ], + }); })(), ); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts index 837c28a3ce0..ea33088f87a 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-testing/test-runner.test.ts @@ -94,7 +94,7 @@ describe("test-runner", () => { eventHandlerSpy, ); - expect(eventHandlerSpy).toHaveBeenCalledTimes(3); + expect(eventHandlerSpy).toHaveBeenCalledTimes(4); expect(eventHandlerSpy).toHaveBeenNthCalledWith(1, { test: mockTestsInfo.dPath, @@ -133,6 +133,27 @@ describe("test-runner", () => { failureStage: "RESULT", messages: [], }); + expect(eventHandlerSpy).toHaveBeenNthCalledWith(4, { + test: mockTestsInfo.kPath, + pass: false, + compilationMs: 7000, + evaluationMs: 8000, + diff: ["jkh", "tuv"], + failureStage: "RESULT", + messages: [ + { + position: { + fileName: mockTestsInfo.kPath, + line: 1, + column: 1, + endLine: 2, + endColumn: 2, + }, + message: "abc", + severity: "WARNING", + }, + ], + }); }); it("should reregister testproj databases around test run", async () => {