swiftlang / swift-markdown

A Swift package for parsing, building, editing, and analyzing Markdown documents.
https://swiftpackageindex.com/swiftlang/swift-markdown/documentation/markdown
Apache License 2.0
2.76k stars 188 forks source link

Add swift-testing dependency #153

Open Kyle-Ye opened 1 year ago

Kyle-Ye commented 1 year ago

Sometimes the XCTAssert's output for diff is not very clear.

(For example I have to copy paste a few times from the result of Swift.print or the failure result of XCTAssertEqual to make it happy)

XCTAssertEqual failed: ("Document @1:1-4:2
└─ BlockDirective @1:1-4:2 name: "Image"
   ├─ Argument text segments:
   |    @2:1-2:25: "  source: "example.png","
   |    @3:1-3:23: "  alt: "Example image""") is not equal to ("Document @1:1-4:2
└─ BlockDirective @1:1-4:2 name: "Image"
   ├─ Argument text segments:
   |    @2:1-2:25: "  source: \"example.png\","
   |    @3:1-3:23: "  alt: \"Example image\""")

I can certainly solve it in the end. But if we can migrate to use swift-testing as our test framework, it will be more clear and more efficient.

Kyle-Ye commented 1 year ago

Here is something blocking items preventing the adoption in Swift-DocC project IMO:

grynspan commented 1 year ago

Hi Kyle! swift-testing has a permanent dependency on Swift 5.9 due to its use of macros. It does not have a declared dependency on swift-docc-plugin.

Kyle-Ye commented 1 year ago

Hi Kyle! swift-testing has a permanent dependency on Swift 5.9 due to its use of macros. It does not have a declared dependency on swift-docc-plugin.

I'll update the statements. I checked it again and found my mistake here.

grynspan commented 1 year ago

Currently it has a dependency on Swift-Syntax of Swift 5.9. Hope SwiftPM will have a syntax declaration later to mark a dependency as testTarget only so our downstream user won't get swift-testing and Swift-Syntax dependency.

Please file an issue against SwiftPM for this?

Kyle-Ye commented 1 year ago

Please file an issue against SwiftPM for this?

Got it. Filed via https://github.com/apple/swift-package-manager/issues/7007

d-ronnqvist commented 1 year ago

Here is something blocking items preventing the adoption in Swift-DocC project IMO:

I agree with those items.

Kyle-Ye commented 6 months ago

Have tried swift-testing for a while. I think it's ready for production use. (Won't make it default or rewrite all test case into it before Swift 6.)

Add a PR to draft the integration. See #175

Here is something blocking items preventing the adoption in Swift-DocC project IMO:

  • [ ] Missing Windows Support due to the macro issue on Windows Platform
  • [ ] Minimal Swift version is currently 5.9
  • [ ] Currently it is in active development stage. Not source stable.
  • [x] ~Currently it has a dependency on swift-docc-plugin.(Maybe wrap it on) So there may be cycle dependency issue if we want to use it in swift-docc-plugin or swift-docc-symbolkit. cc @grynspan~
  • [ ] Currently it has a dependency on Swift-Syntax of Swift 5.9. Hope SwiftPM will have a syntax declaration later to mark a dependency as testTarget only so our downstream user won't get swift-testing and Swift-Syntax dependency.
Kyle-Ye commented 6 months ago
  • We do not need to really bump the version.

That said I still hope #135 can be merged since we have discussed and approved it on SDWG meeting. :)

grynspan commented 6 months ago

Have tried swift-testing for a while. I think it's ready for production use. (Won't make it default or rewrite all test case into it before Swift 6.)

This package remains experimental. We do not consider it ready for production use at this time.

Kyle-Ye commented 6 months ago

Have tried swift-testing for a while. I think it's ready for production use. (Won't make it default or rewrite all test case into it before Swift 6.)

This package remains experimental. We do not consider it ready for production use at this time.

Of course. It won't be production ready before 1.0 release.

Sorry for the incorrect wording. I mean we can consider making it for experiential usage under a feature flag.

d-ronnqvist commented 6 months ago

I don't think we should have experimental dependencies. It's better to wait until swift-testing is considered ready for production.

tayloraswift commented 6 months ago

i encourage swift-markdown (and other packages) to adopt swift-testing. my experience at our organization is that not doing so disincentivizes developers from writing tests, as all tests written against a legacy framework would need to be rewritten later. since we are constantly expecting swift-testing 1.0 to be “just around the corner”, the quality of code can deteriorate markedly as test writing gets repeatedly deferred.

Kyle-Ye commented 4 months ago

As swift-testing is bundled with Xcode 16 beta now and will probably be release together with Xcode 16, can we revisit this policy? @d-ronnqvist

Also would you mind helping to explain the current situation of swift-testing in swift-toolchain? It is not clear to me whether swift-testing will be available in the next Swift 6 toolchain for non-Darwin platform(Linux & Windows)? @grynspan

d-ronnqvist commented 4 months ago

As swift-testing is bundled with Xcode 16 beta now and will probably be release together with Xcode 16, can we revisit this policy? @d-ronnqvist

Also would you mind helping to explain the current situation of swift-testing in swift-toolchain? It is not clear to me whether swift-testing will be available in the next Swift 6 toolchain for non-Darwin platform(Linux & Windows)? @grynspan

I may be wrong about this but my understanding is that this would require a Swift 6 toolchain (Xcode 16) for building on all platforms. If that is correct, we shouldn't adopt Swift Testing yet. We still want to be able to build Swift Markdown, Swift SymbolKit, Swift DocC, etc. with a Swift 5.9 toolchain on all platforms.

Kyle-Ye commented 4 months ago

If that is correct, we shouldn't adopt Swift Testing yet. We still want to be able to build Swift Markdown.

We can have both IMO.

Since swift-markdown is toolchain package, and the main branch corresponds to swift/6.0 or swift/next toolchain.

I do not understand why we need to maintain older Swift development environment.

Of course we will continue to support older SDK build and old runtime running via Package@5.7 or something else.

eg. #if swift>=6.0 #if canImport(Testing) or exclude: [xx] in Pakcage@6.0.swift to protect the new #expect code from being built on older Swift toolchain.

But bumping the development environment to Swift 6 seems no harm to me.

grynspan commented 4 months ago

Swift Testing will only be supported with a Swift 6 toolchain or newer. At this time, it has not been added to prerelease the Swift 6 toolchain.

Kyle-Ye commented 1 month ago

Swift Testing will only be supported with a Swift 6 toolchain or newer. At this time, it has not been added to prerelease the Swift 6 toolchain.

I guess we can use swift-testing now with the following code since swift-markdown need to support Swift 6.0- compiler.

#if compiler(>=6.0)
import Testing

...
#endif

Is there any other thing we need to configure as swift-markdown is part of the Swift Toolchain?

eg. Explicitly declare a dependency relation between swift-markdown on swift-testing on https://github.com/swiftlang/swift

grynspan commented 1 month ago

That should "just work".