Skip to content

Conversation

@stmontgomery
Copy link
Contributor

This PR adopts Swift Testing in place of XCTest in this package repo's tests.

Modifications

I approached these changes as follows:

  • First, I did a very straightforward, mechanical 1-1 replacement of the XCTest API usages for analogous Swift Testing APIs. I landed that as the first commit, and kept the old XCTest file around temporarily for reference as I continued development.
  • Then, I modified the names of test functions and suite types to use Raw Identifiers for better readability.
  • Next, I evaluated each @Test function (there were only a handful) looking for ways to make them more idiomatic Swift Testing-style tests:
    • Some, I decomposed into multiple smaller tests and grouped into a new sub-suite.
    • Others, I transformed into parameterized test functions.
    • In making these changes, I tried my best to preserve existing code comments and the general flow/structure.
  • Finally, once the conversion was complete, I deleted the old XCTest file.

Testing performed

I ran these at-desk from Xcode 26.1 Beta and confirmed all modified tests are passing.

Fixes #27

…t of changes which are strictly necessary to convert XCTest API usages into their Swift Testing counterparts
…efactor where they will be needed at other scopes
…focused test functions in a sub-suite
…ore focused test functions in a new sub-suite named `Malformed proposals` (after the snapshot it references)
…this for `Good dates` test function but it has many more arguments and its performance in Xcode is somewhat poor currently
@stmontgomery
Copy link
Contributor Author

@dempseyatgithub, how does this look? Offline, we discussed a concern about whether it was safe to adopt Swift 6.2 features in this repo yet, but if this Swift CI Jenkins job is what performs that build, it seems like this should be safe since it does not build the package's tests.

Either way, I'm interested to hear your feedback and hopefully get this landed! Thanks.

Copy link
Collaborator

@dempseyatgithub dempseyatgithub left a comment

Choose a reason for hiding this comment

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

I requested / suggested some changes.

If any of them seem way out of line for idiomatic Swift Testing, please let me know. Obviously, you have had much more experience with it than I have, this is my first serious foray.

The two preferences I have (noted with suggestions in the review) are to explicitly annotate suites with @Suite and to name them using title case so they appear more like a heading.

One other question/piece of feedback. I find that sometimes when a test is gathering arguments with a lot of set-up (like Warning and errors() in this file) the actual test - including seeing the name of the test, gets lost in the noise of gathering the arguments.

Do you have any suggestions to aid in that? Is it a poor stylistic choice to always use a test description with @Test as opposed to using a raw identifier? (I realize that leads to the 'name the thing twice' issue)

Thank you for your patience and for doing the migration to Swift Testing.

And also thank you for answering my questions. I am really excited about Swift Testing (I just rewatched the 2024 WWDC Videos before doing this review, and got enthused all over again - such a beautiful API design!)


@testable import EvolutionMetadataModel
@testable import EvolutionMetadataExtraction

Copy link
Collaborator

Choose a reason for hiding this comment

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

The Proposal type could use a custom test description.

I don't know what the best practice is for adding test descriptions. (As part of the original definition of the type, as part of the test bundle, etc.)

In the case, it is a small extension and likely will not be joined by other test descriptions, so defining at the top doesn't add much clutter, but makes it obvious where that description is coming from.

Would appreciate any feedback you have in terms of canonical Swift Testing as to where this should live, but Proposal definitely needs a custom test description.

Suggested change
extension Proposal: CustomTestStringConvertible {
public var testDescription: String { id.isEmpty ? "No ID" : id }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to include such an extension and conformance. In terms of where it should go, that's very much a style question and we, the maintainers of Swift Testing, don't really offer canonical guidance about that so I would apply whatever your preferred Swift code style is generally.

My own personal tendency would be to place it within the // MARK: - Helpers section, which in this file is near the bottom. I generally like the "main" declarations in a file to appear closer to the top. But it's purely subjective/stylistic.

@stmontgomery
Copy link
Contributor Author

I requested / suggested some changes. If any of them seem way out of line for idiomatic Swift Testing, please let me know. Obviously, you have had much more experience with it than I have, this is my first serious foray.

The two preferences I have (noted with suggestions in the review) are to explicitly annotate suites with @Suite and to name them using title case so they appear more like a heading.

Thanks for reviewing! A few suggestions do differ from my own personal style, but it's small stuff and I'm of course happy to conform to your coding style and norms. None of your suggestions are out of line or inappropriate with what I would consider recommended and supported Swift Testing usage patterns.

One other question/piece of feedback. I find that sometimes when a test is gathering arguments with a lot of set-up (like Warning and errors() in this file) the actual test - including seeing the name of the test, gets lost in the noise of gathering the arguments. […] Do you have any suggestions to aid in that?

Yeah I was beginning to feel a few of those arguments: closures were getting too long and distracting from the test body, too. The Warnings and Errors test in particular began to suffer from this moreso as I made your suggested changes, so I have refactored it more and moved the arguments to a static var separate from the @Test. In general I think that's a good pattern to use. It does require you to choose a name for that "helper" computed property, though.

Is it a poor stylistic choice to always use a test description with @Test as opposed to using a raw identifier? (I realize that leads to the 'name the thing twice' issue)

I don't consider it a poor choice but I suspect we're going to begin de-emphasizing the original string-based display names and recommending raw identifiers more.

Thank you for your patience and for doing the migration to Swift Testing.

And also thank you for answering my questions. I am really excited about Swift Testing (I just rewatched the 2024 WWDC Videos before doing this review, and got enthused all over again - such a beautiful API design!)

☺️👍

Copy link
Collaborator

@dempseyatgithub dempseyatgithub left a comment

Choose a reason for hiding this comment

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

This looks good. Thanks for the changes and for the work in converting the tests.

@dempseyatgithub dempseyatgithub merged commit 06e000f into swiftlang:main Nov 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move to Swift Testing when Swift 6 is available in the deployment environment

2 participants