From e682fcf586ddc9609bbdd6bd26345dedfd57c3e5 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Sun, 28 Sep 2025 14:01:02 -0400 Subject: [PATCH 1/4] Filter out incompatible additionalTestArgs when building tests for debugging When debugging tests we were appending `additionalTestArgs` to the build invocation. If the user has any incompatible flags that work for `swift test` but not `swift build`, this invocation will fail (i.e. `--no-parallel`). Filter down `additionalTestArgs` when performing the build to only `swift build` compatible arguments. All `additionalTestArgs` are still passed to the executable when tests are run. Issue: #1863 --- src/debugger/buildConfig.ts | 37 ++- src/toolchain/BuildFlags.ts | 80 ++++-- test/unit-tests/debugger/buildConfig.test.ts | 262 +++++++++++++++++++ 3 files changed, 353 insertions(+), 26 deletions(-) create mode 100644 test/unit-tests/debugger/buildConfig.test.ts diff --git a/src/debugger/buildConfig.ts b/src/debugger/buildConfig.ts index e326c84b2..35e9fee45 100644 --- a/src/debugger/buildConfig.ts +++ b/src/debugger/buildConfig.ts @@ -23,7 +23,7 @@ import { TestLibrary } from "../TestExplorer/TestRunner"; import configuration from "../configuration"; import { SwiftLogger } from "../logging/SwiftLogger"; import { buildOptions } from "../tasks/SwiftTaskProvider"; -import { BuildFlags } from "../toolchain/BuildFlags"; +import { ArgumentFilter, BuildFlags } from "../toolchain/BuildFlags"; import { packageName } from "../utilities/tasks"; import { regexEscapedString, swiftRuntimeEnv } from "../utilities/utilities"; import { Version } from "../utilities/version"; @@ -62,10 +62,13 @@ export class BuildConfigurationFactory { additionalArgs = [...additionalArgs, "-Xswiftc", "-enable-testing"]; } if (this.isTestBuild) { - additionalArgs = [ - ...additionalArgs, - ...configuration.folder(this.ctx.workspaceFolder).additionalTestArguments, - ]; + // Exclude all arguments from TEST_ONLY_ARGUMENTS that would cause a `swift build` to fail. + const buildCompatibleArgs = BuildFlags.filterArguments( + configuration.folder(this.ctx.workspaceFolder).additionalTestArguments, + BuildConfigurationFactory.TEST_ONLY_ARGUMENTS, + true + ); + additionalArgs = [...additionalArgs, ...buildCompatibleArgs]; } } @@ -99,6 +102,30 @@ export class BuildConfigurationFactory { private get baseConfig() { return getBaseConfig(this.ctx, true); } + + /** + * Arguments from additionalTestArguments that should be excluded from swift build commands. + * These are test-only arguments that would cause build failures if passed to swift build. + */ + private static TEST_ONLY_ARGUMENTS: ArgumentFilter[] = [ + { argument: "--parallel", include: 0 }, + { argument: "--no-parallel", include: 0 }, + { argument: "--num-workers", include: 1 }, + { argument: "--filter", include: 1 }, + { argument: "--skip", include: 1 }, + { argument: "-s", include: 1 }, + { argument: "--specifier", include: 1 }, + { argument: "-l", include: 0 }, + { argument: "--list-tests", include: 0 }, + { argument: "--show-codecov-path", include: 0 }, + { argument: "--show-code-coverage-path", include: 0 }, + { argument: "--show-coverage-path", include: 0 }, + { argument: "--xunit-output", include: 1 }, + { argument: "--enable-testable-imports", include: 0 }, + { argument: "--disable-testable-imports", include: 0 }, + { argument: "--attachments-path", include: 1 }, + { argument: "--skip-build", include: 0 }, + ]; } export class SwiftTestingBuildAguments { diff --git a/src/toolchain/BuildFlags.ts b/src/toolchain/BuildFlags.ts index 443d872db..466c32c4f 100644 --- a/src/toolchain/BuildFlags.ts +++ b/src/toolchain/BuildFlags.ts @@ -296,34 +296,72 @@ export class BuildFlags { } /** - * Filter argument list + * Filter argument list with support for both inclusion and exclusion logic * @param args argument list * @param filter argument list filter + * @param exclude if true, remove matching arguments (exclusion mode); if false, keep only matching arguments (inclusion mode) * @returns filtered argument list */ - static filterArguments(args: string[], filter: ArgumentFilter[]): string[] { - const filteredArguments: string[] = []; - let includeCount = 0; - for (const arg of args) { - if (includeCount > 0) { - filteredArguments.push(arg); - includeCount -= 1; - continue; - } - const argFilter = filter.find(item => item.argument === arg); - if (argFilter) { + static filterArguments(args: string[], filter: ArgumentFilter[], exclude = false): string[] { + if (exclude) { + // remove arguments that match the filter + const filteredArguments: string[] = []; + let skipCount = 0; + + for (const arg of args) { + if (skipCount > 0) { + // Skip this argument as it's a parameter to an excluded flag + skipCount -= 1; + continue; + } + + // Check if this is an excluded argument + const excludeFilter = filter.find(item => item.argument === arg); + if (excludeFilter) { + // Skip this argument and any parameters it takes + skipCount = excludeFilter.include; + continue; + } + + // Check for arguments of form --arg=value + const excludeFilter2 = filter.find( + item => item.include === 1 && arg.startsWith(item.argument + "=") + ); + if (excludeFilter2) { + // Skip this combined argument + continue; + } + + // This argument is not excluded, so include it filteredArguments.push(arg); - includeCount = argFilter.include; - continue; } - // find arguments of form arg=value - const argFilter2 = filter.find( - item => item.include === 1 && arg.startsWith(item.argument + "=") - ); - if (argFilter2) { - filteredArguments.push(arg); + + return filteredArguments; + } else { + // keep only arguments that match the filter + const filteredArguments: string[] = []; + let includeCount = 0; + for (const arg of args) { + if (includeCount > 0) { + filteredArguments.push(arg); + includeCount -= 1; + continue; + } + const argFilter = filter.find(item => item.argument === arg); + if (argFilter) { + filteredArguments.push(arg); + includeCount = argFilter.include; + continue; + } + // find arguments of form arg=value + const argFilter2 = filter.find( + item => item.include === 1 && arg.startsWith(item.argument + "=") + ); + if (argFilter2) { + filteredArguments.push(arg); + } } + return filteredArguments; } - return filteredArguments; } } diff --git a/test/unit-tests/debugger/buildConfig.test.ts b/test/unit-tests/debugger/buildConfig.test.ts new file mode 100644 index 000000000..65ec0e587 --- /dev/null +++ b/test/unit-tests/debugger/buildConfig.test.ts @@ -0,0 +1,262 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the VS Code Swift open source project +// +// Copyright (c) 2024 the VS Code Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of VS Code Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// +import { expect } from "chai"; +import * as sinon from "sinon"; + +import { FolderContext } from "@src/FolderContext"; +import { SwiftPackage } from "@src/SwiftPackage"; +import configuration, { FolderConfiguration } from "@src/configuration"; +import { BuildConfigurationFactory } from "@src/debugger/buildConfig"; +import { BuildFlags } from "@src/toolchain/BuildFlags"; +import { SwiftToolchain } from "@src/toolchain/toolchain"; +import { Version } from "@src/utilities/version"; + +import { MockedObject, instance, mockGlobalValue, mockObject } from "../../MockUtils"; + +suite("BuildConfig Test Suite", () => { + let mockedFolderContext: MockedObject; + let mockedSwiftPackage: MockedObject; + let mockedToolchain: MockedObject; + let mockedBuildFlags: MockedObject; + + const additionalTestArgumentsConfig = mockGlobalValue(configuration, "folder"); + + function createMockFolderConfig(additionalTestArguments: string[]): FolderConfiguration { + return { + testEnvironmentVariables: {}, + additionalTestArguments, + searchSubfoldersForPackages: false, + autoGenerateLaunchConfigurations: false, + disableAutoResolve: false, + attachmentsPath: "", + pluginPermissions: () => ({ trusted: false }), + pluginArguments: () => [], + } as FolderConfiguration; + } + + suiteSetup(() => { + mockedBuildFlags = mockObject({ + getDarwinTarget: () => undefined, + }); + + mockedToolchain = mockObject({ + buildFlags: instance(mockedBuildFlags), + swiftVersion: new Version(6, 0, 0), + sanitizer: () => undefined, + }); + + mockedSwiftPackage = mockObject({ + getTargets: sinon.stub().resolves([{ name: "TestTarget" }]), + name: Promise.resolve("TestPackage"), + }); + + mockedFolderContext = mockObject({ + toolchain: instance(mockedToolchain), + swiftPackage: instance(mockedSwiftPackage), + workspaceFolder: { uri: { fsPath: "/test/workspace" }, name: "TestWorkspace" } as any, + swiftVersion: new Version(6, 0, 0), + relativePath: "", + }); + }); + + suite("TEST_ONLY_ARGUMENTS filtering", () => { + let filterArgumentsSpy: sinon.SinonSpy; + + setup(() => { + // Reset any existing spies + sinon.restore(); + + // Spy on the BuildFlags.filterArguments method + filterArgumentsSpy = sinon.spy(BuildFlags, "filterArguments"); + + // Mock configuration.folder to return test arguments + additionalTestArgumentsConfig.setValue(() => createMockFolderConfig([])); + }); + + teardown(() => { + sinon.restore(); + }); + + test("filters out test-only arguments for test builds", async () => { + additionalTestArgumentsConfig.setValue(() => + createMockFolderConfig([ + "--no-parallel", + "--filter", + "TestCase", + "--enable-code-coverage", + ]) + ); + + const config = await BuildConfigurationFactory.buildAll( + instance(mockedFolderContext), + true, // isTestBuild + false // isRelease + ); + + expect(filterArgumentsSpy).to.have.been.calledOnce; + const [args] = filterArgumentsSpy.firstCall.args; + + expect(args).to.deep.equal([ + "--no-parallel", + "--filter", + "TestCase", + "--enable-code-coverage", + ]); + + expect(config.args).to.include("--build-tests"); + }); + + test("preserves build-compatible arguments for test builds", async () => { + additionalTestArgumentsConfig.setValue(() => + createMockFolderConfig([ + "--scratch-path", + "/tmp/build", + "-Xswiftc", + "-enable-testing", + ]) + ); + + // Act: Build configuration for test build + await BuildConfigurationFactory.buildAll( + instance(mockedFolderContext), + true, // isTestBuild + false // isRelease + ); + + expect(filterArgumentsSpy).to.have.been.calledOnce; + const [args] = filterArgumentsSpy.firstCall.args; + expect(args).to.deep.equal([ + "--scratch-path", + "/tmp/build", + "-Xswiftc", + "-enable-testing", + ]); + }); + + test("does not filter arguments for non-test builds", async () => { + additionalTestArgumentsConfig.setValue(() => + createMockFolderConfig(["--no-parallel", "--scratch-path", "/tmp/build"]) + ); + + await BuildConfigurationFactory.buildAll( + instance(mockedFolderContext), + false, // isTestBuild + false // isRelease + ); + + expect(filterArgumentsSpy).to.not.have.been.called; + }); + + test("handles empty additionalTestArguments", async () => { + additionalTestArgumentsConfig.setValue(() => createMockFolderConfig([])); + + await BuildConfigurationFactory.buildAll( + instance(mockedFolderContext), + true, // isTestBuild + false // isRelease + ); + + expect(filterArgumentsSpy).to.have.been.calledOnce; + const [args] = filterArgumentsSpy.firstCall.args; + expect(args).to.deep.equal([]); + }); + + test("handles mixed arguments correctly", async () => { + additionalTestArgumentsConfig.setValue(() => + createMockFolderConfig([ + "--no-parallel", // test-only (should be filtered out) + "--scratch-path", + "/tmp", // build-compatible (should be preserved) + "--filter", + "MyTest", // test-only (should be filtered out) + "-Xswiftc", + "-O", // build-compatible (should be preserved) + "--enable-code-coverage", // test-only (should be filtered out) + "--verbose", // build-compatible (should be preserved) + ]) + ); + + await BuildConfigurationFactory.buildAll( + instance(mockedFolderContext), + true, // isTestBuild + false // isRelease + ); + + expect(filterArgumentsSpy).to.have.been.calledOnce; + const [args] = filterArgumentsSpy.firstCall.args; + expect(args).to.deep.equal([ + "--no-parallel", + "--scratch-path", + "/tmp", + "--filter", + "MyTest", + "-Xswiftc", + "-O", + "--enable-code-coverage", + "--verbose", + ]); + }); + + test("has correct include values for arguments with parameters", () => { + // Access the private static property through the class + const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS; + + // Arguments that take 1 parameter + const oneParamArgs = ["--filter", "--skip", "--num-workers", "--xunit-output"]; + oneParamArgs.forEach(arg => { + const filterItem = filter.find((f: any) => f.argument === arg); + expect(filterItem, `${arg} should be in exclusion filter`).to.exist; + expect(filterItem.include, `${arg} should exclude 1 parameter`).to.equal(1); + }); + + // Arguments that take no parameters (flags) + const noParamArgs = ["--parallel", "--no-parallel", "--list-tests"]; + noParamArgs.forEach(arg => { + const filterItem = filter.find((f: any) => f.argument === arg); + expect(filterItem, `${arg} should be in exclusion filter`).to.exist; + expect(filterItem.include, `${arg} should exclude 0 parameters`).to.equal(0); + }); + }); + + test("excludes expected test-only arguments", () => { + // Access the private static property through the class + const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS; + + expect(filter).to.be.an("array"); + + // Verify test-only arguments are included in the exclusion list + expect(filter.some((f: any) => f.argument === "--no-parallel")).to.be.true; + expect(filter.some((f: any) => f.argument === "--parallel")).to.be.true; + expect(filter.some((f: any) => f.argument === "--filter")).to.be.true; + expect(filter.some((f: any) => f.argument === "--skip")).to.be.true; + expect(filter.some((f: any) => f.argument === "--list-tests")).to.be.true; + expect(filter.some((f: any) => f.argument === "--attachments-path")).to.be.true; + expect(filter.some((f: any) => f.argument === "--enable-testable-imports")).to.be.true; + expect(filter.some((f: any) => f.argument === "--xunit-output")).to.be.true; + }); + + test("does not exclude build-compatible arguments", () => { + // Access the private static property through the class + const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS; + + // Verify build-compatible arguments are NOT in the exclusion list + expect(filter.some((f: any) => f.argument === "--scratch-path")).to.be.false; + expect(filter.some((f: any) => f.argument === "-Xswiftc")).to.be.false; + expect(filter.some((f: any) => f.argument === "--build-system")).to.be.false; + expect(filter.some((f: any) => f.argument === "--sdk")).to.be.false; + expect(filter.some((f: any) => f.argument === "--verbose")).to.be.false; + expect(filter.some((f: any) => f.argument === "--configuration")).to.be.false; + }); + }); +}); From 72a6af8faa70f9aad4672f19842b2307ac8c5065 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 29 Sep 2025 09:27:47 -0400 Subject: [PATCH 2/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38307fd2e..bb9c3ce01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Fix an error when performing "Run/Debug Tests Multiple Times" on Linux ([#1824](https://github.com/swiftlang/vscode-swift/pull/1824)) - Fix the `> Swift: Run Swift Script` command not running unless a Swift Package folder is open ([#1832](https://github.com/swiftlang/vscode-swift/pull/1832)) - Fix the SourceKit-LSP diagnostics reported progress ([#1799](https://github.com/swiftlang/vscode-swift/pull/1799)) +- Omit incompatible `additionalTestArgs` when building tests for debugging ([#1864](https://github.com/swiftlang/vscode-swift/pull/1864)) ## 2.11.20250806 - 2025-08-06 From e00461cca57fdaceb40d2bdadc79b789042054bb Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Mon, 29 Sep 2025 10:39:22 -0400 Subject: [PATCH 3/4] Mock missing property on Linux/windows --- test/unit-tests/debugger/buildConfig.test.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit-tests/debugger/buildConfig.test.ts b/test/unit-tests/debugger/buildConfig.test.ts index 65ec0e587..ec6af2610 100644 --- a/test/unit-tests/debugger/buildConfig.test.ts +++ b/test/unit-tests/debugger/buildConfig.test.ts @@ -15,6 +15,7 @@ import { expect } from "chai"; import * as sinon from "sinon"; import { FolderContext } from "@src/FolderContext"; +import { LinuxMain } from "@src/LinuxMain"; import { SwiftPackage } from "@src/SwiftPackage"; import configuration, { FolderConfiguration } from "@src/configuration"; import { BuildConfigurationFactory } from "@src/debugger/buildConfig"; @@ -67,6 +68,9 @@ suite("BuildConfig Test Suite", () => { workspaceFolder: { uri: { fsPath: "/test/workspace" }, name: "TestWorkspace" } as any, swiftVersion: new Version(6, 0, 0), relativePath: "", + linuxMain: { + exists: true, + } as any as LinuxMain, }); }); From e3cd382270f83337f65b0e5e78c71716ace14e4f Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Tue, 30 Sep 2025 11:37:38 -0400 Subject: [PATCH 4/4] Simplify filterArguments --- src/toolchain/BuildFlags.ts | 81 ++++++++++++++----------------------- 1 file changed, 31 insertions(+), 50 deletions(-) diff --git a/src/toolchain/BuildFlags.ts b/src/toolchain/BuildFlags.ts index 466c32c4f..e4e870db7 100644 --- a/src/toolchain/BuildFlags.ts +++ b/src/toolchain/BuildFlags.ts @@ -303,65 +303,46 @@ export class BuildFlags { * @returns filtered argument list */ static filterArguments(args: string[], filter: ArgumentFilter[], exclude = false): string[] { - if (exclude) { - // remove arguments that match the filter - const filteredArguments: string[] = []; - let skipCount = 0; + const filteredArguments: string[] = []; + let pendingCount = 0; - for (const arg of args) { - if (skipCount > 0) { - // Skip this argument as it's a parameter to an excluded flag - skipCount -= 1; - continue; - } - - // Check if this is an excluded argument - const excludeFilter = filter.find(item => item.argument === arg); - if (excludeFilter) { - // Skip this argument and any parameters it takes - skipCount = excludeFilter.include; - continue; - } - - // Check for arguments of form --arg=value - const excludeFilter2 = filter.find( - item => item.include === 1 && arg.startsWith(item.argument + "=") - ); - if (excludeFilter2) { - // Skip this combined argument - continue; + for (const arg of args) { + if (pendingCount > 0) { + if (!exclude) { + filteredArguments.push(arg); } - - // This argument is not excluded, so include it - filteredArguments.push(arg); + pendingCount -= 1; + continue; } - return filteredArguments; - } else { - // keep only arguments that match the filter - const filteredArguments: string[] = []; - let includeCount = 0; - for (const arg of args) { - if (includeCount > 0) { + // Check if this argument matches any filter + const matchingFilter = filter.find(item => item.argument === arg); + if (matchingFilter) { + if (!exclude) { filteredArguments.push(arg); - includeCount -= 1; - continue; } - const argFilter = filter.find(item => item.argument === arg); - if (argFilter) { - filteredArguments.push(arg); - includeCount = argFilter.include; - continue; - } - // find arguments of form arg=value - const argFilter2 = filter.find( - item => item.include === 1 && arg.startsWith(item.argument + "=") - ); - if (argFilter2) { + pendingCount = matchingFilter.include; + continue; + } + + // Check for arguments of form --arg=value (only for filters with include=1) + const combinedArgFilter = filter.find( + item => item.include === 1 && arg.startsWith(item.argument + "=") + ); + if (combinedArgFilter) { + if (!exclude) { filteredArguments.push(arg); } + continue; } - return filteredArguments; + + // Handle unmatched arguments + if (exclude) { + filteredArguments.push(arg); + } + // In include mode, unmatched arguments are not added } + + return filteredArguments; } }