Add withKnownIssue comments to known Issues #1014
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,10 +93,20 @@ extension Event.HumanReadableOutputRecorder { | |
| /// - Parameters: | ||
| /// - comments: The comments that should be formatted. | ||
| /// | ||
| /// - Returns: An array of formatted messages representing `comments`, or an | ||
| /// empty array if there are none. | ||
| private func _formattedComments(_ comments: [Comment]) -> [Message] { | ||
| comments.map(_formattedComment) | ||
| } | ||
|
|
||
| /// Get a string representing a single comment, formatted for output. | ||
| /// | ||
| /// - Parameters: | ||
| /// - comment: The comment that should be formatted. | ||
| /// | ||
| /// - Returns: A formatted message representing `comment`. | ||
| private func _formattedComment(_ comment: Comment) -> Message { | ||
| Message(symbol: .details, stringValue: comment.rawValue) | ||
| } | ||
|
|
||
| /// Get a string representing the comments attached to a test, formatted for | ||
|
|
@@ -443,6 +453,15 @@ extension Event.HumanReadableOutputRecorder { | |
| additionalMessages.append(Message(symbol: .difference, stringValue: differenceDescription)) | ||
| } | ||
| additionalMessages += _formattedComments(issue.comments) | ||
| if let knownIssueComment = issue.knownIssueContext?.comment { | ||
| if case .errorCaught = issue.kind { | ||
|
There was a problem hiding this comment. Can you explain why this specific case is being singled out? There was a problem hiding this comment. Pushed an updated comment, how does it look to you? There was a problem hiding this comment. Seems like the correct fix might be to remove the comment from There was a problem hiding this comment. Note for compatibility, we would want to clear the withKnownIssue {
...
} matching: { issue in
issue.comments.first?.contains("abc") ?? false
}(Something like that.) There was a problem hiding this comment.
We certainly could, but that has a downside that I mentioned in #1014 (comment):
There was a problem hiding this comment. Yes, but I'm trying not to introduce any regressions along the way. There was a problem hiding this comment. I appreciate that :) And that's a good goal—but we also need to be mindful of creating technical debt for ourselves. There was a problem hiding this comment. OK, so do you think I should take approach 2 despite the regression in Xcode and the weirdness of There was a problem hiding this comment. We discussed this and decided to remove the comment entirely from @Test func knownIssueCommentPassedToIssueMatcher() throws {
struct E: Error {}
try withKnownIssue("Known Issue Comment") {
throw E()
} matching: { issue in
print("Comments passed to matcher:", issue.comments)
// prints: Comments passed to matcher: ["Known Issue Comment"]
// which seems unhelpful since we can see the comment a few lines up in the withKnownIssue call
return true
}
}```There was a problem hiding this comment. I agree we should stop including the comment passed to |
||
| // `_matchError()` put the known issue comment in `issue.comments` | ||
| // when it first created the issue, so it's already included in | ||
| // `additionalMessages`. | ||
| } else { | ||
| additionalMessages.append(_formattedComment(knownIssueComment)) | ||
| } | ||
| } | ||
|
|
||
| if verbosity > 0, case let .expectationFailed(expectation) = issue.kind { | ||
| let expression = expectation.evaluatedExpression | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -497,6 +497,35 @@ struct EventRecorderTests { | |
| recorder.record(Event(.runEnded, testID: nil, testCaseID: nil), in: context) | ||
| } | ||
| } | ||
|
|
||
| @Test( | ||
| "HumanReadableOutputRecorder includes known issue comment in messages array", | ||
| arguments: [ | ||
| ("recordWithoutKnownIssueComment()", ["#expect comment"]), | ||
| ("recordWithKnownIssueComment()", ["#expect comment", "withKnownIssue comment"]), | ||
| ("throwWithoutKnownIssueComment()", []), | ||
| ("throwWithKnownIssueComment()", ["withKnownIssue comment"]), | ||
| ] | ||
| ) | ||
| func knownIssueComments(testName: String, expectedComments: [String]) async throws { | ||
| var configuration = Configuration() | ||
| let recorder = Event.HumanReadableOutputRecorder() | ||
| let messages = Locked<[Event.HumanReadableOutputRecorder.Message]>(rawValue: []) | ||
| configuration.eventHandler = { event, context in | ||
| guard case .issueRecorded = event.kind else { return } | ||
| messages.withLock { | ||
| $0.append(contentsOf: recorder.record(event, in: context)) | ||
| } | ||
| } | ||
|
|
||
| await runTestFunction(named: testName, in: PredictablyFailingKnownIssueTests.self, configuration: configuration) | ||
|
|
||
| // The first message is something along the lines of "Test foo recorded a | ||
| // known issue" and includes a source location, so is inconvenient to | ||
| // include in our expectation here. | ||
| let actualComments = messages.rawValue.dropFirst().map(\.stringValue) | ||
| #expect(actualComments == expectedComments) | ||
| } | ||
| } | ||
|
|
||
| // MARK: - Fixtures | ||
|
|
@@ -639,3 +668,35 @@ struct EventRecorderTests { | |
| #expect(arg > 0) | ||
| } | ||
| } | ||
|
|
||
| @Suite(.hidden) struct PredictablyFailingKnownIssueTests { | ||
| @Test(.hidden) | ||
| func recordWithoutKnownIssueComment() { | ||
| withKnownIssue { | ||
| #expect(Bool(false), "#expect comment") | ||
| } | ||
| } | ||
|
|
||
| @Test(.hidden) | ||
| func recordWithKnownIssueComment() { | ||
| withKnownIssue("withKnownIssue comment") { | ||
| #expect(Bool(false), "#expect comment") | ||
| } | ||
| } | ||
|
|
||
| @Test(.hidden) | ||
| func throwWithoutKnownIssueComment() { | ||
| withKnownIssue { | ||
| struct TheError: Error {} | ||
| throw TheError() | ||
| } | ||
| } | ||
|
|
||
| @Test(.hidden) | ||
| func throwWithKnownIssueComment() { | ||
| withKnownIssue("withKnownIssue comment") { | ||
| struct TheError: Error {} | ||
| throw TheError() | ||
| } | ||
| } | ||
| } | ||
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.