Skip to content

Commit 568270b

Browse files
authored
Filter out incompatible additionalTestArgs when building tests for debugging (#1864)
* 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
1 parent fa61d31 commit 568270b

File tree

4 files changed

+331
-18
lines changed

4 files changed

+331
-18
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Fix an error when performing "Run/Debug Tests Multiple Times" on Linux ([#1824](https://github.com/swiftlang/vscode-swift/pull/1824))
1717
- 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))
1818
- Fix the SourceKit-LSP diagnostics reported progress ([#1799](https://github.com/swiftlang/vscode-swift/pull/1799))
19+
- Omit incompatible `additionalTestArgs` when building tests for debugging ([#1864](https://github.com/swiftlang/vscode-swift/pull/1864))
1920

2021
## 2.11.20250806 - 2025-08-06
2122

src/debugger/buildConfig.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { TestLibrary } from "../TestExplorer/TestRunner";
2323
import configuration from "../configuration";
2424
import { SwiftLogger } from "../logging/SwiftLogger";
2525
import { buildOptions } from "../tasks/SwiftTaskProvider";
26-
import { BuildFlags } from "../toolchain/BuildFlags";
26+
import { ArgumentFilter, BuildFlags } from "../toolchain/BuildFlags";
2727
import { packageName } from "../utilities/tasks";
2828
import { regexEscapedString, swiftRuntimeEnv } from "../utilities/utilities";
2929
import { Version } from "../utilities/version";
@@ -62,10 +62,13 @@ export class BuildConfigurationFactory {
6262
additionalArgs = [...additionalArgs, "-Xswiftc", "-enable-testing"];
6363
}
6464
if (this.isTestBuild) {
65-
additionalArgs = [
66-
...additionalArgs,
67-
...configuration.folder(this.ctx.workspaceFolder).additionalTestArguments,
68-
];
65+
// Exclude all arguments from TEST_ONLY_ARGUMENTS that would cause a `swift build` to fail.
66+
const buildCompatibleArgs = BuildFlags.filterArguments(
67+
configuration.folder(this.ctx.workspaceFolder).additionalTestArguments,
68+
BuildConfigurationFactory.TEST_ONLY_ARGUMENTS,
69+
true
70+
);
71+
additionalArgs = [...additionalArgs, ...buildCompatibleArgs];
6972
}
7073
}
7174

@@ -99,6 +102,30 @@ export class BuildConfigurationFactory {
99102
private get baseConfig() {
100103
return getBaseConfig(this.ctx, true);
101104
}
105+
106+
/**
107+
* Arguments from additionalTestArguments that should be excluded from swift build commands.
108+
* These are test-only arguments that would cause build failures if passed to swift build.
109+
*/
110+
private static TEST_ONLY_ARGUMENTS: ArgumentFilter[] = [
111+
{ argument: "--parallel", include: 0 },
112+
{ argument: "--no-parallel", include: 0 },
113+
{ argument: "--num-workers", include: 1 },
114+
{ argument: "--filter", include: 1 },
115+
{ argument: "--skip", include: 1 },
116+
{ argument: "-s", include: 1 },
117+
{ argument: "--specifier", include: 1 },
118+
{ argument: "-l", include: 0 },
119+
{ argument: "--list-tests", include: 0 },
120+
{ argument: "--show-codecov-path", include: 0 },
121+
{ argument: "--show-code-coverage-path", include: 0 },
122+
{ argument: "--show-coverage-path", include: 0 },
123+
{ argument: "--xunit-output", include: 1 },
124+
{ argument: "--enable-testable-imports", include: 0 },
125+
{ argument: "--disable-testable-imports", include: 0 },
126+
{ argument: "--attachments-path", include: 1 },
127+
{ argument: "--skip-build", include: 0 },
128+
];
102129
}
103130

104131
export class SwiftTestingBuildAguments {

src/toolchain/BuildFlags.ts

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -296,34 +296,53 @@ export class BuildFlags {
296296
}
297297

298298
/**
299-
* Filter argument list
299+
* Filter argument list with support for both inclusion and exclusion logic
300300
* @param args argument list
301301
* @param filter argument list filter
302+
* @param exclude if true, remove matching arguments (exclusion mode); if false, keep only matching arguments (inclusion mode)
302303
* @returns filtered argument list
303304
*/
304-
static filterArguments(args: string[], filter: ArgumentFilter[]): string[] {
305+
static filterArguments(args: string[], filter: ArgumentFilter[], exclude = false): string[] {
305306
const filteredArguments: string[] = [];
306-
let includeCount = 0;
307+
let pendingCount = 0;
308+
307309
for (const arg of args) {
308-
if (includeCount > 0) {
309-
filteredArguments.push(arg);
310-
includeCount -= 1;
310+
if (pendingCount > 0) {
311+
if (!exclude) {
312+
filteredArguments.push(arg);
313+
}
314+
pendingCount -= 1;
311315
continue;
312316
}
313-
const argFilter = filter.find(item => item.argument === arg);
314-
if (argFilter) {
315-
filteredArguments.push(arg);
316-
includeCount = argFilter.include;
317+
318+
// Check if this argument matches any filter
319+
const matchingFilter = filter.find(item => item.argument === arg);
320+
if (matchingFilter) {
321+
if (!exclude) {
322+
filteredArguments.push(arg);
323+
}
324+
pendingCount = matchingFilter.include;
317325
continue;
318326
}
319-
// find arguments of form arg=value
320-
const argFilter2 = filter.find(
327+
328+
// Check for arguments of form --arg=value (only for filters with include=1)
329+
const combinedArgFilter = filter.find(
321330
item => item.include === 1 && arg.startsWith(item.argument + "=")
322331
);
323-
if (argFilter2) {
332+
if (combinedArgFilter) {
333+
if (!exclude) {
334+
filteredArguments.push(arg);
335+
}
336+
continue;
337+
}
338+
339+
// Handle unmatched arguments
340+
if (exclude) {
324341
filteredArguments.push(arg);
325342
}
343+
// In include mode, unmatched arguments are not added
326344
}
345+
327346
return filteredArguments;
328347
}
329348
}
Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,266 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the VS Code Swift open source project
4+
//
5+
// Copyright (c) 2024 the VS Code Swift project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of VS Code Swift project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
import { expect } from "chai";
15+
import * as sinon from "sinon";
16+
17+
import { FolderContext } from "@src/FolderContext";
18+
import { LinuxMain } from "@src/LinuxMain";
19+
import { SwiftPackage } from "@src/SwiftPackage";
20+
import configuration, { FolderConfiguration } from "@src/configuration";
21+
import { BuildConfigurationFactory } from "@src/debugger/buildConfig";
22+
import { BuildFlags } from "@src/toolchain/BuildFlags";
23+
import { SwiftToolchain } from "@src/toolchain/toolchain";
24+
import { Version } from "@src/utilities/version";
25+
26+
import { MockedObject, instance, mockGlobalValue, mockObject } from "../../MockUtils";
27+
28+
suite("BuildConfig Test Suite", () => {
29+
let mockedFolderContext: MockedObject<FolderContext>;
30+
let mockedSwiftPackage: MockedObject<SwiftPackage>;
31+
let mockedToolchain: MockedObject<SwiftToolchain>;
32+
let mockedBuildFlags: MockedObject<BuildFlags>;
33+
34+
const additionalTestArgumentsConfig = mockGlobalValue(configuration, "folder");
35+
36+
function createMockFolderConfig(additionalTestArguments: string[]): FolderConfiguration {
37+
return {
38+
testEnvironmentVariables: {},
39+
additionalTestArguments,
40+
searchSubfoldersForPackages: false,
41+
autoGenerateLaunchConfigurations: false,
42+
disableAutoResolve: false,
43+
attachmentsPath: "",
44+
pluginPermissions: () => ({ trusted: false }),
45+
pluginArguments: () => [],
46+
} as FolderConfiguration;
47+
}
48+
49+
suiteSetup(() => {
50+
mockedBuildFlags = mockObject<BuildFlags>({
51+
getDarwinTarget: () => undefined,
52+
});
53+
54+
mockedToolchain = mockObject<SwiftToolchain>({
55+
buildFlags: instance(mockedBuildFlags),
56+
swiftVersion: new Version(6, 0, 0),
57+
sanitizer: () => undefined,
58+
});
59+
60+
mockedSwiftPackage = mockObject<SwiftPackage>({
61+
getTargets: sinon.stub().resolves([{ name: "TestTarget" }]),
62+
name: Promise.resolve("TestPackage"),
63+
});
64+
65+
mockedFolderContext = mockObject<FolderContext>({
66+
toolchain: instance(mockedToolchain),
67+
swiftPackage: instance(mockedSwiftPackage),
68+
workspaceFolder: { uri: { fsPath: "/test/workspace" }, name: "TestWorkspace" } as any,
69+
swiftVersion: new Version(6, 0, 0),
70+
relativePath: "",
71+
linuxMain: {
72+
exists: true,
73+
} as any as LinuxMain,
74+
});
75+
});
76+
77+
suite("TEST_ONLY_ARGUMENTS filtering", () => {
78+
let filterArgumentsSpy: sinon.SinonSpy;
79+
80+
setup(() => {
81+
// Reset any existing spies
82+
sinon.restore();
83+
84+
// Spy on the BuildFlags.filterArguments method
85+
filterArgumentsSpy = sinon.spy(BuildFlags, "filterArguments");
86+
87+
// Mock configuration.folder to return test arguments
88+
additionalTestArgumentsConfig.setValue(() => createMockFolderConfig([]));
89+
});
90+
91+
teardown(() => {
92+
sinon.restore();
93+
});
94+
95+
test("filters out test-only arguments for test builds", async () => {
96+
additionalTestArgumentsConfig.setValue(() =>
97+
createMockFolderConfig([
98+
"--no-parallel",
99+
"--filter",
100+
"TestCase",
101+
"--enable-code-coverage",
102+
])
103+
);
104+
105+
const config = await BuildConfigurationFactory.buildAll(
106+
instance(mockedFolderContext),
107+
true, // isTestBuild
108+
false // isRelease
109+
);
110+
111+
expect(filterArgumentsSpy).to.have.been.calledOnce;
112+
const [args] = filterArgumentsSpy.firstCall.args;
113+
114+
expect(args).to.deep.equal([
115+
"--no-parallel",
116+
"--filter",
117+
"TestCase",
118+
"--enable-code-coverage",
119+
]);
120+
121+
expect(config.args).to.include("--build-tests");
122+
});
123+
124+
test("preserves build-compatible arguments for test builds", async () => {
125+
additionalTestArgumentsConfig.setValue(() =>
126+
createMockFolderConfig([
127+
"--scratch-path",
128+
"/tmp/build",
129+
"-Xswiftc",
130+
"-enable-testing",
131+
])
132+
);
133+
134+
// Act: Build configuration for test build
135+
await BuildConfigurationFactory.buildAll(
136+
instance(mockedFolderContext),
137+
true, // isTestBuild
138+
false // isRelease
139+
);
140+
141+
expect(filterArgumentsSpy).to.have.been.calledOnce;
142+
const [args] = filterArgumentsSpy.firstCall.args;
143+
expect(args).to.deep.equal([
144+
"--scratch-path",
145+
"/tmp/build",
146+
"-Xswiftc",
147+
"-enable-testing",
148+
]);
149+
});
150+
151+
test("does not filter arguments for non-test builds", async () => {
152+
additionalTestArgumentsConfig.setValue(() =>
153+
createMockFolderConfig(["--no-parallel", "--scratch-path", "/tmp/build"])
154+
);
155+
156+
await BuildConfigurationFactory.buildAll(
157+
instance(mockedFolderContext),
158+
false, // isTestBuild
159+
false // isRelease
160+
);
161+
162+
expect(filterArgumentsSpy).to.not.have.been.called;
163+
});
164+
165+
test("handles empty additionalTestArguments", async () => {
166+
additionalTestArgumentsConfig.setValue(() => createMockFolderConfig([]));
167+
168+
await BuildConfigurationFactory.buildAll(
169+
instance(mockedFolderContext),
170+
true, // isTestBuild
171+
false // isRelease
172+
);
173+
174+
expect(filterArgumentsSpy).to.have.been.calledOnce;
175+
const [args] = filterArgumentsSpy.firstCall.args;
176+
expect(args).to.deep.equal([]);
177+
});
178+
179+
test("handles mixed arguments correctly", async () => {
180+
additionalTestArgumentsConfig.setValue(() =>
181+
createMockFolderConfig([
182+
"--no-parallel", // test-only (should be filtered out)
183+
"--scratch-path",
184+
"/tmp", // build-compatible (should be preserved)
185+
"--filter",
186+
"MyTest", // test-only (should be filtered out)
187+
"-Xswiftc",
188+
"-O", // build-compatible (should be preserved)
189+
"--enable-code-coverage", // test-only (should be filtered out)
190+
"--verbose", // build-compatible (should be preserved)
191+
])
192+
);
193+
194+
await BuildConfigurationFactory.buildAll(
195+
instance(mockedFolderContext),
196+
true, // isTestBuild
197+
false // isRelease
198+
);
199+
200+
expect(filterArgumentsSpy).to.have.been.calledOnce;
201+
const [args] = filterArgumentsSpy.firstCall.args;
202+
expect(args).to.deep.equal([
203+
"--no-parallel",
204+
"--scratch-path",
205+
"/tmp",
206+
"--filter",
207+
"MyTest",
208+
"-Xswiftc",
209+
"-O",
210+
"--enable-code-coverage",
211+
"--verbose",
212+
]);
213+
});
214+
215+
test("has correct include values for arguments with parameters", () => {
216+
// Access the private static property through the class
217+
const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS;
218+
219+
// Arguments that take 1 parameter
220+
const oneParamArgs = ["--filter", "--skip", "--num-workers", "--xunit-output"];
221+
oneParamArgs.forEach(arg => {
222+
const filterItem = filter.find((f: any) => f.argument === arg);
223+
expect(filterItem, `${arg} should be in exclusion filter`).to.exist;
224+
expect(filterItem.include, `${arg} should exclude 1 parameter`).to.equal(1);
225+
});
226+
227+
// Arguments that take no parameters (flags)
228+
const noParamArgs = ["--parallel", "--no-parallel", "--list-tests"];
229+
noParamArgs.forEach(arg => {
230+
const filterItem = filter.find((f: any) => f.argument === arg);
231+
expect(filterItem, `${arg} should be in exclusion filter`).to.exist;
232+
expect(filterItem.include, `${arg} should exclude 0 parameters`).to.equal(0);
233+
});
234+
});
235+
236+
test("excludes expected test-only arguments", () => {
237+
// Access the private static property through the class
238+
const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS;
239+
240+
expect(filter).to.be.an("array");
241+
242+
// Verify test-only arguments are included in the exclusion list
243+
expect(filter.some((f: any) => f.argument === "--no-parallel")).to.be.true;
244+
expect(filter.some((f: any) => f.argument === "--parallel")).to.be.true;
245+
expect(filter.some((f: any) => f.argument === "--filter")).to.be.true;
246+
expect(filter.some((f: any) => f.argument === "--skip")).to.be.true;
247+
expect(filter.some((f: any) => f.argument === "--list-tests")).to.be.true;
248+
expect(filter.some((f: any) => f.argument === "--attachments-path")).to.be.true;
249+
expect(filter.some((f: any) => f.argument === "--enable-testable-imports")).to.be.true;
250+
expect(filter.some((f: any) => f.argument === "--xunit-output")).to.be.true;
251+
});
252+
253+
test("does not exclude build-compatible arguments", () => {
254+
// Access the private static property through the class
255+
const filter = (BuildConfigurationFactory as any).TEST_ONLY_ARGUMENTS;
256+
257+
// Verify build-compatible arguments are NOT in the exclusion list
258+
expect(filter.some((f: any) => f.argument === "--scratch-path")).to.be.false;
259+
expect(filter.some((f: any) => f.argument === "-Xswiftc")).to.be.false;
260+
expect(filter.some((f: any) => f.argument === "--build-system")).to.be.false;
261+
expect(filter.some((f: any) => f.argument === "--sdk")).to.be.false;
262+
expect(filter.some((f: any) => f.argument === "--verbose")).to.be.false;
263+
expect(filter.some((f: any) => f.argument === "--configuration")).to.be.false;
264+
});
265+
});
266+
});

0 commit comments

Comments
 (0)