Skip to content

Commit 6a49142

Browse files
stmontgomerystmontgomery
authored
Introduce a severity level for issues, and a 'warning' severity (#931)
This introduces the concept of severity to the `Issue` type, represented by a new enum `Issue.Severity` with two cases: `.warning` and `.error`. Error is the default severity for all issues, matching current behavior, but warning is provided as a new option which does not cause the test the issue is associated with to be marked as a failure. In this PR, these are [SPI](https://github.com/swiftlang/swift-testing/blob/main/Documentation/SPI.md) but they could be considered for promotion to public API eventually. Additional work would be needed to permit test authors to record issues with severity < `.error`, since APIs like `Issue.record()` are not being modified at this time to allow customizing the severity. ### Motivation: There are certain situations where a problem may arise during a test that doesn't necessarily affect its outcome or signal an important problem, but is worth calling attention to. A specific example use case I have in mind is to allow the testing library to record a warning issue about problems with the arguments passed to a parameterized test, such as having duplicate arguments. ### Modifications: - Introduce `Issue.Severity` as an SPI enum. - Introduce an SPI property `severity` to `Issue` with default value `.error`. - Modify entry point logic to exit with `EXIT_SUCCESS` if all issues recorded had severity < `.error`. - Modify console output formatting logic and data structures to represent warning issues sensibly. ### 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. - [x] Add new tests
1 parent 63eb1d9 commit 6a49142

18 files changed

+468
-109
lines changed

‎Sources/Testing/ABI/EntryPoints/ABIEntryPoint.swift‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ extension ABIv0 {
4747
/// callback.
4848
public static var entryPoint: EntryPoint {
4949
return { configurationJSON, recordHandler in
50-
try await Testing.entryPoint(
50+
try await _entryPoint(
5151
configurationJSON: configurationJSON,
5252
recordHandler: recordHandler
5353
) == EXIT_SUCCESS
@@ -87,7 +87,7 @@ typealias ABIEntryPoint_v0 = @Sendable (
8787
@usableFromInline func copyABIEntryPoint_v0() -> UnsafeMutableRawPointer {
8888
let result = UnsafeMutablePointer<ABIEntryPoint_v0>.allocate(capacity: 1)
8989
result.initialize { configurationJSON, recordHandler in
90-
try await entryPoint(
90+
try await _entryPoint(
9191
configurationJSON: configurationJSON,
9292
eventStreamVersionIfNil: -1,
9393
recordHandler: recordHandler
@@ -104,7 +104,7 @@ typealias ABIEntryPoint_v0 = @Sendable (
104104
///
105105
/// This function will be removed (with its logic incorporated into
106106
/// ``ABIv0/entryPoint-swift.type.property``) in a future update.
107-
private func entryPoint(
107+
private func _entryPoint(
108108
configurationJSON: UnsafeRawBufferPointer?,
109109
eventStreamVersionIfNil: Int? = nil,
110110
recordHandler: @escaping @Sendable (_ recordJSON: UnsafeRawBufferPointer) -> Void

‎Sources/Testing/ABI/EntryPoints/EntryPoint.swift‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ private import _TestingInternals
2020
/// writes events to the standard error stream in addition to passing them
2121
/// to this function.
2222
///
23+
/// - Returns: An exit code representing the result of running tests.
24+
///
2325
/// External callers cannot call this function directly. The can use
2426
/// ``ABIv0/entryPoint-swift.type.property`` to get a reference to an ABI-stable
2527
/// version of this function.
@@ -40,7 +42,7 @@ func entryPoint(passing args: __CommandLineArguments_v0?, eventHandler: Event.Ha
4042

4143
// Set up the event handler.
4244
configuration.eventHandler = { [oldEventHandler = configuration.eventHandler] event, context in
43-
if case let .issueRecorded(issue) = event.kind, !issue.isKnown {
45+
if case let .issueRecorded(issue) = event.kind, !issue.isKnown, issue.severity >= .error {
4446
exitCode.withLock { exitCode in
4547
exitCode = EXIT_FAILURE
4648
}
@@ -270,6 +272,13 @@ public struct __CommandLineArguments_v0: Sendable {
270272
/// The value(s) of the `--skip` argument.
271273
public var skip: [String]?
272274

275+
/// Whether or not to include tests with the `.hidden` trait when constructing
276+
/// a test filter based on these arguments.
277+
///
278+
/// This property is intended for use in testing the testing library itself.
279+
/// It is not parsed as a command-line argument.
280+
var includeHiddenTests: Bool?
281+
273282
/// The value of the `--repetitions` argument.
274283
public var repetitions: Int?
275284

@@ -278,6 +287,13 @@ public struct __CommandLineArguments_v0: Sendable {
278287

279288
/// The value of the `--experimental-attachments-path` argument.
280289
public var experimentalAttachmentsPath: String?
290+
291+
/// Whether or not the experimental warning issue severity feature should be
292+
/// enabled.
293+
///
294+
/// This property is intended for use in testing the testing library itself.
295+
/// It is not parsed as a command-line argument.
296+
var isWarningIssueRecordedEventEnabled: Bool?
281297
}
282298

283299
extension __CommandLineArguments_v0: Codable {
@@ -517,6 +533,9 @@ public func configurationForEntryPoint(from args: __CommandLineArguments_v0) thr
517533
filters.append(try testFilter(forRegularExpressions: args.skip, label: "--skip", membership: .excluding))
518534

519535
configuration.testFilter = filters.reduce(.unfiltered) { $0.combining(with: $1) }
536+
if args.includeHiddenTests == true {
537+
configuration.testFilter.includeHiddenTests = true
538+
}
520539

521540
// Set up the iteration policy for the test run.
522541
var repetitionPolicy: Configuration.RepetitionPolicy = .once
@@ -547,6 +566,22 @@ public func configurationForEntryPoint(from args: __CommandLineArguments_v0) thr
547566
configuration.exitTestHandler = ExitTest.handlerForEntryPoint()
548567
#endif
549568

569+
// Warning issues (experimental).
570+
if args.isWarningIssueRecordedEventEnabled == true {
571+
configuration.eventHandlingOptions.isWarningIssueRecordedEventEnabled = true
572+
} else {
573+
switch args.eventStreamVersion {
574+
case .some(...0):
575+
// If the event stream version was explicitly specified to a value < 1,
576+
// disable the warning issue event to maintain legacy behavior.
577+
configuration.eventHandlingOptions.isWarningIssueRecordedEventEnabled = false
578+
default:
579+
// Otherwise the requested event stream version is ≥ 1, so don't change
580+
// the warning issue event setting.
581+
break
582+
}
583+
}
584+
550585
return configuration
551586
}
552587

‎Sources/Testing/ABI/v0/Encoded/ABIv0.EncodedIssue.swift‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,19 @@ extension ABIv0 {
1616
/// assists in converting values to JSON; clients that consume this JSON are
1717
/// expected to write their own decoders.
1818
struct EncodedIssue: Sendable {
19+
/// An enumeration representing the level of severity of a recorded issue.
20+
///
21+
/// For descriptions of individual cases, see ``Issue/Severity-swift.enum``.
22+
enum Severity: String, Sendable {
23+
case warning
24+
case error
25+
}
26+
27+
/// The severity of this issue.
28+
///
29+
/// - Warning: Severity is not yet part of the JSON schema.
30+
var _severity: Severity
31+
1932
/// Whether or not this issue is known to occur.
2033
var isKnown: Bool
2134

@@ -33,6 +46,11 @@ extension ABIv0 {
3346
var _error: EncodedError?
3447

3548
init(encoding issue: borrowing Issue, in eventContext: borrowing Event.Context) {
49+
_severity = switch issue.severity {
50+
case .warning: .warning
51+
case .error: .error
52+
}
53+
3654
isKnown = issue.isKnown
3755
sourceLocation = issue.sourceLocation
3856
if let backtrace = issue.sourceContext.backtrace {
@@ -48,3 +66,4 @@ extension ABIv0 {
4866
// MARK: - Codable
4967

5068
extension ABIv0.EncodedIssue: Codable {}
69+
extension ABIv0.EncodedIssue.Severity: Codable {}

‎Sources/Testing/ABI/v0/Encoded/ABIv0.EncodedMessage.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ extension ABIv0 {
2525
case `default`
2626
case skip
2727
case pass
28+
case passWithWarnings = "_passWithWarnings"
2829
case passWithKnownIssue
2930
case fail
3031
case difference
@@ -44,6 +45,8 @@ extension ABIv0 {
4445
} else {
4546
.pass
4647
}
48+
case .passWithWarnings:
49+
.passWithWarnings
4750
case .fail:
4851
.fail
4952
case .difference:

‎Sources/Testing/Events/Event.swift‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -290,10 +290,7 @@ extension Event {
290290
if let configuration = configuration ?? Configuration.current {
291291
// The caller specified a configuration, or the current task has an
292292
// associated configuration. Post to either configuration's event handler.
293-
switch kind {
294-
case .expectationChecked where !configuration.deliverExpectationCheckedEvents:
295-
break
296-
default:
293+
if configuration.eventHandlingOptions.shouldHandleEvent(self) {
297294
configuration.handleEvent(self, in: context)
298295
}
299296
} else {

‎Sources/Testing/Events/Recorder/Event.ConsoleOutputRecorder.swift‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,8 @@ extension Event.Symbol {
162162
return "\(_ansiEscapeCodePrefix)90m\(symbolCharacter)\(_resetANSIEscapeCode)"
163163
}
164164
return "\(_ansiEscapeCodePrefix)92m\(symbolCharacter)\(_resetANSIEscapeCode)"
165+
case .passWithWarnings:
166+
return "\(_ansiEscapeCodePrefix)93m\(symbolCharacter)\(_resetANSIEscapeCode)"
165167
case .fail:
166168
return "\(_ansiEscapeCodePrefix)91m\(symbolCharacter)\(_resetANSIEscapeCode)"
167169
case .warning:

‎Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift‎

Lines changed: 39 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ extension Event {
5656
/// The instant at which the test started.
5757
var startInstant: Test.Clock.Instant
5858

59-
/// The number of issues recorded for the test.
60-
var issueCount = 0
59+
/// The number of issues recorded for the test, grouped by their
60+
/// level of severity.
61+
var issueCount: [Issue.Severity: Int] = [:]
6162

6263
/// The number of known issues recorded for the test.
6364
var knownIssueCount = 0
@@ -114,27 +115,36 @@ extension Event.HumanReadableOutputRecorder {
114115
/// - graph: The graph to walk while counting issues.
115116
///
116117
/// - Returns: A tuple containing the number of issues recorded in `graph`.
117-
private func _issueCounts(in graph: Graph<String, Event.HumanReadableOutputRecorder._Context.TestData?>?) -> (issueCount: Int, knownIssueCount: Int, totalIssueCount: Int, description: String) {
118+
private func _issueCounts(in graph: Graph<String, Event.HumanReadableOutputRecorder._Context.TestData?>?) -> (errorIssueCount: Int, warningIssueCount: Int, knownIssueCount: Int, totalIssueCount: Int, description: String) {
118119
guard let graph else {
119-
return (0, 0, 0, "")
120+
return (0, 0, 0, 0, "")
120121
}
121-
let issueCount = graph.compactMap(\.value?.issueCount).reduce(into: 0, +=)
122+
let errorIssueCount = graph.compactMap(\.value?.issueCount[.error]).reduce(into: 0, +=)
123+
let warningIssueCount = graph.compactMap(\.value?.issueCount[.warning]).reduce(into: 0, +=)
122124
let knownIssueCount = graph.compactMap(\.value?.knownIssueCount).reduce(into: 0, +=)
123-
let totalIssueCount = issueCount + knownIssueCount
125+
let totalIssueCount = errorIssueCount + warningIssueCount + knownIssueCount
124126

125127
// Construct a string describing the issue counts.
126-
let description = switch (issueCount > 0, knownIssueCount > 0) {
127-
case (true, true):
128+
let description = switch (errorIssueCount > 0, warningIssueCount > 0, knownIssueCount > 0) {
129+
case (true, true, true):
130+
" with \(totalIssueCount.counting("issue")) (including \(warningIssueCount.counting("warning")) and \(knownIssueCount.counting("known issue")))"
131+
case (true, false, true):
128132
" with \(totalIssueCount.counting("issue")) (including \(knownIssueCount.counting("known issue")))"
129-
case (false, true):
133+
case (false, true, true):
134+
" with \(warningIssueCount.counting("warning")) and \(knownIssueCount.counting("known issue"))"
135+
case (false, false, true):
130136
" with \(knownIssueCount.counting("known issue"))"
131-
case (true, false):
137+
case (true, true, false):
138+
" with \(totalIssueCount.counting("issue")) (including \(warningIssueCount.counting("warning")))"
139+
case (true, false, false):
132140
" with \(totalIssueCount.counting("issue"))"
133-
case(false, false):
141+
case(false, true, false):
142+
" with \(warningIssueCount.counting("warning"))"
143+
case(false, false, false):
134144
""
135145
}
136146

137-
return (issueCount, knownIssueCount, totalIssueCount, description)
147+
return (errorIssueCount, warningIssueCount, knownIssueCount, totalIssueCount, description)
138148
}
139149
}
140150

@@ -267,7 +277,8 @@ extension Event.HumanReadableOutputRecorder {
267277
if issue.isKnown {
268278
testData.knownIssueCount += 1
269279
} else {
270-
testData.issueCount += 1
280+
let issueCount = testData.issueCount[issue.severity] ?? 0
281+
testData.issueCount[issue.severity] = issueCount + 1
271282
}
272283
context.testData[id] = testData
273284

@@ -355,15 +366,15 @@ extension Event.HumanReadableOutputRecorder {
355366
let testData = testDataGraph?.value ?? .init(startInstant: instant)
356367
let issues = _issueCounts(in: testDataGraph)
357368
let duration = testData.startInstant.descriptionOfDuration(to: instant)
358-
return if issues.issueCount > 0 {
369+
return if issues.errorIssueCount > 0 {
359370
CollectionOfOne(
360371
Message(
361372
symbol: .fail,
362373
stringValue: "\(_capitalizedTitle(for: test)) \(testName) failed after \(duration)\(issues.description)."
363374
)
364375
) + _formattedComments(for: test)
365376
} else {
366-
[
377+
[
367378
Message(
368379
symbol: .pass(knownIssueCount: issues.knownIssueCount),
369380
stringValue: "\(_capitalizedTitle(for: test)) \(testName) passed after \(duration)\(issues.description)."
@@ -400,13 +411,19 @@ extension Event.HumanReadableOutputRecorder {
400411
""
401412
}
402413
let symbol: Event.Symbol
403-
let known: String
414+
let subject: String
404415
if issue.isKnown {
405416
symbol = .pass(knownIssueCount: 1)
406-
known = " known"
417+
subject = "a known issue"
407418
} else {
408-
symbol = .fail
409-
known = "n"
419+
switch issue.severity {
420+
case .warning:
421+
symbol = .passWithWarnings
422+
subject = "a warning"
423+
case .error:
424+
symbol = .fail
425+
subject = "an issue"
426+
}
410427
}
411428

412429
var additionalMessages = [Message]()
@@ -435,13 +452,13 @@ extension Event.HumanReadableOutputRecorder {
435452
let primaryMessage: Message = if parameterCount == 0 {
436453
Message(
437454
symbol: symbol,
438-
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded a\(known) issue\(atSourceLocation): \(issue.kind)",
455+
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded \(subject)\(atSourceLocation): \(issue.kind)",
439456
conciseStringValue: String(describing: issue.kind)
440457
)
441458
} else {
442459
Message(
443460
symbol: symbol,
444-
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded a\(known) issue with \(parameterCount.counting("argument")) \(labeledArguments)\(atSourceLocation): \(issue.kind)",
461+
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded \(subject) with \(parameterCount.counting("argument")) \(labeledArguments)\(atSourceLocation): \(issue.kind)",
445462
conciseStringValue: String(describing: issue.kind)
446463
)
447464
}
@@ -498,7 +515,7 @@ extension Event.HumanReadableOutputRecorder {
498515
let runStartInstant = context.runStartInstant ?? instant
499516
let duration = runStartInstant.descriptionOfDuration(to: instant)
500517

501-
return if issues.issueCount > 0 {
518+
return if issues.errorIssueCount > 0 {
502519
[
503520
Message(
504521
symbol: .fail,

‎Sources/Testing/Events/Recorder/Event.Symbol.swift‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,14 @@ extension Event {
2222
/// The symbol to use when a test passes.
2323
///
2424
/// - Parameters:
25-
/// - knownIssueCount: The number of known issues encountered by the end
26-
/// of the test.
25+
/// - knownIssueCount: The number of known issues recorded for the test.
26+
/// The default value is `0`.
2727
case pass(knownIssueCount: Int = 0)
2828

29+
/// The symbol to use when a test passes with one or more warnings.
30+
@_spi(Experimental)
31+
case passWithWarnings
32+
2933
/// The symbol to use when a test fails.
3034
case fail
3135

@@ -62,6 +66,8 @@ extension Event.Symbol {
6266
} else {
6367
("\u{10105B}", "checkmark.diamond.fill")
6468
}
69+
case .passWithWarnings:
70+
("\u{100123}", "questionmark.diamond.fill")
6571
case .fail:
6672
("\u{100884}", "xmark.diamond.fill")
6773
case .difference:
@@ -122,6 +128,9 @@ extension Event.Symbol {
122128
// Unicode: HEAVY CHECK MARK
123129
return "\u{2714}"
124130
}
131+
case .passWithWarnings:
132+
// Unicode: QUESTION MARK
133+
return "\u{003F}"
125134
case .fail:
126135
// Unicode: HEAVY BALLOT X
127136
return "\u{2718}"
@@ -157,6 +166,9 @@ extension Event.Symbol {
157166
// Unicode: SQUARE ROOT
158167
return "\u{221A}"
159168
}
169+
case .passWithWarnings:
170+
// Unicode: QUESTION MARK
171+
return "\u{003F}"
160172
case .fail:
161173
// Unicode: MULTIPLICATION SIGN
162174
return "\u{00D7}"

0 commit comments

Comments
 (0)