Skip to content

Commit aff817e

Browse files
authored
Ensure serially-scheduled tests run in source order. (#569)
This PR ensures that tests, when scheduled serially, run in declared source order (or as close to it as suite layout allows.) When tests are run in parallel, we don't bother to sort them because they will run non-deterministically anyway. Although we don't explicitly shuffle them, they are stored in a `Dictionary` and in Swift, hashcodes are salted with a random value, so their iteration order will change on every test run. (In other words, they're already effectively shuffled.) Resolves #566. Resolves rdar://132285155. ### 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.
1 parent 0e93abd commit aff817e

File tree

2 files changed

+82
-2
lines changed

2 files changed

+82
-2
lines changed

Sources/Testing/Running/Runner.swift

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,22 @@ extension Runner {
222222
}
223223
}
224224

225+
/// Get the source location corresponding to the root of a plan step graph.
226+
///
227+
/// - Parameters:
228+
/// - stepGraph: The plan step graph whose root node is of interest.
229+
///
230+
/// - Returns: The source location of the root node of `stepGraph`, or of the
231+
/// first descendant node thereof (sorted by source location.)
232+
private func _sourceLocation(of stepGraph: Graph<String, Plan.Step?>) -> SourceLocation? {
233+
if let result = stepGraph.value?.test.sourceLocation {
234+
return result
235+
}
236+
return stepGraph.children.lazy
237+
.compactMap { _sourceLocation(of: $0.value) }
238+
.min()
239+
}
240+
225241
/// Recursively run the tests that are children of a given plan step.
226242
///
227243
/// - Parameters:
@@ -243,8 +259,36 @@ extension Runner {
243259
// runnable steps' options.
244260
let stepOrAncestor = stepGraph.value ?? lastAncestorStep
245261

246-
// Run the children of this step.
247-
let childGraphs = stepGraph.children.sorted { $0.key < $1.key }
262+
let isParallelizationEnabled = stepOrAncestor?.action.isParallelizationEnabled ?? configuration.isParallelizationEnabled
263+
let childGraphs = if isParallelizationEnabled {
264+
// Explicitly shuffle the steps to help detect accidental dependencies
265+
// between tests due to their ordering.
266+
Array(stepGraph.children)
267+
} else {
268+
// Sort the children by source order. If a child node is empty but has
269+
// descendants, the lowest-ordered child node is used. If a child node is
270+
// empty and has no descendant nodes with source location information,
271+
// then we sort it before nodes with source location information (though
272+
// it won't end up doing anything in the test run.)
273+
//
274+
// FIXME: this operation is likely O(n log n) or worse when amortized
275+
// across the entire test plan; Graph should adopt OrderedDictionary if
276+
// possible so it can pre-sort its nodes once.
277+
stepGraph.children.sorted { lhs, rhs in
278+
switch (_sourceLocation(of: lhs.value), _sourceLocation(of: rhs.value)) {
279+
case let (.some(lhs), .some(rhs)):
280+
lhs < rhs
281+
case (.some, _):
282+
false // x < nil == false
283+
case (_, .some):
284+
true // nil < x == true
285+
default:
286+
false // stable ordering
287+
}
288+
}
289+
}
290+
291+
// Run the child nodes.
248292
try await _forEach(in: childGraphs, for: stepOrAncestor) { _, childGraph in
249293
try await _runStep(atRootOf: childGraph, depth: depth + 1, lastAncestorStep: stepOrAncestor)
250294
}

Tests/TestingTests/RunnerTests.swift

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,5 +878,41 @@ final class RunnerTests: XCTestCase {
878878
await runTest(for: DeprecatedVersionTests.self, configuration: configuration)
879879
await fulfillment(of: [testStarted, testSkipped], timeout: 0.0)
880880
}
881+
882+
func testSerializedSortOrder() async {
883+
OrderedTests.state.withLock { state in
884+
state = 0
885+
}
886+
await runTest(for: OrderedTests.self, configuration: .init())
887+
}
888+
}
889+
890+
// MARK: - Fixtures
891+
892+
extension OrderedTests.Inner {
893+
@Test(.hidden) func s() { XCTAssertEqual(OrderedTests.state.increment(), 5) }
894+
}
895+
896+
@Suite(.hidden, .serialized) struct OrderedTests {
897+
static let state = Locked(rawValue: 0)
898+
899+
@Test(.hidden) func z() { XCTAssertEqual(Self.state.increment(), 1) }
900+
@Test(.hidden) func y() { XCTAssertEqual(Self.state.increment(), 2) }
901+
@Test(.hidden) func x() { XCTAssertEqual(Self.state.increment(), 3) }
902+
@Test(.hidden) func w() { XCTAssertEqual(Self.state.increment(), 4) }
903+
@Suite(.hidden) struct Inner {
904+
// s() in extension above, numbered 5
905+
@Test(.hidden) func t() { XCTAssertEqual(OrderedTests.state.increment(), 6) }
906+
// u() in extension below, numbered 7
907+
}
908+
909+
@Test(.hidden) func d() { XCTAssertEqual(Self.state.increment(), 8) }
910+
@Test(.hidden) func c() { XCTAssertEqual(Self.state.increment(), 9) }
911+
@Test(.hidden) func b() { XCTAssertEqual(Self.state.increment(), 10) }
912+
@Test(.hidden) func a() { XCTAssertEqual(Self.state.increment(), 11) }
913+
}
914+
915+
extension OrderedTests.Inner {
916+
@Test(.hidden) func u() { XCTAssertEqual(OrderedTests.state.increment(), 7) }
881917
}
882918
#endif

0 commit comments

Comments
 (0)