TaskLocal test trait#3329
Conversation
Corrected typos and improved clarity in the documentation regarding the task local test trait.
|
|
||
| ## Alternatives considered | ||
|
|
||
| A more general name for this API could be used instead of `.taskLocal`: |
There was a problem hiding this comment.
I think the workgroup discussed a couple of other possible names for this trait; please include any you remember here. (I specifically mentioned withValue<T>(_: T, for taskLocal: TaskLocal<T>), and I think somebody else had at least one other suggestion but I don't recall offhand what it was.)
There was a problem hiding this comment.
My suggestion was .binding(T, to: V). The verb "bind" has some precedence in the TaskLocal documentation.
grynspan
left a comment
There was a problem hiding this comment.
Minor comments only. Please make sure to update the proposal with any corresponding changes in the implementation PR!
Co-authored-by: Jonathan Grynspan <grynspan@me.com>
Co-authored-by: Jonathan Grynspan <grynspan@me.com>
ktoso
left a comment
There was a problem hiding this comment.
This looks good to me, just a minor nitpick about avoiding the word override (we don't really use that term with task locals).
The naming .taskLocal(..., value) also sounds good to me
|
|
||
| There are two primary motivations for adding this convenience: | ||
|
|
||
| * Task locals are by far the [most common] reason to define custom `TestScoping` |
There was a problem hiding this comment.
| * Task locals are by far the [most common] reason to define custom `TestScoping` | |
| * Task locals are by far the most common reason to define custom `TestScoping` |
not sure why the [], just "the most common ..."
There was a problem hiding this comment.
The brackets link just below to some context: https://github.com/search?q=testscoping+language%3ASwift+&type=code
We just found that a lot of the public TestScoping conformances are related to task locals.
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Co-authored-by: Konrad `ktoso` Malawski <konrad.malawski@project13.pl>
Updated terminology from 'overriding' to 'binding' for clarity in the context of task locals. Adjusted related descriptions to reflect this change throughout the document.
|
@grynspan Requested changes applied! Let us know if there's anything else. |
|
|
||
| In [ST-0007], test scoping traits were introduced to allow wrapping the execution | ||
| of tests so that code could be run before and after each test. The main | ||
| motivation for this feature is overriding [`TaskLocal`s][SE-0311: TaskLocal] for tests, and the |
There was a problem hiding this comment.
Hmm, this still isn't showing the link in the rendered version of the document. Might be a markdown syntax quirk
There was a problem hiding this comment.
This might work better as a standard markdown link instead of the brackets link.
| /// A type that binds a task local value for the scope of a test. | ||
| /// | ||
| /// To add this trait to a test, use ``Trait/taskLocal(_:_:)``. | ||
| public struct TaskLocalTrait<Value: Sendable>: SuiteTrait, TestScoping, TestTrait { |
There was a problem hiding this comment.
Minor stylistic thing, but I'd list TestTrait before TestScoping—it's a trait type first and foremost
| If Swift macros gain the ability to see symbols defined by other macros, we will be able to drop | ||
| the restriction that task locals be defined outside the test target. |
There was a problem hiding this comment.
I understand where this comes from, but the phrasing of "…we will be able to drop the restriction…" could be a little confusing: the API itself isn't imposing any such restriction, it's a consequence of current language restrictions outside the scope of this proposal.
I'd suggest tweaking this by saying something like: if this macro limitation were resolved, then this proposed API would become usable in more contexts/situations. The thing I'm looking to clarify is that no further action or change to the API would be required; it would "only" require a change to the language or macro system.
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Co-authored-by: Stuart Montgomery <smontgomery@apple.com>
Corrected references to TaskLocal in the motivation section and updated the link formatting.
A proposal for a new
.taskLocaltest trait for overriding task locals in tests.View the rendered proposal.