From de4b4e2932435b84537a04bbc8f403cfab7ecef0 Mon Sep 17 00:00:00 2001 From: Adam Ward Date: Thu, 20 Feb 2025 10:35:08 -0500 Subject: [PATCH] Automatically teardown settings for diagnotics tests Moved these tests over to the fixture to make teardown less error --- assets/test/.vscode/settings.json | 4 +- src/configuration.ts | 7 +- .../DiagnosticsManager.test.ts | 441 +++++++++--------- 3 files changed, 229 insertions(+), 223 deletions(-) diff --git a/assets/test/.vscode/settings.json b/assets/test/.vscode/settings.json index 4a09afde5..99dc3219d 100644 --- a/assets/test/.vscode/settings.json +++ b/assets/test/.vscode/settings.json @@ -7,5 +7,7 @@ "-DTEST_ARGUMENT_SET_VIA_TEST_BUILD_ARGUMENTS_SETTING" ], "lldb.verboseLogging": true, - "swift.backgroundCompilation": false + "swift.backgroundCompilation": false, + "swift.diagnosticsStyle": "llvm", + "swift.diagnosticsCollection": "onlySwiftc" } \ No newline at end of file diff --git a/src/configuration.ts b/src/configuration.ts index 5f7abdf29..85c88bee9 100644 --- a/src/configuration.ts +++ b/src/configuration.ts @@ -29,6 +29,7 @@ export type DiagnosticCollectionOptions = | "keepSwiftc" | "keepSourceKit" | "keepAll"; +export type DiagnosticStyle = "default" | "llvm" | "swift"; /** sourcekit-lsp configuration */ export interface LSPConfiguration { @@ -288,8 +289,10 @@ const configuration = { .get("diagnosticsCollection", "keepSourceKit"); }, /** set the -diagnostic-style option when running `swift` tasks */ - get diagnosticsStyle(): "default" | "llvm" | "swift" { - return vscode.workspace.getConfiguration("swift").get("diagnosticsStyle", "llvm"); + get diagnosticsStyle(): DiagnosticStyle { + return vscode.workspace + .getConfiguration("swift") + .get("diagnosticsStyle", "llvm"); }, /** where to show the build progress for the running task */ get showBuildStatus(): ShowBuildStatusOptions { diff --git a/test/integration-tests/DiagnosticsManager.test.ts b/test/integration-tests/DiagnosticsManager.test.ts index b0a783090..9d14cfa5b 100644 --- a/test/integration-tests/DiagnosticsManager.test.ts +++ b/test/integration-tests/DiagnosticsManager.test.ts @@ -23,7 +23,12 @@ import { DiagnosticsManager } from "../../src/DiagnosticsManager"; import { FolderContext } from "../../src/FolderContext"; import { Version } from "../../src/utilities/version"; import { Workbench } from "../../src/utilities/commands"; -import { activateExtensionForSuite, folderInRootWorkspace } from "./utilities/testutilities"; +import { + activateExtensionForSuite, + folderInRootWorkspace, + updateSettings, +} from "./utilities/testutilities"; +import { DiagnosticStyle } from "../../src/configuration"; const isEqual = (d1: vscode.Diagnostic, d2: vscode.Diagnostic) => { return ( @@ -61,8 +66,6 @@ suite("DiagnosticsManager Test Suite", async function () { // Was hitting a timeout in suiteSetup during CI build once in a while this.timeout(5000); - const swiftConfig = vscode.workspace.getConfiguration("swift"); - let workspaceContext: WorkspaceContext; let folderContext: FolderContext; let cFolderContext: FolderContext; @@ -85,7 +88,8 @@ suite("DiagnosticsManager Test Suite", async function () { | undefined; // Wait for all the expected diagnostics to be recieved. This may happen over several `onChangeDiagnostics` events. - const waitForDiagnostics = (expectedDiagnostics: { [uri: string]: vscode.Diagnostic[] }) => { + type ExpectedDiagnostics = { [uri: string]: vscode.Diagnostic[] }; + const waitForDiagnostics = (expectedDiagnostics: ExpectedDiagnostics) => { return new Promise(resolve => { if (diagnosticWaiterDisposable) { console.warn( @@ -212,208 +216,200 @@ suite("DiagnosticsManager Test Suite", async function () { }, ]; - // SourceKit-LSP sometimes sends diagnostics - // after first build and can cause intermittent - // failure if `swiftc` diagnostic is fixed - suiteSetup(async function () { - this.timeout(3 * 60 * 1000); // Allow 3 minutes to build - const task = createBuildAllTask(folderContext); - // This return exit code and output for the task but we will omit it here - // because the failures are expected and we just want the task to build - await executeTaskAndWaitForResult(task); - }); - - suiteTeardown(async () => { - await swiftConfig.update("diagnosticsStyle", undefined); - }); - - test("default diagnosticsStyle", async function () { - // Swift 5.10 and 6.0 on Windows have a bug where the - // diagnostics are not emitted on their own line. - const swiftVersion = workspaceContext.toolchain.swiftVersion; - if ( - process.platform === "win32" && - swiftVersion.isGreaterThanOrEqual(new Version(5, 10, 0)) && - swiftVersion.isLessThanOrEqual(new Version(6, 0, 999)) - ) { - this.skip(); - } - await swiftConfig.update("diagnosticsStyle", "default"); - - await Promise.all([ - waitForDiagnostics({ - [mainUri.fsPath]: [ - expectedWarningDiagnostic, - expectedMainErrorDiagnostic, - expectedMainDictErrorDiagnostic, - ...(workspaceContext.swiftVersion.isGreaterThanOrEqual( - new Version(6, 0, 0) - ) - ? [expectedMacroDiagnostic] - : []), - ], // Should have parsed correct severity - [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file - }), - executeTaskAndWaitForResult(createBuildAllTask(folderContext)), - ]); - - await waitForNoRunningTasks(); - }); - - test("swift diagnosticsStyle", async function () { - // This is only supported in swift versions >=5.10.0. - // Swift 5.10 and 6.0 on Windows have a bug where the - // diagnostics are not emitted on their own line. - const swiftVersion = workspaceContext.toolchain.swiftVersion; - if ( - swiftVersion.isLessThan(new Version(5, 10, 0)) || - (process.platform === "win32" && - swiftVersion.isGreaterThanOrEqual(new Version(5, 10, 0)) && - swiftVersion.isLessThanOrEqual(new Version(6, 0, 999))) - ) { - this.skip(); - } - await swiftConfig.update("diagnosticsStyle", "swift"); - - await Promise.all([ - waitForDiagnostics({ - [mainUri.fsPath]: [ - expectedWarningDiagnostic, - expectedMainErrorDiagnostic, - expectedMainDictErrorDiagnostic, - ], // Should have parsed correct severity - [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file - }), - executeTaskAndWaitForResult(createBuildAllTask(folderContext)), - ]); - await waitForNoRunningTasks(); - }); - - test("llvm diagnosticsStyle", async () => { - await swiftConfig.update("diagnosticsStyle", "llvm"); - - await Promise.all([ - waitForDiagnostics({ - [mainUri.fsPath]: [ - expectedWarningDiagnostic, - expectedMainErrorDiagnostic, - expectedMainDictErrorDiagnostic, - ], // Should have parsed correct severity - [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file - }), - executeTaskAndWaitForResult(createBuildAllTask(folderContext)), - ]); - await waitForNoRunningTasks(); + function runTestDiagnosticStyle( + style: DiagnosticStyle, + expected: () => ExpectedDiagnostics, + callback?: () => void + ) { + suite(`${style} diagnosticsStyle`, async function () { + // SourceKit-LSP sometimes sends diagnostics + // after first build and can cause intermittent + // failure if `swiftc` diagnostic is fixed + suiteSetup(async function () { + // Swift 5.10 and 6.0 on Windows have a bug where the + // diagnostics are not emitted on their own line. + const swiftVersion = workspaceContext.toolchain.swiftVersion; + if ( + process.platform === "win32" && + swiftVersion.isGreaterThanOrEqual(new Version(5, 10, 0)) && + swiftVersion.isLessThanOrEqual(new Version(6, 0, 999)) + ) { + this.skip(); + } + this.timeout(3 * 60 * 1000); // Allow 3 minutes to build + + // Clean up any lingering diagnostics + workspaceContext.diagnostics.clear(); + workspaceContext.focusFolder(null); + + const teardown = await updateSettings({ "swift.diagnosticsStyle": style }); + const task = createBuildAllTask(folderContext); + // This return exit code and output for the task but we will omit it here + // because the failures are expected and we just want the task to build + await executeTaskAndWaitForResult(task).catch(() => { + /* Ignore */ + }); + return teardown; + }); - // Should have parsed severity - const diagnostic = assertHasDiagnostic(mainUri, expectedMainErrorDiagnostic); - // Should have parsed related note - assert.equal(diagnostic.relatedInformation?.length, 1); - assert.equal( - diagnostic.relatedInformation![0].message, - "Change 'let' to 'var' to make it mutable" - ); - assert.equal(diagnostic.relatedInformation![0].location.uri.fsPath, mainUri.fsPath); - assert.equal( - diagnostic.relatedInformation![0].location.range.isEqual( - new vscode.Range(new vscode.Position(6, 0), new vscode.Position(6, 0)) - ), - true - ); - }); + test("succeeds", async function () { + await Promise.all([ + waitForDiagnostics(expected()), + executeTaskAndWaitForResult(createBuildAllTask(folderContext)), + ]); + await waitForNoRunningTasks(); + }); - test("Parses C diagnostics", async function () { - const swiftVersion = workspaceContext.toolchain.swiftVersion; - // SPM will sometimes improperly clear diagnostics from the terminal, leading - // to a flakey test. - if (swiftVersion.isLessThan(new Version(5, 7, 0))) { - this.skip(); - } + callback && callback(); + }); + } - await swiftConfig.update("diagnosticsStyle", "llvm"); + runTestDiagnosticStyle("default", () => ({ + [mainUri.fsPath]: [ + expectedWarningDiagnostic, + expectedMainErrorDiagnostic, + expectedMainDictErrorDiagnostic, + ...(workspaceContext.swiftVersion.isGreaterThanOrEqual(new Version(6, 0, 0)) + ? [expectedMacroDiagnostic] + : []), + ], // Should have parsed correct severity + [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file + })); + + runTestDiagnosticStyle("swift", () => ({ + [mainUri.fsPath]: [ + expectedWarningDiagnostic, + expectedMainErrorDiagnostic, + expectedMainDictErrorDiagnostic, + ], // Should have parsed correct severity + [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file + })); + + runTestDiagnosticStyle( + "llvm", + () => ({ + [mainUri.fsPath]: [ + expectedWarningDiagnostic, + expectedMainErrorDiagnostic, + expectedMainDictErrorDiagnostic, + ], // Should have parsed correct severity + [funcUri.fsPath]: [expectedFuncErrorDiagnostic], // Check parsed for other file + }), + () => { + test("Parses related information", async () => { + const diagnostic = assertHasDiagnostic( + mainUri, + expectedMainErrorDiagnostic + ); + // Should have parsed related note + assert.equal(diagnostic.relatedInformation?.length, 1); + assert.equal( + diagnostic.relatedInformation![0].message, + "Change 'let' to 'var' to make it mutable" + ); + assert.equal( + diagnostic.relatedInformation![0].location.uri.fsPath, + mainUri.fsPath + ); + assert.equal( + diagnostic.relatedInformation![0].location.range.isEqual( + new vscode.Range( + new vscode.Position(6, 0), + new vscode.Position(6, 0) + ) + ), + true + ); + }); - // Should have parsed severity - const expectedDiagnostic1 = new vscode.Diagnostic( - new vscode.Range(new vscode.Position(5, 10), new vscode.Position(5, 10)), - "Use of undeclared identifier 'bar'", - vscode.DiagnosticSeverity.Error - ); - expectedDiagnostic1.source = "swiftc"; - const expectedDiagnostic2 = new vscode.Diagnostic( - new vscode.Range(new vscode.Position(6, 6), new vscode.Position(6, 6)), - "No member named 'z' in 'struct MyPoint'", - vscode.DiagnosticSeverity.Error - ); - expectedDiagnostic2.source = "swiftc"; - - await Promise.all([ - waitForDiagnostics({ - [cUri.fsPath]: [expectedDiagnostic1, expectedDiagnostic2], - }), - executeTaskAndWaitForResult(createBuildAllTask(cFolderContext)), - ]); - await waitForNoRunningTasks(); - }); + test("Parses C diagnostics", async function () { + // Should have parsed severity + const expectedDiagnostic1 = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(5, 10), + new vscode.Position(5, 10) + ), + "Use of undeclared identifier 'bar'", + vscode.DiagnosticSeverity.Error + ); + expectedDiagnostic1.source = "swiftc"; + const expectedDiagnostic2 = new vscode.Diagnostic( + new vscode.Range(new vscode.Position(6, 6), new vscode.Position(6, 6)), + "No member named 'z' in 'struct MyPoint'", + vscode.DiagnosticSeverity.Error + ); + expectedDiagnostic2.source = "swiftc"; + + await Promise.all([ + waitForDiagnostics({ + [cUri.fsPath]: [expectedDiagnostic1, expectedDiagnostic2], + }), + executeTaskAndWaitForResult(createBuildAllTask(cFolderContext)), + ]); + await waitForNoRunningTasks(); + }); - test("Parses C++ diagnostics", async function () { - const swiftVersion = workspaceContext.toolchain.swiftVersion; - // SPM will sometimes improperly clear diagnostics from the terminal, leading - // to a flakey test. - if (swiftVersion.isLessThan(new Version(5, 7, 0))) { - this.skip(); + test("Parses C++ diagnostics", async function () { + // Should have parsed severity + const expectedDiagnostic1 = new vscode.Diagnostic( + new vscode.Range(new vscode.Position(6, 5), new vscode.Position(6, 5)), + "Member reference type 'MyPoint *' is a pointer; did you mean to use '->'?", + vscode.DiagnosticSeverity.Error + ); + expectedDiagnostic1.source = "swiftc"; + + // Should have parsed releated information + const expectedDiagnostic2 = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(3, 21), + new vscode.Position(3, 21) + ), + "Unknown type name 'MyPoint2'; did you mean 'MyPoint'?", + vscode.DiagnosticSeverity.Error + ); + expectedDiagnostic2.source = "swiftc"; + + // Message should not contain [-Wreturn-mismatch] so it can be merged with + // SourceKit diagnostics if required + const expectedDiagnostic3 = new vscode.Diagnostic( + new vscode.Range( + new vscode.Position(11, 4), + new vscode.Position(11, 4) + ), + "Non-void function 'main' should return a value", + vscode.DiagnosticSeverity.Error + ); + expectedDiagnostic3.source = "swiftc"; + + await Promise.all([ + waitForDiagnostics({ + [cppUri.fsPath]: [ + expectedDiagnostic1, + expectedDiagnostic2, + expectedDiagnostic3, + ], + }), + executeTaskAndWaitForResult(createBuildAllTask(cppFolderContext)), + ]); + await waitForNoRunningTasks(); + + const diagnostic = assertHasDiagnostic(cppUri, expectedDiagnostic2); + assert.equal( + diagnostic.relatedInformation![0].location.uri.fsPath, + cppHeaderUri.fsPath + ); + assert.equal( + diagnostic.relatedInformation![0].location.range.isEqual( + new vscode.Range( + new vscode.Position(0, 6), + new vscode.Position(0, 6) + ) + ), + true + ); + }); } - - await swiftConfig.update("diagnosticsStyle", "llvm"); - - // Should have parsed severity - const expectedDiagnostic1 = new vscode.Diagnostic( - new vscode.Range(new vscode.Position(6, 5), new vscode.Position(6, 5)), - "Member reference type 'MyPoint *' is a pointer; did you mean to use '->'?", - vscode.DiagnosticSeverity.Error - ); - expectedDiagnostic1.source = "swiftc"; - - // Should have parsed releated information - const expectedDiagnostic2 = new vscode.Diagnostic( - new vscode.Range(new vscode.Position(3, 21), new vscode.Position(3, 21)), - "Unknown type name 'MyPoint2'; did you mean 'MyPoint'?", - vscode.DiagnosticSeverity.Error - ); - expectedDiagnostic2.source = "swiftc"; - - // Message should not contain [-Wreturn-mismatch] so it can be merged with - // SourceKit diagnostics if required - const expectedDiagnostic3 = new vscode.Diagnostic( - new vscode.Range(new vscode.Position(11, 4), new vscode.Position(11, 4)), - "Non-void function 'main' should return a value", - vscode.DiagnosticSeverity.Error - ); - expectedDiagnostic3.source = "swiftc"; - - await Promise.all([ - waitForDiagnostics({ - [cppUri.fsPath]: [ - expectedDiagnostic1, - expectedDiagnostic2, - expectedDiagnostic3, - ], - }), - executeTaskAndWaitForResult(createBuildAllTask(cppFolderContext)), - ]); - await waitForNoRunningTasks(); - - const diagnostic = assertHasDiagnostic(cppUri, expectedDiagnostic2); - assert.equal( - diagnostic.relatedInformation![0].location.uri.fsPath, - cppHeaderUri.fsPath - ); - assert.equal( - diagnostic.relatedInformation![0].location.range.isEqual( - new vscode.Range(new vscode.Position(0, 6), new vscode.Position(0, 6)) - ), - true - ); - }); + ); }); suite("Controlled output", () => { @@ -550,14 +546,11 @@ suite("DiagnosticsManager Test Suite", async function () { sourcekitWarningDiagnostic.source = "SourceKit"; }); - suiteTeardown(async () => { - // So test asset settings.json doesn't changedq - await swiftConfig.update("diagnosticsCollection", undefined); - }); - suite("keepAll", () => { - setup(async () => { - await swiftConfig.update("diagnosticsCollection", "keepAll"); + suiteSetup(async function () { + return await updateSettings({ + "swift.diagnosticsCollection": "keepAll", + }); }); test("merge in SourceKit diagnostics", async () => { @@ -604,8 +597,10 @@ suite("DiagnosticsManager Test Suite", async function () { }); suite("keepSourceKit", () => { - setup(async () => { - await swiftConfig.update("diagnosticsCollection", "keepSourceKit"); + suiteSetup(async function () { + return await updateSettings({ + "swift.diagnosticsCollection": "keepSourceKit", + }); }); test("merge in SourceKit diagnostics", async () => { @@ -716,8 +711,10 @@ suite("DiagnosticsManager Test Suite", async function () { }); suite("keepSwiftc", () => { - setup(async () => { - await swiftConfig.update("diagnosticsCollection", "keepSwiftc"); + suiteSetup(async function () { + return await updateSettings({ + "swift.diagnosticsCollection": "keepSwiftc", + }); }); test("merge in swiftc diagnostics", async () => { @@ -804,8 +801,10 @@ suite("DiagnosticsManager Test Suite", async function () { }); suite("onlySourceKit", () => { - setup(async () => { - await swiftConfig.update("diagnosticsCollection", "onlySourceKit"); + suiteSetup(async function () { + return await updateSettings({ + "swift.diagnosticsCollection": "onlySourceKit", + }); }); test("merge in SourceKit diagnostics", async () => { @@ -844,6 +843,7 @@ suite("DiagnosticsManager Test Suite", async function () { }); test("clean old swiftc diagnostics", async () => { + const swiftConfig = vscode.workspace.getConfiguration("swift"); // Add initial swiftc diagnostics await swiftConfig.update("diagnosticsCollection", "keepAll"); workspaceContext.diagnostics.handleDiagnostics( @@ -871,8 +871,10 @@ suite("DiagnosticsManager Test Suite", async function () { }); suite("onlySwiftc", () => { - setup(async () => { - await swiftConfig.update("diagnosticsCollection", "onlySwiftc"); + suiteSetup(async function () { + return await updateSettings({ + "swift.diagnosticsCollection": "onlySwiftc", + }); }); test("merge in swiftc diagnostics", async () => { @@ -911,6 +913,7 @@ suite("DiagnosticsManager Test Suite", async function () { }); test("clean old SourceKit diagnostics", async () => { + const swiftConfig = vscode.workspace.getConfiguration("swift"); // Add initial SourceKit diagnostics await swiftConfig.update("diagnosticsCollection", "keepAll"); workspaceContext.diagnostics.handleDiagnostics( @@ -1012,11 +1015,9 @@ suite("DiagnosticsManager Test Suite", async function () { } workspaceContext.diagnostics.clear(); workspaceContext.focusFolder(null); - await swiftConfig.update("diagnosticsCollection", "onlySourceKit"); // So waitForDiagnostics only resolves from LSP - }); - - suiteTeardown(async () => { - await swiftConfig.update("diagnosticsCollection", undefined); + return await updateSettings({ + diagnosticsCollection: "onlySourceKit", // So waitForDiagnostics only resolves from LSP + }); }); teardown(async () => {