Skip to content
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.

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

Adopt Swift-Testing in test utils such as SwiftSyntaxMacrosTestSupport #2720

Open
MahdiBM opened this issue Jul 8, 2024 · 10 comments
Open
Labels
enhancement New feature or request

Comments

@MahdiBM
Copy link
Contributor

MahdiBM commented Jul 8, 2024

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 MahdiBM added the enhancement New feature or request label Jul 8, 2024
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Jul 8, 2024

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.

@ahoppen
Copy link
Member

ahoppen commented Jul 8, 2024

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

@MahdiBM MahdiBM changed the title Adapt Swift-Testing in test utils such as SwiftSyntaxMacrosTestSupport Adopt Swift-Testing in test utils such as SwiftSyntaxMacrosTestSupport Jul 8, 2024
@grynspan
Copy link
Contributor

grynspan commented Oct 4, 2024

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

kevinrpb added a commit to kevinrpb/Defaults that referenced this issue Nov 19, 2024
swift-testing is not supported for macro expansion tests yet.
See swiftlang/swift-syntax#2720
@MahdiBM
Copy link
Contributor Author

MahdiBM commented Nov 21, 2024

@ahoppen we're moving our tests away from XCTest, and other than vapor/vapor#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
Copy link
Contributor Author

MahdiBM commented Nov 21, 2024

@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?

@grynspan
Copy link
Contributor

The circular dependency still exists.

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Nov 21, 2024

@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.

@grynspan
Copy link
Contributor

Now build the toolchain from source. :)

@MahdiBM
Copy link
Contributor Author

MahdiBM commented Nov 22, 2024

@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

@grynspan
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants