Skip to content

Commit 44e71d4

Browse files
committed
Fix parameterized test display and group test cases under parent test with proper indentation hierarchy
1 parent 3acb802 commit 44e71d4

File tree

1 file changed

+106
-58
lines changed

1 file changed

+106
-58
lines changed

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

Lines changed: 106 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,15 @@ extension Event {
1414
/// This type encapsulates the logic for collecting failed tests from a test
1515
/// data graph and formatting them into a human-readable failure summary.
1616
private struct TestRunSummary: Sendable {
17+
/// Information about a single failed test case (for parameterized tests).
18+
struct FailedTestCase: Sendable {
19+
/// The test case arguments for this parameterized test case.
20+
var arguments: String
21+
22+
/// All issues recorded for this test case.
23+
var issues: [HumanReadableOutputRecorder.Context.TestData.IssueInfo]
24+
}
25+
1726
/// Information about a single failed test.
1827
struct FailedTest: Sendable {
1928
/// The full hierarchical path to the test (e.g., suite names).
@@ -22,29 +31,14 @@ extension Event {
2231
/// The test's simple name (last component of the path).
2332
var name: String
2433

25-
/// All issues recorded for this test.
26-
var issues: [IssueInfo]
27-
2834
/// The test's display name, if any.
2935
var displayName: String?
3036

31-
/// The test case arguments for parameterized tests, if any.
32-
var testCaseArguments: String?
33-
}
34-
35-
/// Information about a single issue within a failed test.
36-
struct IssueInfo: Sendable {
37-
/// The source location where the issue occurred.
38-
var sourceLocation: SourceLocation?
37+
/// For non-parameterized tests: issues recorded directly on the test.
38+
var issues: [HumanReadableOutputRecorder.Context.TestData.IssueInfo]
3939

40-
/// A detailed description of what failed.
41-
var description: String
42-
43-
/// Whether this issue is a known issue.
44-
var isKnown: Bool
45-
46-
/// The severity of this issue.
47-
var severity: Issue.Severity
40+
/// For parameterized tests: test cases with their issues.
41+
var testCases: [FailedTestCase]
4842
}
4943

5044
/// The list of failed tests collected from the test run.
@@ -55,62 +49,95 @@ extension Event {
5549
/// - Parameters:
5650
/// - testData: The root test data graph to traverse.
5751
fileprivate init(from testData: Graph<HumanReadableOutputRecorder.Context.TestDataKey, HumanReadableOutputRecorder.Context.TestData?>) {
58-
var collected: [FailedTest] = []
52+
var testMap: [String: FailedTest] = [:]
5953

6054
// Traverse the graph to find all tests with failures
61-
func traverse(graph: Graph<HumanReadableOutputRecorder.Context.TestDataKey, HumanReadableOutputRecorder.Context.TestData?>, path: [String]) {
55+
func traverse(graph: Graph<HumanReadableOutputRecorder.Context.TestDataKey, HumanReadableOutputRecorder.Context.TestData?>, path: [String], isTestCase: Bool = false) {
6256
// Check if this node has test data with failures
6357
if let testData = graph.value, !testData.issues.isEmpty {
6458
let testName = path.last ?? "Unknown"
6559

66-
// Convert Context.TestData.IssueInfo to TestRunSummary.IssueInfo
67-
let issues = testData.issues.map { issue in
68-
IssueInfo(
69-
sourceLocation: issue.sourceLocation,
70-
description: issue.description,
71-
isKnown: issue.isKnown,
72-
severity: issue.severity
60+
// Use issues directly from testData
61+
let issues = testData.issues
62+
63+
if isTestCase {
64+
// This is a test case node - add it to the parent test's testCases array
65+
// The parent test path is the path without the test case ID component
66+
let parentPath = path.filter { !$0.hasPrefix("arguments:") }
67+
let parentPathKey = parentPath.joined(separator: "/")
68+
69+
if var parentTest = testMap[parentPathKey] {
70+
// Add this test case to the parent
71+
if let arguments = testData.testCaseArguments, !arguments.isEmpty {
72+
parentTest.testCases.append(FailedTestCase(
73+
arguments: arguments,
74+
issues: issues
75+
))
76+
testMap[parentPathKey] = parentTest
77+
}
78+
} else {
79+
// Parent test not found in map, but should exist - create it
80+
let parentTest = FailedTest(
81+
path: parentPath,
82+
name: parentPath.last ?? "Unknown",
83+
displayName: testData.displayName,
84+
issues: [],
85+
testCases: (testData.testCaseArguments?.isEmpty ?? true) ? [] : [FailedTestCase(
86+
arguments: testData.testCaseArguments ?? "",
87+
issues: issues
88+
)]
89+
)
90+
testMap[parentPathKey] = parentTest
91+
}
92+
} else {
93+
// This is a test node (not a test case)
94+
let pathKey = path.joined(separator: "/")
95+
let failedTest = FailedTest(
96+
path: path,
97+
name: testName,
98+
displayName: testData.displayName,
99+
issues: issues,
100+
testCases: []
73101
)
102+
testMap[pathKey] = failedTest
74103
}
75-
76-
collected.append(FailedTest(
77-
path: path,
78-
name: testName,
79-
issues: issues,
80-
displayName: testData.displayName,
81-
testCaseArguments: testData.testCaseArguments
82-
))
83104
}
84105

85106
// Recursively traverse children
86107
for (key, childGraph) in graph.children {
87108
let pathComponent: String?
109+
let isChildTestCase: Bool
88110
switch key {
89111
case let .string(s):
90112
let parts = s.split(separator: ":")
91113
if s.hasSuffix(".swift:") || (parts.count >= 2 && parts[0].hasSuffix(".swift")) {
92114
pathComponent = nil // Filter out source location strings
115+
isChildTestCase = false
93116
} else {
94117
pathComponent = s
118+
isChildTestCase = false
95119
}
96120
case let .testCaseID(id):
97121
// Only include parameterized test case IDs in path
98122
if let argumentIDs = id.argumentIDs, let discriminator = id.discriminator {
99123
pathComponent = "arguments: \(argumentIDs), discriminator: \(discriminator)"
124+
isChildTestCase = true
100125
} else {
101126
pathComponent = nil // Filter out non-parameterized test case IDs
127+
isChildTestCase = false
102128
}
103129
}
104130

105131
let newPath = pathComponent.map { path + [$0] } ?? path
106-
traverse(graph: childGraph, path: newPath)
132+
traverse(graph: childGraph, path: newPath, isTestCase: isChildTestCase)
107133
}
108134
}
109135

110136
// Start traversal from root
111137
traverse(graph: testData, path: [])
112138

113-
self.failedTests = collected
139+
// Convert map to array, ensuring we only include tests that have failures
140+
self.failedTests = Array(testMap.values).filter { !$0.issues.isEmpty || !$0.testCases.isEmpty }
114141
}
115142

116143
/// Generate a formatted failure summary string.
@@ -145,7 +172,13 @@ extension Event {
145172
/// - Returns: A string containing the header line.
146173
private func header() -> String {
147174
let failedTestsPhrase = failedTests.count.counting("test")
148-
let totalIssuesCount = failedTests.reduce(0) { $0 + $1.issues.count }
175+
var totalIssuesCount = 0
176+
for test in failedTests {
177+
totalIssuesCount += test.issues.count
178+
for testCase in test.testCases {
179+
totalIssuesCount += testCase.issues.count
180+
}
181+
}
149182
let issuesPhrase = totalIssuesCount.counting("issue")
150183
return "Test run had \(failedTestsPhrase) which recorded \(issuesPhrase) total:\n"
151184
}
@@ -166,14 +199,21 @@ extension Event {
166199

167200
result += "\(symbol) \(fullyQualifiedName)\n"
168201

169-
// Show test case arguments for parameterized tests (once per test)
170-
if let arguments = failedTest.testCaseArguments, !arguments.isEmpty {
171-
result += " (\(arguments))\n"
172-
}
173-
174-
// List each issue for this test with indentation
175-
for issue in failedTest.issues {
176-
result += formatIssue(issue)
202+
// For parameterized tests: show test cases grouped under the parent test
203+
if !failedTest.testCases.isEmpty {
204+
for testCase in failedTest.testCases {
205+
// Show test case arguments with additional indentation
206+
result += " (\(testCase.arguments))\n"
207+
// List each issue for this test case with additional indentation
208+
for issue in testCase.issues {
209+
result += formatIssue(issue, indentLevel: 2)
210+
}
211+
}
212+
} else {
213+
// For non-parameterized tests: show issues directly
214+
for issue in failedTest.issues {
215+
result += formatIssue(issue)
216+
}
177217
}
178218

179219
return result
@@ -185,31 +225,39 @@ extension Event {
185225
/// - failedTest: The failed test.
186226
///
187227
/// - Returns: The fully qualified name, with display name substituted if
188-
/// available.
228+
/// available. Test case ID components are filtered out since they're
229+
/// shown separately.
189230
private func fullyQualifiedName(for failedTest: FailedTest) -> String {
190231
// Omit the leading path component representing the module name from the
191232
// fully-qualified name of the test.
192-
let path = failedTest.path.dropFirst()
233+
var path = Array(failedTest.path.dropFirst())
193234

194-
// Use display name for the last component if available. Otherwise, join
195-
// the path components.
196-
return if let displayName = failedTest.displayName, !failedTest.path.isEmpty {
197-
(path.dropLast() + [#""\#(displayName)""#]).joined(separator: "/")
198-
} else {
199-
path.joined(separator: "/")
235+
// Filter out test case ID components (they're shown separately with arguments)
236+
path = path.filter { !$0.hasPrefix("arguments:") }
237+
238+
// If we have a display name, replace the function name component (which is
239+
// now the last component after filtering) with the display name. This avoids
240+
// showing both the function name and display name.
241+
if let displayName = failedTest.displayName, !path.isEmpty {
242+
path[path.count - 1] = #""\#(displayName)""#
200243
}
244+
245+
return path.joined(separator: "/")
201246
}
202247

203248
/// Format a single issue entry.
204249
///
205250
/// - Parameters:
206251
/// - issue: The issue to format.
252+
/// - indentLevel: The number of indentation levels (each level is 2 spaces).
253+
/// Defaults to 1.
207254
///
208255
/// - Returns: A formatted string representing the issue with indentation.
209-
private func formatIssue(_ issue: IssueInfo) -> String {
210-
var result = " - \(issue.description)\n"
256+
private func formatIssue(_ issue: HumanReadableOutputRecorder.Context.TestData.IssueInfo, indentLevel: Int = 1) -> String {
257+
let indent = String(repeating: " ", count: indentLevel)
258+
var result = "\(indent)- \(issue.description)\n"
211259
if let location = issue.sourceLocation {
212-
result += " at \(location)\n"
260+
result += "\(indent) at \(location)\n"
213261
}
214262
return result
215263
}

0 commit comments

Comments
 (0)