From f9df4591d64b72636d16803ff39df2537911df01 Mon Sep 17 00:00:00 2001 From: Paul LeMarquand Date: Fri, 20 Sep 2024 09:18:51 -0400 Subject: [PATCH] Add line terminator to XCTest filter regex The regex to specify a specific XCTest to run did not specify that there should be no more text after the test ID. As a result, attempting to run a test like `testFoo` would also run a test in the same suite called `testFooBar`. Swift-testing tests do not have the same issue as their IDs include the function parentheses and argument labels, and no text can appear after the closing paren. Issue: #1084 --- .../Tests/PackageTests/PackageTests.swift | 5 +++++ src/TestExplorer/TestRunArguments.ts | 2 +- src/debugger/buildConfig.ts | 8 +++++++- src/utilities/utilities.ts | 4 ++-- .../TestExplorerIntegration.test.ts | 19 +++++++++++++++++++ test/suite/utilities/utilities.test.ts | 7 +++++++ 6 files changed, 41 insertions(+), 4 deletions(-) diff --git a/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift b/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift index 1551b09e1..506b2824f 100644 --- a/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift +++ b/assets/test/defaultPackage/Tests/PackageTests/PackageTests.swift @@ -100,3 +100,8 @@ struct MixedSwiftTestingSuite { } #endif + +final class DuplicateSuffixTests: XCTestCase { + func testPassing() throws {} + func testPassingSuffix() throws {} +} diff --git a/src/TestExplorer/TestRunArguments.ts b/src/TestExplorer/TestRunArguments.ts index ab4b9caa2..fda612691 100644 --- a/src/TestExplorer/TestRunArguments.ts +++ b/src/TestExplorer/TestRunArguments.ts @@ -200,7 +200,7 @@ export class TestRunArguments { // Only add leaf items to the list of arguments to pass to the test runner. if (this.isLeafTestItem(testItem, !!isXCTest)) { if (isXCTest) { - xcTestArgs.push(testItem.id); + xcTestArgs.push(`${testItem.id}$`); } else { swiftTestArgs.push(testItem.id); } diff --git a/src/debugger/buildConfig.ts b/src/debugger/buildConfig.ts index d09e59293..d2b936d86 100644 --- a/src/debugger/buildConfig.ts +++ b/src/debugger/buildConfig.ts @@ -449,7 +449,13 @@ export class TestingConfigurationFactory { } private addTestsToArgs(args: string[]): string[] { - return [...args, ...this.testList.flatMap(arg => ["--filter", regexEscapedString(arg)])]; + return [ + ...args, + ...this.testList.flatMap(arg => [ + "--filter", + regexEscapedString(arg, new Set(["$", "^"])), + ]), + ]; } private addXCTestExecutableTestsToArgs(args: string[]): string[] { diff --git a/src/utilities/utilities.ts b/src/utilities/utilities.ts index 3ca8d89ce..5515806e9 100644 --- a/src/utilities/utilities.ts +++ b/src/utilities/utilities.ts @@ -306,10 +306,10 @@ const regexEscapedCharacters = new Set(["(", ")", "[", "]", ".", "$", "^", "?", * @param string A string to escape * @returns The escaped string */ -export function regexEscapedString(string: string): string { +export function regexEscapedString(string: string, omitting?: Set): string { let result = ""; for (const c of string) { - if (regexEscapedCharacters.has(c)) { + if (regexEscapedCharacters.has(c) && (!omitting || !omitting.has(c))) { result += `\\${c}`; } else { result += c; diff --git a/test/suite/testexplorer/TestExplorerIntegration.test.ts b/test/suite/testexplorer/TestExplorerIntegration.test.ts index bb24092d4..3be9766d5 100644 --- a/test/suite/testexplorer/TestExplorerIntegration.test.ts +++ b/test/suite/testexplorer/TestExplorerIntegration.test.ts @@ -148,6 +148,8 @@ suite("Test Explorer Suite", function () { ["testPassing()", "testFailing()", "testDisabled()"], "testWithKnownIssue()", "testWithKnownIssueAndUnknownIssue()", + "DuplicateSuffixTests", + ["testPassing()", "testPassingSuffix()"], ], ]); } else if (workspaceContext.swiftVersion.isLessThanOrEqual(new Version(5, 10, 0))) { @@ -158,6 +160,8 @@ suite("Test Explorer Suite", function () { [ "DebugReleaseTestSuite", ["testDebug", "testRelease"], + "DuplicateSuffixTests", + ["testPassing", "testPassingSuffix"], "FailingXCTestSuite", ["testFailing"], "MixedXCTestSuite", @@ -331,6 +335,21 @@ suite("Test Explorer Suite", function () { }); }); + suite("Only runs specified test", async function () { + const passingRun = await runTest( + testExplorer.controller, + TestKind.standard, + "PackageTests.DuplicateSuffixTests/testPassing" + ); + + assertTestResults(passingRun, { + passed: [ + "PackageTests.DuplicateSuffixTests", + "PackageTests.DebugReleaseTestSuite/testPassing", + ], + }); + }); + suite("Runs multiple", function () { const numIterations = 5; const windowMock = mockNamespace(vscode, "window"); diff --git a/test/suite/utilities/utilities.test.ts b/test/suite/utilities/utilities.test.ts index 00da7ab02..ed19c358c 100644 --- a/test/suite/utilities/utilities.test.ts +++ b/test/suite/utilities/utilities.test.ts @@ -175,6 +175,13 @@ suite("Utilities Test Suite", () => { ); }); + test("should escape a string that omits some characters", () => { + assert.strictEqual( + regexEscapedString(".^$|()?[]/:", new Set(["^", "$", "a"])), + "\\.^$\\|\\(\\)\\?\\[\\]\\/\\:" + ); + }); + test("should return an empty string when input is an empty string", () => { assert.strictEqual(regexEscapedString(""), ""); });