[ST-NNNN] Add ConditionTrait.evaluate() #2740
Conversation
| of the callback in `Kind.conditional`: | ||
|
|
||
| ```swift | ||
| public typealias EvaluationResult = (wasMet: Bool, comment: Comment?) |
There was a problem hiding this comment.
I have a vague recollection of us discussing whether this Comment? was necessary in the first place but I am having trouble finding it. Did I imagine it?
There was a problem hiding this comment.
I believe that would be this message:
https://forums.swift.org/t/pitch-introduce-conditiontrait-evaluate/77242/8
Since there wasn't a conclusion on that, I left it in.
There was a problem hiding this comment.
While reviewing the code I found that we can eliminate this "override comment", so I posted swiftlang/swift-testing#1036
I think we can and should simplify this proposal by eliminating the need for the EvaluationResult tuple and have the evaluate() method just return Bool.
There was a problem hiding this comment.
Should I take that as authoritative, or does anyone else need to weigh in?
There was a problem hiding this comment.
Stuart and I discussed this at the Testing Workgroup meeting yesterday. While we see value in returning a customized comment, it's not actually functionality that exists today. The code inside Swift Testing to customize a comment is, in effect, dead and there's no way to actually return a custom comment here.
If that's something that you want to pursue, we can still do so, but we should probably do it as a separate proposal.
I hope that makes sense?
There was a problem hiding this comment.
Sure. I'm not at all pushing to keep the comment, I was just trying to make my change low-impact.
stmontgomery
added
the
workgroup: needs development
grynspan
changed the title
…1036) This removes a mechanism in the implementation of `ConditionTrait` for a condition closure to provide an "override" comment. ### Motivation: While reviewing a draft evolution proposal for `ConditionTrait` (swiftlang/swift-evolution#2740) I realized that we don't use this "comment override" mechanism anywhere in the testing library. I believe we did back when it was first added, but it's no longer used. Removing this would allow the public API being proposed above to be simplified: the `evaluate()` method could return `Bool` instead of a tuple. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
stmontgomery
added
workgroup: ready
Proposal document moved from swiftlang/swift-testing#909