-
Notifications
You must be signed in to change notification settings - Fork 129
Highlight parallel-by-default test run in case of XCTest migration #1176
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
Highlight parallel-by-default test run in case of XCTest migration #1176
Conversation
…CTest migration Handling swiftlang#863
|
@iamleeg Would you please help us review? |
|
@petermolnar-dev It looks like you've opened this PR and made changes using two separate GitHub accounts. Was this intentional? |
iamleeg
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this PR!
| For more information about suites and how to declare and customize them, see | ||
| <doc:OrganizingTests>. | ||
|
|
||
| > Important: While XCTest test cases running sequentially by default, the test execution happens in parallel by default in Swift Testing. See the [Run tests sequentially](#Run-tests-sequentially) section in this document for further information. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence doesn't say anything that isn't already covered in "Run tests sequentially," below. Additionally, the aside's content isn't really relevant to the topic of this section, which is about creating test suites. I'd recommend thinking about the structure of this content differently.
Is there something that "Run tests sequentially" ought to tell developers, but doesn't? If so, add information there.
Is there something that "Convert test classes" ought to tell developers about concurrency, so they know to read "Run tests sequentially"? If so, work that into the prose in this section.
If your goal is to ensure that developers remember to think about concurrency, they should probably also remember to think about some of the other points in this doc. Perhaps you need to expand the Overview, to give developers a list of important points to consider when they migrate tests.
|
Hi @petermolnar-dev, thank you for the PR. It looks like your motivation for submitting it was to resolve the issue #863, is that correct? We realized upon reviewing that issue that it was probably resolved already by @iamleeg's subsequent PR #960 — or at least, it's not clear what additional information beyond that should be added — we just forgot to close #863 after #960 was merged. I'm going to close this PR for now given that. However, if you feel there is additional guidance on the Migrating document which should still be added, please clarify and we can re-open and reconsider this PR. |
Adding highlight and link to the XCTest Migration document to help understand and mitigate the parallel-by-default test execution.
Motivation:
Handling #863
Modifications:
Adding the Important aside, highlighting the parallel-by-default nature of Swift Testing test execution while migrating from XCTest.
Checklist: