Skip to content

Show the number of test cases that ran at the end of a test #972

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.

Sign up for GitHub

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 14 commits into from
Feb 25, 2025
Merged

Show the number of test cases that ran at the end of a test #972

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
14184b1
[943] minor change: - show number of args passed to at the end of a test
hrayatnia Feb 23, 2025
a1f7b4b
minor change: - update test cases & excluding suites
hrayatnia Feb 24, 2025
928f67b
Rename _includeNumberOfTestCases to _includeNumberOfTestCasesIfNeeded…
hrayatnia Feb 24, 2025
2a796ec
update the documentation
hrayatnia Feb 24, 2025
16a997e
fix typo in the method name `_includeNumberOfTestCasesIfNeeded`
hrayatnia Feb 24, 2025
ea309da
Refactor code to use existing methods and align with document style f…
hrayatnia Feb 24, 2025
4592e06
Update test case count increment to occur during the test run instead…
hrayatnia Feb 24, 2025
4cf3306
refactor: remove redundant helper methods and unnecessary test object…
hrayatnia Feb 24, 2025
46d4910
Cleanup: Remove unnecessary newline
hrayatnia Feb 24, 2025
701506c
nitpick: remove redundant whitespace in `numberOfTestsCasesAtTestEnd`
hrayatnia Feb 25, 2025
9616b98
Remove verbosity limit on `testCasesCount` and update related unit tests
hrayatnia Feb 25, 2025
8f61da7
add a minor arguments to `numberOfTestsCasesAtTestEnd` test
hrayatnia Feb 25, 2025
5c52756
Remove `testCasesCount` type definition
hrayatnia Feb 25, 2025
a5bac97
Refactor the test title for `numberOfTestCasesAtTestEnd` test for bet…
hrayatnia Feb 25, 2025
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
29 changes: 8 additions & 21 deletions Sources/Testing/Events/Recorder/Event.HumanReadableOutputRecorder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,31 +147,18 @@ extension Event.HumanReadableOutputRecorder {
return (errorIssueCount, warningIssueCount, knownIssueCount, totalIssueCount, description)
}

/// Calculates the total number of arguments across all test cases in a sequence.
///
/// - Parameters:
/// - testCases: A sequence of `Test.Case` objects containing the test cases..
///
/// - Returns: The total count of arguments across all test cases. If `testCases` is `nil` or empty, returns `0`.
///
private func _numberOfArguments(for testCases: (any Sequence<Test.Case>)?) -> Int {
testCases?.reduce(into: 0) { result, testCase in
result += testCase.arguments.count
} ?? 0
}

/// Returns a formatted string describing the number of arguments in a test, based on verbosity level.
///
/// - Parameters:
/// - test: The `Test` object that contains a sequence of `testCases`.
/// - 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 arguments in the test cases, or an empty string if it's not very verbose level.
/// - Returns: A string describing the number of argument(s) in the test cases, or an empty string if it's not very verbose level.
Copy link
Contributor

Choose a reason for hiding this comment

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

Documentation comments (but not code) should wrap at 80 columns. This helps give readers a sense of how long the documentation will be when rendered with DocC.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied, sorry for not double-checking it.

Copy link
Contributor

Choose a reason for hiding this comment

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

For more details see the Style Guide.

///
private func _includeNumberOfArguments(_ test: Test, verbose: Int) -> String {
if verbose == 2 { // very verbose
let numberOfArguments = _numberOfArguments(for: test.testCases)
return " with \(numberOfArguments) " + (numberOfArguments > 1 ? "arguments" : "argument")
private func _includeNumberOfTestCases(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")
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a helper function for pluralizing English nouns (used elsewhere in this file.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, applied. 🙏

}
return ""
}
Expand Down Expand Up @@ -400,14 +387,14 @@ extension Event.HumanReadableOutputRecorder {
CollectionOfOne(
Message(
symbol: .fail,
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfArguments(test, verbose: verbosity)) failed after \(duration)\(issues.description)."
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfTestCases(for: test, verbose: verbosity)) failed after \(duration)\(issues.description)."
)
) + _formattedComments(for: test)
} else {
[
Message(
symbol: .pass(knownIssueCount: issues.knownIssueCount),
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfArguments(test, verbose: verbosity)) passed after \(duration)\(issues.description)."
stringValue: "\(_capitalizedTitle(for: test)) \(testName)\(_includeNumberOfTestCases(for: test, verbose: verbosity)) passed after \(duration)\(issues.description)."
)
]
}
Expand Down
17 changes: 13 additions & 4 deletions Tests/TestingTests/EventRecorderTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,11 @@ struct EventRecorderTests {
@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()", #".*"# , 0),
("g()", #".* with .+ argument.*"# , 2),
("PredictablyFailingTests", #".*"# , 1),
("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 {
Expand All @@ -206,8 +208,9 @@ struct EventRecorderTests {
}

let aurgmentRegex = try Regex(expectedPattern)

#expect(
(try? buffer
(try buffer
.split(whereSeparator: \.isNewline)
.compactMap(aurgmentRegex.wholeMatch(in:))
.first) != nil
Expand Down Expand Up @@ -572,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 @@ -602,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?