Skip to content

Conversation

alemohamad
Copy link
Member

@alemohamad alemohamad commented Jan 15, 2025

Following the issues #1038 and #594, this proposal suggests updating the Testing documentation to include Swift Testing as an alternative to XCTest.

@alemohamad alemohamad added the updates Adding new or updated documentation. label Jan 15, 2025
@alemohamad alemohamad requested a review from MahdiBM January 15, 2025 21:29
@alemohamad alemohamad self-assigned this Jan 15, 2025
@alemohamad alemohamad force-pushed the update-testing-documentation branch from 5d929d7 to e0f2bec Compare January 17, 2025 16:41
Copy link
Contributor

@MahdiBM MahdiBM 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 but not sure if we want to replace the XCT docs, or add a new one for Swift Testing.

Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

I agree with @MahdiBM - we should keep the old XCTVapor docs but put a warning telling people they should choose VaporTesting unless they have a reason not to

@alemohamad
Copy link
Member Author

I understand. In my opinion we should be promoting the Swift Testing option first and then add a note about the XCTest framework. I think that having a complete example with the new framework can be another way of encouraging its usage.

What do you prefer? Maintain the XCTest with a note about Swift Testing or the other way around?

@0xTim
Copy link
Member

0xTim commented Jan 17, 2025

What do you prefer? Maintain the XCTest with a note about Swift Testing or the other way around?

Swift Testing first, XCTest at the bottom

@alemohamad alemohamad requested review from 0xTim and MahdiBM January 17, 2025 19:13
@alemohamad alemohamad force-pushed the update-testing-documentation branch from c131203 to b8652c5 Compare January 17, 2025 19:15
@alemohamad alemohamad force-pushed the update-testing-documentation branch from b8652c5 to 942f287 Compare January 17, 2025 22:52
@alemohamad alemohamad requested a review from 0xTim January 17, 2025 23:04
@alemohamad alemohamad force-pushed the update-testing-documentation branch from 11901be to 74128ea Compare January 18, 2025 20:00
@alemohamad
Copy link
Member Author

@0xTim now that we are reviewing the Testing page, should we look at this issue #865 ? We can remove the text about precondition failure, if this is resolved. Let me know.

@0xTim
Copy link
Member

0xTim commented Jan 21, 2025

@alemohamad you still need to call it to shutdown services, but if you use the withApp stuff then it's done for you

@alemohamad alemohamad requested review from MahdiBM and 0xTim January 21, 2025 15:00
Copy link
Member

@0xTim 0xTim left a comment

Choose a reason for hiding this comment

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

Thanks!

@0xTim 0xTim enabled auto-merge (squash) January 21, 2025 20:28
@alemohamad alemohamad disabled auto-merge January 21, 2025 20:40
@alemohamad
Copy link
Member Author

@0xTim sorry, tapped on cancel the auto-merge.

@alemohamad alemohamad requested a review from a team as a code owner January 22, 2025 09:42
@alemohamad alemohamad enabled auto-merge (squash) January 22, 2025 09:42
@alemohamad alemohamad merged commit c6b79d7 into vapor:main Jan 22, 2025
1 check passed
@penny-for-vapor penny-for-vapor bot mentioned this pull request Jan 22, 2025
9 tasks
@alemohamad alemohamad deleted the update-testing-documentation branch January 22, 2025 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
updates Adding new or updated documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants