-
Notifications
You must be signed in to change notification settings - Fork 128
Clarify note about supported target types in "Defining test functions" article #1291
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
Conversation
| testing library into an application, library, or binary target isn't | ||
| - Note: Only import the testing library into a test target or library meant for | ||
| test targets. Importing the testing library into a target intended for | ||
| distribution such as an application, app library, or executable target isn't |
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.
Open to suggestions for better phrasing @iamleeg, but I hope my intent is clear!
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.
Looks good, thank you!
|
|
||
| - Note: Only import the testing library into a test target. Importing the | ||
| testing library into an application, library, or binary target isn't | ||
| - Note: Only import the testing library into a test target or library meant for |
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.
Very nit: would it be clearer to say "library used only in test targets"? I can see the original wording as being interpreted overly broadly (e.g. some sort of utils used in both an executable and test library)
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.
I see what you're going for there. I worded it this way to keep it slightly more open because it's also supported (though less common) to have one test-only library which is used exclusively by another test-only library which in turn is used by exclusively by test targets. So the guidance is really "keep the transitive closure of these libraries confined to testing contexts". It's just that I'd like this doc phrasing to read naturally. So I felt "meant for test targets" captured that well enough, without excluding that use case
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 for example is valid in a library if you are building from source:
#if canImport(Testing)
public func foo(_ test: Test) {
...
}
#endif
This adjusts the wording of a
- Note:in our Defining test functions documentation article which advises that users only import the testing library in test targets. It expands the kinds of supported targets to include "library targets meant for test targets", which is a supported use case.Motivation:
The wording should not imply that it's never supported to vend a library which extends the testing library. There is an important distinction between libraries which are distributed to end users (e.g. those bundles within apps or alongside executables) vs. libraries intended solely to support test targets or other testing libraries.
Checklist: