Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,23 @@ extension Event.HumanReadableOutputRecorder {

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

/// Returns a formatted string describing the number of arguments in a test, based on verbosity level.
///
/// - Parameters:
/// - test: to get the number of `testCases` out of a ``Test``.
/// - verbose: If the level is very verbose, a detailed description is returned.
///
/// - Returns: A string describing the number of test cases in the test, or an empty string if it's not very verbose level.
///
private func _includeNumberOfTestCasesIfNeeded(for test: Test, verbose: Int) -> String {
if verbose == 2 && !test.isSuite { // very verbose
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably mean >= 2, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you want to count test cases, you should use test.isParameterized, not !test.isSuite, since it doesn't make sense to count test cases for a non-parameterized test function either.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

let testCasesCount = test.testCases?.count(where: { _ in true }) ?? 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling testCases here will produce a second sequence of test cases, which makes getting the number of test cases an O(n) operation instead of O(1). What we would normally do is keep a running count of test cases in the TestData structure, incrementing it for each .testCaseStarted event.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, it helps me understand the way you guys think 🙏.

Honestly, it was my first idea; however, I thought it is the same due to the fact that O(n) gets distributed among each test run and also adds a negligible amount of miscalculation to the compute time (because it happens after the start instant as well as it adds 8 bytes of additional data to the test data). For those reasons, I went with the second solution.

I fixed the issue.

return" with \(testCasesCount) \(testCasesCount > 1 ? "test cases" : "test case")"
}
return ""
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: stray newline? :)

}

/// Generate a title for the specified test (either "Test" or "Suite"),
Expand Down Expand Up @@ -370,14 +387,14 @@ extension Event.HumanReadableOutputRecorder {
CollectionOfOne(
Message(
symbol: .fail,
stringValue: "\(_capitalizedTitle(for: test)) \(testName) failed after \(duration)\(issues.description)."
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfTestCasesIfNeeded(for: test, verbose: verbosity)) 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)."
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfTestCasesIfNeeded(for: test, verbose: verbosity)) passed after \(duration)\(issues.description)."
)
]
}
Expand Down
45 changes: 45 additions & 0 deletions Tests/TestingTests/EventRecorderTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,45 @@ struct EventRecorderTests {
.first != nil
)
}

@available(_regexAPI, *)
@Test(
"number of arguments based on verbosity level at the end of test run",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The display name of this test can be rephrased now to not mention verbosity. I'd also prefer if we could capitalize the first word to match prevailing style.

arguments: [
("f()", #".* Test f\(\) failed after .*"# , 0),
("f()", #".* Test f\(\) with 1 test case failed after .*"# , 2),
("d(_:)", #".* Test d\(_:\) with .+ test cases passed after.*"# , 2),
("PredictablyFailingTests", #".* Suite PredictablyFailingTests failed after .*"# , 1),
("PredictablyFailingTests", #".* Suite PredictablyFailingTests failed after .*"# , 2),
]
)
func numberOfArgumentsAtTheEndOfTests(testName: String, expectedPattern: String, verbosity: Int) async throws {
let stream = Stream()

var configuration = Configuration()
let eventRecorder = Event.ConsoleOutputRecorder(writingUsing: stream.write)
configuration.eventHandler = { event, context in
eventRecorder.record(event, in: context)
}
configuration.verbosity = verbosity

await runTest(for: PredictablyFailingTests.self, configuration: configuration)

let buffer = stream.buffer.rawValue
if testsWithSignificantIOAreEnabled {
print(buffer, terminator: "")
}

let aurgmentRegex = try Regex(expectedPattern)

#expect(
(try buffer
.split(whereSeparator: \.isNewline)
.compactMap(aurgmentRegex.wholeMatch(in:))
.first) != nil
)
}


@available(_regexAPI, *)
@Test(
Expand Down Expand Up @@ -536,6 +575,11 @@ struct EventRecorderTests {
#expect(Bool(false))
}
}

@Test(.hidden, arguments: [1, 2, 3])
func d(_ arg: Int) {
#expect(arg > 0)
}

@Test(.hidden) func g() {
#expect(Bool(false))
Expand Down Expand Up @@ -566,3 +610,4 @@ struct EventRecorderTests {
}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: stray newline?