Skip to content

Add support for Swift Testing in SwiftSyntaxMacrosTestsSupport #3192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0xTim wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add support for Swift Testing in SwiftSyntaxMacrosTestsSupport #3192

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2ddc19b
Record issues if we're in a Swift testing environment
0xTim Nov 21, 2025
ba15ee0
Add support in other method
0xTim Nov 21, 2025
dff2179
Add tests
0xTim Nov 21, 2025
d7fd8ff
Private imports
0xTim Nov 21, 2025
8f55686
Fix some of the CI issues
0xTim Nov 21, 2025
be537f1
Fix formatting
0xTim Nov 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions Sources/SwiftSyntaxMacrosTestSupport/Assertions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ public import SwiftSyntaxMacroExpansion
public import SwiftSyntaxMacros
@_spi(XCTestFailureLocation) public import SwiftSyntaxMacrosGenericTestSupport
private import XCTest
#if canImport(Testing)
import Testing
#endif
#else
import SwiftIfConfig
import SwiftSyntax
Expand Down Expand Up @@ -61,8 +64,10 @@ public func assertMacroExpansion(
testFileName: String = "test.swift",
indentationWidth: Trivia = .spaces(4),
buildConfiguration: (any BuildConfiguration)? = nil,
fileID: StaticString = #fileID,
file: StaticString = #filePath,
line: UInt = #line
line: UInt = #line,
column: UInt = #column
) {
let specs = macros.mapValues { MacroSpec(type: $0) }
assertMacroExpansion(
Expand All @@ -76,8 +81,10 @@ public func assertMacroExpansion(
testFileName: testFileName,
indentationWidth: indentationWidth,
buildConfiguration: buildConfiguration,
fileID: fileID,
file: file,
line: line
line: line,
column: column
)
}

Expand Down Expand Up @@ -110,8 +117,10 @@ public func assertMacroExpansion(
testFileName: String = "test.swift",
indentationWidth: Trivia = .spaces(4),
buildConfiguration: (any BuildConfiguration)? = nil,
fileID: StaticString = #fileID,
file: StaticString = #filePath,
line: UInt = #line
line: UInt = #line,
column: UInt = #column
) {
SwiftSyntaxMacrosGenericTestSupport.assertMacroExpansion(
originalSource,
Expand All @@ -125,7 +134,15 @@ public func assertMacroExpansion(
indentationWidth: indentationWidth,
buildConfiguration: buildConfiguration,
failureHandler: {
#if canImport(Testing)
if Test.current != nil {
Copy link
Contributor

@grynspan grynspan Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not the correct test to determine which library is in use because code can run in a detached task. See swiftlang/swift-testing#475

Copy link
Member Author

@0xTim 0xTim Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the correct way? Should we split it out into a expect function instead?

Copy link
Contributor

@grynspan grynspan Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no correct way at this time, which is why that issue is still open. Jerry's work should allow us to just call #expect() here and have it work under all configurations.

Copy link
Member Author

@0xTim 0xTim Nov 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pitch looks like it will solve the issue, but still require work in the library to migrate over to Swift Testing APIs. What I propose is that we land this now, as it solves a problem that exists for users today (and potentially provide a release in the next monthly Linux release/Swift patch release) and then fix forward when the proposal lands. Given it's still in the pitch phase it likely won't be landed until 6.4 and waiting 10 months for a solution seems like a bad idea.

Regarding the Test.current issue - from my understanding this works in all instances apart from those running in a detached task. For this specific case, I can't see a scenario when a user would be using the assertMacroExpansion from a detached task so we can fix this for the majority of the users and those attempting to use it from a detached task will see no change in behaviour.

Copy link
Contributor

@grynspan grynspan Nov 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stmontgomery Your take? You okay with this presumably being nonfunctional with the package build?

For this specific case, I can't see a scenario when a user would be using the assertMacroExpansion from a detached task so we can fix this for the majority of the users and those attempting to use it from a detached task will see no change in behaviour.

Let's at least document it as unsupported in the symbol's Markup?

Copy link
Contributor

@grynspan grynspan Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we can agree here that SwiftSyntax can provide Testing support by depending on the toolchain's Testing module can we unblock the PR on this matter?

I'd be morally okay with saying "the features in the swift-syntax repository are dependent on the built-in copy of Swift Testing even if you include a package dependency" however this will break builds on non-Apple platforms with flat linker namespaces due to duplicate symbols at link time.

I think there is still the larger question on how this is currently implemented. The if Test.current != nil check in this method is certainly less than ideal. Instead I would propose that we add an entirely new method called expectMacroExpansion that is based on Testing unconditionally. So existing users of assertMacroExpansion can continue to use it with XCTest and Testing users can start adopting the new one. How does that sound to everyone?

Once @jerryjrchen's work on the interop feature lands, it will be possible to implement this in a way that depends on neither XCTest nor Swift Testing. It may be a better idea to just wait until that work is done and revisit the problem at that point.

Copy link
Member

@FranzBusch FranzBusch Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be morally okay with saying "the features in the swift-syntax repository are dependent on the built-in copy of Swift Testing even if you include a package dependency" however this will break builds on non-Apple platforms with flat linker namespaces due to duplicate symbols at link time.

Only if there is both Testing from the toolchain and from the package right? Which I thought we agreed is only really valid in development environments.

Copy link
Contributor

@grynspan grynspan Dec 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only if there is both Testing from the toolchain and from the package right? Which I thought we agreed is only really valid in development environments.

All testing environments are development environments.

If swift-syntax explicitly links the copy of Swift Testing in the toolchain, that will break developers who want to test their macros and have a package dependency on Swift Testing.

In addition, if they are using the package copy of Swift Testing and you are using the toolchain's copy, your calls to e.g. #expect() won't be routed to the infrastructure the developer's test target links against, so a failure will be invisible.

Jerry's work should give us an escape hatch for this problem, so we should wait until it lands and then make the necessary changes here.

Copy link
Member Author

@0xTim 0xTim Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern with waiting is that it could be a year until that's shipped, meanwhile users have no indications their tests are passing incorrectly, whereas we could fix it today for the majority of use cases

Copy link
Contributor

@grynspan grynspan Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I appreciate the concern, but there are technical blockers here. We must not cause build failures for teams using Swift Testing as a package.

Issue.record(Comment(rawValue: $0.message), sourceLocation: .init(fileID: fileID.description, filePath: file.description, line: Int(line), column: Int(column)))
} else {
XCTFail($0.message, file: $0.location.staticFilePath, line: $0.location.unsignedLine)
}
#else
XCTFail($0.message, file: $0.location.staticFilePath, line: $0.location.unsignedLine)
#endif
},
fileID: "", // Not used in the failure handler
filePath: file,
Expand Down
2 changes: 1 addition & 1 deletion Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import SwiftSyntaxMacros
import SwiftSyntaxMacrosTestSupport
import XCTest

private struct ConstantOneGetter: AccessorMacro {
struct ConstantOneGetter: AccessorMacro {
static func expansion(
of node: AttributeSyntax,
providingAccessorsOf declaration: some DeclSyntaxProtocol,
Expand Down
49 changes: 49 additions & 0 deletions Tests/SwiftSyntaxMacroExpansionTest/SwiftTestingTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#if canImport(Testing)
import Testing
import SwiftSyntaxMacrosTestSupport

@Suite("Swift Testing Macro Expansion Tests")
struct SwiftTestingMacroExpansionTests {
@Test("Test Happy Path")
func testHappyPathWorks() {
assertMacroExpansion(
"""
@constantOne
var x: Int /*1*/ // hello
""",
expandedSource: """
var x: Int { /*1*/ // hello
get {
return 1
}
}
""",
macros: ["constantOne": ConstantOneGetter.self],
indentationWidth: .spaces(2)
)
}

@Test("Test Failure")
func failureReportedCorrectly() {
withKnownIssue {
assertMacroExpansion(
"""
@constantOne
var x: Int /*1*/ // hello
""",
expandedSource: """
var x: Int { /*1*/ // hello
get {
return 1
}
}
""",
macros: ["constantOne": ConstantOneGetter.self],
indentationWidth: .spaces(4)
)
} matching: { issue in
issue.description.contains("Macro expansion did not produce the expected expanded source")
}
}
}
#endif
Loading