Remove SPIAwareTrait and adopt TestScoping in its place (#901)

This is a follow-on from #733 which introduced the Test Scoping Traits
feature. As that proposal indicated, the `SPIAwareTrait` SPI protocol is
no longer necessary, so this PR removes it and adopts `TestScoping` in
its place.

### Motivation:

Adopting Test Scoping Traits for the purpose which `SPIAwareTrait`
previously served simplifies a lot of runner and planning logic.

### Modifications:

- Delete `SPIAwareTrait` and related SPI.
- Adopt `TestScoping` in `ParallelizationTrait` instead.
- Changed `ParallelizationTrait` to _not_ always have the value `true`
for `isRecursive`. Test Scoping Traits makes this no longer necessary:
the invocation of children of any suites which have `.serialized` is
scoped to have parallelization disabled.
- Removed tracking of `isParallelizationEnabled` on the
`Runner.Plan.Action.RunOptions` type. For now, I kept the `RunOptions`
struct, since it could be useful in the future and it's SPI.
- Removed obsolete logic in `Runner` for propagating ancestor steps, and
refactored slightly to make many things `static`. This is to help ensure
certain functions don't access the `self.configuration` property when
instead they should be accessing `Configuration.current`.

### Result:

No functional change, but implementation is simplified.

### 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.

Author: stmontgomery

Sources/Testing/CMakeLists.txt

@@ -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

Sources/Testing/Running/Runner.Plan.swift

@@ -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.
@@ -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.
@@ -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 {
@@ -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

Sources/Testing/Running/Runner.swift

@@ -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.
@@ -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
@@ -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.
@@ -126,7 +128,7 @@ extension Runner {
         }
 
         // If not parallelizing, wait after each task.
-        if !isParallelizationEnabled {
+        if !_configuration.isParallelizationEnabled {
           try await taskGroup.waitForAll()
         }
       }
@@ -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.
@@ -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()
 
@@ -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)
@@ -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)
     }
   }
 
@@ -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
     }
@@ -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)
@@ -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)
     }
   }
 
@@ -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)
     }
   }
@@ -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)
@@ -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.
@@ -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()
         }

Sources/Testing/Traits/ParallelizationTrait.swift

@@ -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)
   }
 }
 

Sources/Testing/Traits/SPIAwareTrait.swift

@@ -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()