Skip to content

Remove SPIAwareTrait and adopt TestScoping in its place #901

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 7 commits into from
Jan 9, 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
1 change: 0 additions & 1 deletion Sources/Testing/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ add_library(Testing
Traits/ConditionTrait+Macro.swift
Traits/HiddenTrait.swift
Traits/ParallelizationTrait.swift
Traits/SPIAwareTrait.swift
Traits/Tags/Tag.Color.swift
Traits/Tags/Tag.Color+Loading.swift
Traits/Tags/Tag.List.swift
Expand Down
29 changes: 9 additions & 20 deletions Sources/Testing/Running/Runner.Plan.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,13 @@ extension Runner {
/// ## See Also
///
/// - ``ParallelizationTrait``
public var isParallelizationEnabled: Bool
@available(*, deprecated, message: "The 'isParallelizationEnabled' property is deprecated and no longer used. Its value is always false.")
public var isParallelizationEnabled: Bool {
get {
false
}
set {}
}
}

/// The test should be run.
Expand Down Expand Up @@ -65,19 +71,6 @@ extension Runner {
return true
}
}

/// Whether or not this action enables parallelization.
///
/// If this action is of case ``run(options:)``, the value of this
/// property equals the value of its associated
/// ``RunOptions/isParallelizationEnabled`` property. Otherwise, the value
/// of this property is `nil`.
var isParallelizationEnabled: Bool? {
if case let .run(options) = self {
return options.isParallelizationEnabled
}
return nil
}
}

/// A type describing a step in a runner plan.
Expand Down Expand Up @@ -218,7 +211,7 @@ extension Runner.Plan {
// Convert the list of test into a graph of steps. The actions for these
// steps will all be .run() *unless* an error was thrown while examining
// them, in which case it will be .recordIssue().
let runAction = Action.run(options: .init(isParallelizationEnabled: configuration.isParallelizationEnabled))
let runAction = Action.run(options: .init())
var testGraph = Graph<String, Test?>()
var actionGraph = Graph<String, Action>(value: runAction)
for test in tests {
Expand Down Expand Up @@ -278,11 +271,7 @@ extension Runner.Plan {
// `SkipInfo`, the error should not be recorded.
for trait in test.traits {
do {
if let trait = trait as? any SPIAwareTrait {
try await trait.prepare(for: test, action: &action)
} else {
try await trait.prepare(for: test)
}
try await trait.prepare(for: test)
} catch let error as SkipInfo {
action = .skip(error)
break
Expand Down
81 changes: 33 additions & 48 deletions Sources/Testing/Running/Runner.swift
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ public struct Runner: Sendable {
// MARK: - Running tests

extension Runner {
/// The current configuration _while_ running.
///
/// This should be used from the functions in this extension which access the
/// current configuration. This is important since individual tests or suites
/// may have traits which customize the execution scope of their children,
/// including potentially modifying the current configuration.
private static var _configuration: Configuration {
.current ?? .init()
}

/// Apply the custom scope for any test scope providers of the traits
/// associated with a specified test by calling their
/// ``TestScoping/provideScope(for:testCase:performing:)`` function.
Expand All @@ -70,7 +80,7 @@ extension Runner {
///
/// - Throws: Whatever is thrown by `body` or by any of the
/// ``TestScoping/provideScope(for:testCase:performing:)`` function calls.
private func _applyScopingTraits(
private static func _applyScopingTraits(
for test: Test,
testCase: Test.Case?,
_ body: @escaping @Sendable () async throws -> Void
Expand Down Expand Up @@ -103,21 +113,13 @@ extension Runner {
///
/// - Parameters:
/// - sequence: The sequence to enumerate.
/// - step: The plan step that controls parallelization. If `nil`, or if its
/// ``Runner/Plan/Step/action`` property is not of case
/// ``Runner/Plan/Action/run(options:)``, the
/// ``Configuration/isParallelizationEnabled`` property of this runner's
/// ``configuration`` property is used instead to determine if
/// parallelization is enabled.
/// - body: The function to invoke.
///
/// - Throws: Whatever is thrown by `body`.
private func _forEach<E>(
private static func _forEach<E>(
in sequence: some Sequence<E>,
for step: Plan.Step?,
_ body: @Sendable @escaping (E) async throws -> Void
) async throws where E: Sendable {
let isParallelizationEnabled = step?.action.isParallelizationEnabled ?? configuration.isParallelizationEnabled
try await withThrowingTaskGroup(of: Void.self) { taskGroup in
for element in sequence {
// Each element gets its own subtask to run in.
Expand All @@ -126,7 +128,7 @@ extension Runner {
}

// If not parallelizing, wait after each task.
if !isParallelizationEnabled {
if !_configuration.isParallelizationEnabled {
try await taskGroup.waitForAll()
}
}
Expand All @@ -137,12 +139,6 @@ extension Runner {
///
/// - Parameters:
/// - stepGraph: The subgraph whose root value, a step, is to be run.
/// - depth: How deep into the step graph this call is. The first call has a
/// depth of `0`.
/// - lastAncestorStep: The last-known ancestral step, if any, of the step
/// at the root of `stepGraph`. The options in this step (if its action is
/// of case ``Runner/Plan/Action/run(options:)``) inform the execution of
/// `stepGraph`.
///
/// - Throws: Whatever is thrown from the test body. Thrown errors are
/// normally reported as test failures.
Expand All @@ -157,7 +153,7 @@ extension Runner {
/// ## See Also
///
/// - ``Runner/run()``
private func _runStep(atRootOf stepGraph: Graph<String, Plan.Step?>, depth: Int, lastAncestorStep: Plan.Step?) async throws {
private static func _runStep(atRootOf stepGraph: Graph<String, Plan.Step?>) async throws {
// Exit early if the task has already been cancelled.
try Task.checkCancellation()

Expand All @@ -166,6 +162,8 @@ extension Runner {
// example, a skip event only sends `.testSkipped`.
let shouldSendTestEnded: Bool

let configuration = _configuration

// Determine what action to take for this step.
if let step = stepGraph.value {
Event.post(.planStepStarted(step), for: (step.test, nil), configuration: configuration)
Expand Down Expand Up @@ -204,14 +202,14 @@ extension Runner {
}

// Run the children of this test (i.e. the tests in this suite.)
try await _runChildren(of: stepGraph, depth: depth, lastAncestorStep: lastAncestorStep)
try await _runChildren(of: stepGraph)
}
}
}
} else {
// There is no test at this node in the graph, so just skip down to the
// child nodes.
try await _runChildren(of: stepGraph, depth: depth, lastAncestorStep: lastAncestorStep)
try await _runChildren(of: stepGraph)
}
}

Expand All @@ -222,7 +220,7 @@ extension Runner {
///
/// - Returns: The source location of the root node of `stepGraph`, or of the
/// first descendant node thereof (sorted by source location.)
private func _sourceLocation(of stepGraph: Graph<String, Plan.Step?>) -> SourceLocation? {
private static func _sourceLocation(of stepGraph: Graph<String, Plan.Step?>) -> SourceLocation? {
if let result = stepGraph.value?.test.sourceLocation {
return result
}
Expand All @@ -234,26 +232,13 @@ extension Runner {
/// Recursively run the tests that are children of a given plan step.
///
/// - Parameters:
/// - stepGraph: The subgraph whose root value, a step, is to be run.
/// - depth: How deep into the step graph this call is. The first call has a
/// depth of `0`.
/// - lastAncestorStep: The last-known ancestral step, if any, of the step
/// at the root of `stepGraph`. The options in this step (if its action is
/// of case ``Runner/Plan/Action/run(options:)``) inform the execution of
/// `stepGraph`.
/// - stepGraph: The subgraph whose root value, a step, will be used to
/// find children to run.
///
/// - Throws: Whatever is thrown from the test body. Thrown errors are
/// normally reported as test failures.
private func _runChildren(of stepGraph: Graph<String, Plan.Step?>, depth: Int, lastAncestorStep: Plan.Step?) async throws {
// Figure out the last-good step, either the one at the root of `stepGraph`
// or, if it is nil, the one passed into this function. We need to track
// this value in case we run into sparse sections of the graph so we don't
// lose track of the recursive `isParallelizationEnabled` property in the
// runnable steps' options.
let stepOrAncestor = stepGraph.value ?? lastAncestorStep

let isParallelizationEnabled = stepOrAncestor?.action.isParallelizationEnabled ?? configuration.isParallelizationEnabled
let childGraphs = if isParallelizationEnabled {
private static func _runChildren(of stepGraph: Graph<String, Plan.Step?>) async throws {
let childGraphs = if _configuration.isParallelizationEnabled {
// Explicitly shuffle the steps to help detect accidental dependencies
// between tests due to their ordering.
Array(stepGraph.children)
Expand Down Expand Up @@ -282,8 +267,8 @@ extension Runner {
}

// Run the child nodes.
try await _forEach(in: childGraphs, for: stepOrAncestor) { _, childGraph in
try await _runStep(atRootOf: childGraph, depth: depth + 1, lastAncestorStep: stepOrAncestor)
try await _forEach(in: childGraphs) { _, childGraph in
try await _runStep(atRootOf: childGraph)
}
}

Expand All @@ -298,13 +283,14 @@ extension Runner {
///
/// If parallelization is supported and enabled, the generated test cases will
/// be run in parallel using a task group.
private func _runTestCases(_ testCases: some Sequence<Test.Case>, within step: Plan.Step) async throws {
private static func _runTestCases(_ testCases: some Sequence<Test.Case>, within step: Plan.Step) async throws {
// Apply the configuration's test case filter.
let testCaseFilter = _configuration.testCaseFilter
let testCases = testCases.lazy.filter { testCase in
configuration.testCaseFilter(testCase, step.test)
testCaseFilter(testCase, step.test)
}

try await _forEach(in: testCases, for: step) { testCase in
try await _forEach(in: testCases) { testCase in
try await _runTestCase(testCase, within: step)
}
}
Expand All @@ -320,10 +306,12 @@ extension Runner {
///
/// This function sets ``Test/Case/current``, then invokes the test case's
/// body closure.
private func _runTestCase(_ testCase: Test.Case, within step: Plan.Step) async throws {
private static func _runTestCase(_ testCase: Test.Case, within step: Plan.Step) async throws {
// Exit early if the task has already been cancelled.
try Task.checkCancellation()

let configuration = _configuration

Event.post(.testCaseStarted, for: (step.test, testCase), configuration: configuration)
defer {
Event.post(.testCaseEnded, for: (step.test, testCase), configuration: configuration)
Expand Down Expand Up @@ -357,9 +345,6 @@ extension Runner {
///
/// - Parameters:
/// - runner: The runner to run.
/// - configuration: The configuration to use for running. The value of this
/// argument temporarily replaces the value of `runner`'s
/// ``Runner/configuration`` property.
///
/// This function is `static` so that it cannot accidentally reference `self`
/// or `self.configuration` when it should use a modified copy of either.
Expand Down Expand Up @@ -399,7 +384,7 @@ extension Runner {

await withTaskGroup(of: Void.self) { [runner] taskGroup in
_ = taskGroup.addTaskUnlessCancelled {
try? await runner._runStep(atRootOf: runner.plan.stepGraph, depth: 0, lastAncestorStep: nil)
try? await _runStep(atRootOf: runner.plan.stepGraph)
}
await taskGroup.waitForAll()
}
Expand Down
34 changes: 15 additions & 19 deletions Sources/Testing/Traits/ParallelizationTrait.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,32 @@
/// A type that affects whether or not a test or suite is parallelized.
///
/// When added to a parameterized test function, this trait causes that test to
/// run its cases serially instead of in parallel. When applied to a
/// non-parameterized test function, this trait has no effect. When applied to a
/// test suite, this trait causes that suite to run its contained test functions
/// and sub-suites serially instead of in parallel.
/// run its cases serially instead of in parallel. When added to a
/// non-parameterized test function, this trait has no effect.
///
/// This trait is recursively applied: if it is applied to a suite, any
/// parameterized tests or test suites contained in that suite are also
/// serialized (as are any tests contained in those suites, and so on.)
/// When added to a test suite, this trait causes that suite to run its
/// contained test functions (including their cases, when parameterized) and
/// sub-suites serially instead of in parallel. Any children of sub-suites are
/// also run serially.
///
/// This trait does not affect the execution of a test relative to its peers or
/// to unrelated tests. This trait has no effect if test parallelization is
/// globally disabled (by, for example, passing `--no-parallel` to the
/// `swift test` command.)
///
/// To add this trait to a test, use ``Trait/serialized``.
public struct ParallelizationTrait: TestTrait, SuiteTrait {
public var isRecursive: Bool {
true
}
}
public struct ParallelizationTrait: TestTrait, SuiteTrait {}

// MARK: - SPIAwareTrait
// MARK: - TestScoping

@_spi(ForToolsIntegrationOnly)
extension ParallelizationTrait: SPIAwareTrait {
public func prepare(for test: Test, action: inout Runner.Plan.Action) async throws {
if case var .run(options) = action {
options.isParallelizationEnabled = false
action = .run(options: options)
extension ParallelizationTrait: TestScoping {
public func provideScope(for test: Test, testCase: Test.Case?, performing function: @Sendable () async throws -> Void) async throws {
guard var configuration = Configuration.current else {
throw SystemError(description: "There is no current Configuration when attempting to provide scope for test '\(test.name)'. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
}

configuration.isParallelizationEnabled = false
try await Configuration.withCurrent(configuration, perform: function)
}
}

Expand Down
38 changes: 0 additions & 38 deletions Sources/Testing/Traits/SPIAwareTrait.swift

This file was deleted.

10 changes: 0 additions & 10 deletions Tests/TestingTests/Traits/ParallelizationTraitTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@

@Suite("Parallelization Trait Tests", .tags(.traitRelated))
struct ParallelizationTraitTests {
@Test(".serialized trait is recursively applied")
func serializedTrait() async {
var configuration = Configuration()
configuration.isParallelizationEnabled = true
let plan = await Runner.Plan(selecting: OuterSuite.self, configuration: configuration)
for step in plan.steps {
#expect(step.action.isParallelizationEnabled == false, "Step \(step) should have had parallelization disabled")
}
}

@Test(".serialized trait serializes parameterized test")
func serializesParameterizedTestFunction() async {
var configuration = Configuration()
Expand Down