swift-syntax icon indicating copy to clipboard operation
swift-syntax copied to clipboard

Adopt Swift-Testing in test utils such as `SwiftSyntaxMacrosTestSupport`

Open MahdiBM opened this issue 1 year ago • 23 comments

Description

Currently you need to use XCTest to run macro tests with SwiftSyntaxMacrosTestSupport functions because they rely on some XCTest functions such as XCTFail.

swift-syntax tests utils should also support Swift-Testing.

MahdiBM avatar Jul 08 '24 16:07 MahdiBM

Not that hard to do, even manually. This works like the currently-provided XCTest-related functions.

import SwiftSyntax
import SwiftSyntaxMacrosGenericTestSupport
import SwiftSyntaxMacros
import SwiftSyntaxMacroExpansion
import Testing

func assertMacroExpansionWithSwiftTesting(
    _ originalSource: String,
    expandedSource expectedExpandedSource: String,
    diagnostics: [DiagnosticSpec] = [],
    macros: [String: any Macro.Type],
    applyFixIts: [String]? = nil,
    fixedSource expectedFixedSource: String? = nil,
    testModuleName: String = "TestModule",
    testFileName: String = "test.swift",
    indentationWidth: Trivia = .spaces(4),
    sourceLocation: Testing.SourceLocation = Testing.SourceLocation()
) {
    let macroSpecs = macros.mapValues { MacroSpec(type: $0) }
    SwiftSyntaxMacrosGenericTestSupport.assertMacroExpansion(
        originalSource,
        expandedSource: expectedExpandedSource,
        diagnostics: diagnostics,
        macroSpecs: macroSpecs,
        applyFixIts: applyFixIts,
        fixedSource: expectedFixedSource,
        testModuleName: testModuleName,
        testFileName: testFileName,
        indentationWidth: indentationWidth,
        failureHandler: {
            #expect(Bool(false), .init(stringLiteral: $0.message), sourceLocation: sourceLocation)
        },
        fileID: "",  // Not used in the failure handler
        filePath: "", /// MahdiBM comment: requires StaticString so just set it to "" for now.
        line: UInt(sourceLocation.line),
        column: 0  // Not used in the failure handler
    )
}

[Update]: added some more imports to the code.

MahdiBM avatar Jul 08 '24 16:07 MahdiBM

Synced to Apple’s issue tracker as rdar://131312113

ahoppen avatar Jul 08 '24 16:07 ahoppen

FWIW rather than #expect(Bool(false), ...), write Issue.record(...) :)

grynspan avatar Oct 04 '24 13:10 grynspan

@ahoppen we're moving our tests away from XCTest, and other than https://github.com/vapor/vapor/pull/3257, swift-testing support in SwiftSyntax, specifically for MacrosTestSupport target, is the only thing left.

I might be able to get away with putting some things together for our specific project in that repo itself, but in terms of swift-testing support in SwiftSyntax, what do think is needed to be done, generally?

Just want to make sure I have a somewhat accurate idea about what this issue requires. I'm personally interested in swift-testing support only for MacrosTestSupport, but I'd appreciate if you explain the situation with the rest of the test utilities as well.

MahdiBM avatar Nov 21 '24 12:11 MahdiBM

@ahoppen I noticed this:

  • SwiftSyntaxMacrosGenericTestSupport
    • Description: A version of the SwiftSyntaxMacrosTestSupport module that doesn't depend on Foundation or XCTest and can thus be used to write macro tests using swift-testing. Since swift-syntax can't depend on swift-testing (which would incur a circular dependency since swift-testing depends on swift-syntax), users need to manually specify a failure handler like the following, that fails the swift-testing test: Issue.record("\($0.message)", fileID: $0.location.fileID, filePath: $0.location.filePath, line: $0.location.line, column: $0.location.column)

More specifically:

Since swift-syntax can't depend on swift-testing (which would incur a circular dependency since swift-testing depends on swift-syntax)

I assume it's no longer true considering swift-testing has made its way to the toolchains? Would i be able to do a PR for swift-testing integration now?

MahdiBM avatar Nov 21 '24 13:11 MahdiBM

The circular dependency still exists.

grynspan avatar Nov 21 '24 13:11 grynspan

@grynspan Then how come my branch right here builds and runs swift-testing tests just fine both on macOS and swift:6.0-jammy? The branch contains a new SwiftSyntaxMacrosTesting module for swift-testing compatibility as a counterpart to SwiftSyntaxMacrosTestSupport. And SwiftSyntaxMacrosTestingTests as tests.

Try it yourself:

docker run --rm -it swift:6.0-jammy bash -c "git clone https://github.com/MahdiBM/swift-syntax.git && cd swift-syntax && git checkout mmbm-swift-testing-macros-test-support && swift build --build-tests && swift test --filter SwiftSyntaxMacrosTestingTests"

result:

Cloning into 'swift-syntax'...
remote: Enumerating objects: 57916, done.
remote: Counting objects: 100% (3116/3116), done.
remote: Compressing objects: 100% (1099/1099), done.
remote: Total 57916 (delta 2346), reused 2494 (delta 1997), pack-reused 54800 (from 1)
Receiving objects: 100% (57916/57916), 25.91 MiB | 3.27 MiB/s, done.
Resolving deltas: 100% (45856/45856), done.
Branch 'mmbm-swift-testing-macros-test-support' set up to track remote branch 'mmbm-swift-testing-macros-test-support' from 'origin'.
Switched to a new branch 'mmbm-swift-testing-macros-test-support'
warning: 'swift-syntax': found 1 file(s) which are unhandled; explicitly declare them as resources or exclude from the target
    /swift-syntax/Sources/SwiftLexicalLookup/CMakeLists.txt
Building for debugging...
[726/726] Linking swift-syntaxPackageTests.xctest
Build complete! (38.88s)
warning: 'swift-syntax': found 1 file(s) which are unhandled; explicitly declare them as resources or exclude from the target
    /swift-syntax/Sources/SwiftLexicalLookup/CMakeLists.txt
Building for debugging...
[1/1] Write swift-version-24593BA9C3E375BF.txt
Build complete! (0.32s)
Test Suite 'Selected tests' started at 2024-11-21 14:29:55.739
Test Suite 'Selected tests' passed at 2024-11-21 14:29:55.741
	 Executed 0 tests, with 0 failures (0 unexpected) in 0.0 (0.0) seconds
◇ Test run started.
↳ Testing Library Version: 6.0.1 (324940967b09dbb)
◇ Suite AssertionsTests started.
◇ Test assertMacroExpansionMatchesSingleStringHighlight() started.
◇ Test assertMacroExpansionIgnoresHighlightMatchingIfNil() started.
◇ Test assertMacroExpansionMatchesArrayHighlight() started.
✔ Test assertMacroExpansionMatchesSingleStringHighlight() passed after 0.005 seconds.
✔ Test assertMacroExpansionIgnoresHighlightMatchingIfNil() passed after 0.005 seconds.
✔ Test assertMacroExpansionMatchesArrayHighlight() passed after 0.005 seconds.
✔ Suite AssertionsTests passed after 0.005 seconds.
✔ Test run with 3 tests passed after 0.005 seconds.

MahdiBM avatar Nov 21 '24 14:11 MahdiBM

Now build the toolchain from source. :)

grynspan avatar Nov 21 '24 17:11 grynspan

@grynspan I see, but is that a blocking matter? We could work something out by using env vars in the Package.swift file so the toolchain build doesn't include these 2 new testing-related targets. Theoretically "Package Traits" should be able to solve this problem as well, but not sure if they are potent enough in their current form. We could have a trait SwiftTestingIntegration which is enabled by default but disabled by the toolchain build.

cc @ahoppen

MahdiBM avatar Nov 22 '24 13:11 MahdiBM

It would prevent using Swift Testing as a package. Although Swift Testing is in the toolchain today, the core team is interested in moving it to a package in the future, and that would not be possible if it created a circular dependency at package resolution time.

grynspan avatar Nov 23 '24 13:11 grynspan

To fix the circular dependency issue, could Swift Testing be modified to contain a special target containing these macro specific symbols? This way it would be such that both the testing macros already there and the macro testing support symbols would depend on swift-syntax.

thafner0 avatar Jan 21 '25 18:01 thafner0

You're describing what's called a "cross-import overlay". We've recently enabled support for these modules when building test targets with swift test and swift build --build-tests, and it would (I think) be feasible to create one across Testing and SwiftSyntax.

Layering-wise, we'd probably need it to live in the swift-testing repo.

grynspan avatar Jan 21 '25 19:01 grynspan