Skip to content

Revert "Introduce a severity level for issues, and a 'warning' severity (#931)" #950

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

Merged
merged 1 commit into from
Feb 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Sources/Testing/ABI/EntryPoints/ABIEntryPoint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ extension ABIv0 {
/// callback.
public static var entryPoint: EntryPoint {
return { configurationJSON, recordHandler in
try await _entryPoint(
try await Testing.entryPoint(
configurationJSON: configurationJSON,
recordHandler: recordHandler
) == EXIT_SUCCESS
Expand Down Expand Up @@ -87,7 +87,7 @@ typealias ABIEntryPoint_v0 = @Sendable (
@usableFromInline func copyABIEntryPoint_v0() -> UnsafeMutableRawPointer {
let result = UnsafeMutablePointer<ABIEntryPoint_v0>.allocate(capacity: 1)
result.initialize { configurationJSON, recordHandler in
try await _entryPoint(
try await entryPoint(
configurationJSON: configurationJSON,
eventStreamVersionIfNil: -1,
recordHandler: recordHandler
Expand All @@ -104,7 +104,7 @@ typealias ABIEntryPoint_v0 = @Sendable (
///
/// This function will be removed (with its logic incorporated into
/// ``ABIv0/entryPoint-swift.type.property``) in a future update.
private func _entryPoint(
private func entryPoint(
configurationJSON: UnsafeRawBufferPointer?,
eventStreamVersionIfNil: Int? = nil,
recordHandler: @escaping @Sendable (_ recordJSON: UnsafeRawBufferPointer) -> Void
Expand Down
37 changes: 1 addition & 36 deletions Sources/Testing/ABI/EntryPoints/EntryPoint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ private import _TestingInternals
/// writes events to the standard error stream in addition to passing them
/// to this function.
///
/// - Returns: An exit code representing the result of running tests.
///
/// External callers cannot call this function directly. The can use
/// ``ABIv0/entryPoint-swift.type.property`` to get a reference to an ABI-stable
/// version of this function.
Expand All @@ -42,7 +40,7 @@ func entryPoint(passing args: __CommandLineArguments_v0?, eventHandler: Event.Ha

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

/// Whether or not to include tests with the `.hidden` trait when constructing
/// a test filter based on these arguments.
///
/// This property is intended for use in testing the testing library itself.
/// It is not parsed as a command-line argument.
var includeHiddenTests: Bool?

/// The value of the `--repetitions` argument.
public var repetitions: Int?

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

/// The value of the `--experimental-attachments-path` argument.
public var experimentalAttachmentsPath: String?

/// Whether or not the experimental warning issue severity feature should be
/// enabled.
///
/// This property is intended for use in testing the testing library itself.
/// It is not parsed as a command-line argument.
var isWarningIssueRecordedEventEnabled: Bool?
}

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

configuration.testFilter = filters.reduce(.unfiltered) { $0.combining(with: $1) }
if args.includeHiddenTests == true {
configuration.testFilter.includeHiddenTests = true
}

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

// Warning issues (experimental).
if args.isWarningIssueRecordedEventEnabled == true {
configuration.eventHandlingOptions.isWarningIssueRecordedEventEnabled = true
} else {
switch args.eventStreamVersion {
case .some(...0):
// If the event stream version was explicitly specified to a value < 1,
// disable the warning issue event to maintain legacy behavior.
configuration.eventHandlingOptions.isWarningIssueRecordedEventEnabled = false
default:
// Otherwise the requested event stream version is ≥ 1, so don't change
// the warning issue event setting.
break
}
}

return configuration
}

Expand Down
19 changes: 0 additions & 19 deletions Sources/Testing/ABI/v0/Encoded/ABIv0.EncodedIssue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,6 @@ extension ABIv0 {
/// assists in converting values to JSON; clients that consume this JSON are
/// expected to write their own decoders.
struct EncodedIssue: Sendable {
/// An enumeration representing the level of severity of a recorded issue.
///
/// For descriptions of individual cases, see ``Issue/Severity-swift.enum``.
enum Severity: String, Sendable {
case warning
case error
}

/// The severity of this issue.
///
/// - Warning: Severity is not yet part of the JSON schema.
var _severity: Severity

/// Whether or not this issue is known to occur.
var isKnown: Bool

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

init(encoding issue: borrowing Issue, in eventContext: borrowing Event.Context) {
_severity = switch issue.severity {
case .warning: .warning
case .error: .error
}

isKnown = issue.isKnown
sourceLocation = issue.sourceLocation
if let backtrace = issue.sourceContext.backtrace {
Expand All @@ -66,4 +48,3 @@ extension ABIv0 {
// MARK: - Codable

extension ABIv0.EncodedIssue: Codable {}
extension ABIv0.EncodedIssue.Severity: Codable {}
3 changes: 0 additions & 3 deletions Sources/Testing/ABI/v0/Encoded/ABIv0.EncodedMessage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ extension ABIv0 {
case `default`
case skip
case pass
case passWithWarnings = "_passWithWarnings"
case passWithKnownIssue
case fail
case difference
Expand All @@ -45,8 +44,6 @@ extension ABIv0 {
} else {
.pass
}
case .passWithWarnings:
.passWithWarnings
case .fail:
.fail
case .difference:
Expand Down
5 changes: 4 additions & 1 deletion Sources/Testing/Events/Event.swift
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,10 @@ extension Event {
if let configuration = configuration ?? Configuration.current {
// The caller specified a configuration, or the current task has an
// associated configuration. Post to either configuration's event handler.
if configuration.eventHandlingOptions.shouldHandleEvent(self) {
switch kind {
case .expectationChecked where !configuration.deliverExpectationCheckedEvents:
break
default:
configuration.handleEvent(self, in: context)
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,6 @@ extension Event.Symbol {
return "\(_ansiEscapeCodePrefix)90m\(symbolCharacter)\(_resetANSIEscapeCode)"
}
return "\(_ansiEscapeCodePrefix)92m\(symbolCharacter)\(_resetANSIEscapeCode)"
case .passWithWarnings:
return "\(_ansiEscapeCodePrefix)93m\(symbolCharacter)\(_resetANSIEscapeCode)"
case .fail:
return "\(_ansiEscapeCodePrefix)91m\(symbolCharacter)\(_resetANSIEscapeCode)"
case .warning:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,8 @@ extension Event {
/// The instant at which the test started.
var startInstant: Test.Clock.Instant

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

/// The number of known issues recorded for the test.
var knownIssueCount = 0
Expand Down Expand Up @@ -115,36 +114,27 @@ extension Event.HumanReadableOutputRecorder {
/// - graph: The graph to walk while counting issues.
///
/// - Returns: A tuple containing the number of issues recorded in `graph`.
private func _issueCounts(in graph: Graph<String, Event.HumanReadableOutputRecorder._Context.TestData?>?) -> (errorIssueCount: Int, warningIssueCount: Int, knownIssueCount: Int, totalIssueCount: Int, description: String) {
private func _issueCounts(in graph: Graph<String, Event.HumanReadableOutputRecorder._Context.TestData?>?) -> (issueCount: Int, knownIssueCount: Int, totalIssueCount: Int, description: String) {
guard let graph else {
return (0, 0, 0, 0, "")
return (0, 0, 0, "")
}
let errorIssueCount = graph.compactMap(\.value?.issueCount[.error]).reduce(into: 0, +=)
let warningIssueCount = graph.compactMap(\.value?.issueCount[.warning]).reduce(into: 0, +=)
let issueCount = graph.compactMap(\.value?.issueCount).reduce(into: 0, +=)
let knownIssueCount = graph.compactMap(\.value?.knownIssueCount).reduce(into: 0, +=)
let totalIssueCount = errorIssueCount + warningIssueCount + knownIssueCount
let totalIssueCount = issueCount + knownIssueCount

// Construct a string describing the issue counts.
let description = switch (errorIssueCount > 0, warningIssueCount > 0, knownIssueCount > 0) {
case (true, true, true):
" with \(totalIssueCount.counting("issue")) (including \(warningIssueCount.counting("warning")) and \(knownIssueCount.counting("known issue")))"
case (true, false, true):
let description = switch (issueCount > 0, knownIssueCount > 0) {
case (true, true):
" with \(totalIssueCount.counting("issue")) (including \(knownIssueCount.counting("known issue")))"
case (false, true, true):
" with \(warningIssueCount.counting("warning")) and \(knownIssueCount.counting("known issue"))"
case (false, false, true):
case (false, true):
" with \(knownIssueCount.counting("known issue"))"
case (true, true, false):
" with \(totalIssueCount.counting("issue")) (including \(warningIssueCount.counting("warning")))"
case (true, false, false):
case (true, false):
" with \(totalIssueCount.counting("issue"))"
case(false, true, false):
" with \(warningIssueCount.counting("warning"))"
case(false, false, false):
case(false, false):
""
}

return (errorIssueCount, warningIssueCount, knownIssueCount, totalIssueCount, description)
return (issueCount, knownIssueCount, totalIssueCount, description)
}
}

Expand Down Expand Up @@ -277,8 +267,7 @@ extension Event.HumanReadableOutputRecorder {
if issue.isKnown {
testData.knownIssueCount += 1
} else {
let issueCount = testData.issueCount[issue.severity] ?? 0
testData.issueCount[issue.severity] = issueCount + 1
testData.issueCount += 1
}
context.testData[id] = testData

Expand Down Expand Up @@ -366,15 +355,15 @@ extension Event.HumanReadableOutputRecorder {
let testData = testDataGraph?.value ?? .init(startInstant: instant)
let issues = _issueCounts(in: testDataGraph)
let duration = testData.startInstant.descriptionOfDuration(to: instant)
return if issues.errorIssueCount > 0 {
return if issues.issueCount > 0 {
CollectionOfOne(
Message(
symbol: .fail,
stringValue: "\(_capitalizedTitle(for: test)) \(testName) failed after \(duration)\(issues.description)."
)
) + _formattedComments(for: test)
} else {
[
[
Message(
symbol: .pass(knownIssueCount: issues.knownIssueCount),
stringValue: "\(_capitalizedTitle(for: test)) \(testName) passed after \(duration)\(issues.description)."
Expand Down Expand Up @@ -411,19 +400,13 @@ extension Event.HumanReadableOutputRecorder {
""
}
let symbol: Event.Symbol
let subject: String
let known: String
if issue.isKnown {
symbol = .pass(knownIssueCount: 1)
subject = "a known issue"
known = " known"
} else {
switch issue.severity {
case .warning:
symbol = .passWithWarnings
subject = "a warning"
case .error:
symbol = .fail
subject = "an issue"
}
symbol = .fail
known = "n"
}

var additionalMessages = [Message]()
Expand Down Expand Up @@ -452,13 +435,13 @@ extension Event.HumanReadableOutputRecorder {
let primaryMessage: Message = if parameterCount == 0 {
Message(
symbol: symbol,
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded \(subject)\(atSourceLocation): \(issue.kind)",
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded a\(known) issue\(atSourceLocation): \(issue.kind)",
conciseStringValue: String(describing: issue.kind)
)
} else {
Message(
symbol: symbol,
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded \(subject) with \(parameterCount.counting("argument")) \(labeledArguments)\(atSourceLocation): \(issue.kind)",
stringValue: "\(_capitalizedTitle(for: test)) \(testName) recorded a\(known) issue with \(parameterCount.counting("argument")) \(labeledArguments)\(atSourceLocation): \(issue.kind)",
conciseStringValue: String(describing: issue.kind)
)
}
Expand Down Expand Up @@ -515,7 +498,7 @@ extension Event.HumanReadableOutputRecorder {
let runStartInstant = context.runStartInstant ?? instant
let duration = runStartInstant.descriptionOfDuration(to: instant)

return if issues.errorIssueCount > 0 {
return if issues.issueCount > 0 {
[
Message(
symbol: .fail,
Expand Down
16 changes: 2 additions & 14 deletions Sources/Testing/Events/Recorder/Event.Symbol.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,10 @@ extension Event {
/// The symbol to use when a test passes.
///
/// - Parameters:
/// - knownIssueCount: The number of known issues recorded for the test.
/// The default value is `0`.
/// - knownIssueCount: The number of known issues encountered by the end
/// of the test.
case pass(knownIssueCount: Int = 0)

/// The symbol to use when a test passes with one or more warnings.
@_spi(Experimental)
case passWithWarnings

/// The symbol to use when a test fails.
case fail

Expand Down Expand Up @@ -66,8 +62,6 @@ extension Event.Symbol {
} else {
("\u{10105B}", "checkmark.diamond.fill")
}
case .passWithWarnings:
("\u{100123}", "questionmark.diamond.fill")
case .fail:
("\u{100884}", "xmark.diamond.fill")
case .difference:
Expand Down Expand Up @@ -128,9 +122,6 @@ extension Event.Symbol {
// Unicode: HEAVY CHECK MARK
return "\u{2714}"
}
case .passWithWarnings:
// Unicode: QUESTION MARK
return "\u{003F}"
case .fail:
// Unicode: HEAVY BALLOT X
return "\u{2718}"
Expand Down Expand Up @@ -166,9 +157,6 @@ extension Event.Symbol {
// Unicode: SQUARE ROOT
return "\u{221A}"
}
case .passWithWarnings:
// Unicode: QUESTION MARK
return "\u{003F}"
case .fail:
// Unicode: MULTIPLICATION SIGN
return "\u{00D7}"
Expand Down
Loading