Ensure tests representing containing types of suites are synthesized if necessary (#407)

stmontgomery · web-flow · commit 00eb790272e6 · 2024-11-15T12:00:50.000-06:00
Ensure that when a suite is nested in one or more containing suite types, and those outer suites do not have an explicit `@Suite` attribute, we synthesize `Test` instances representing all containing suites. ### 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. Resolves rdar://127770218
diff --git a/Sources/Testing/Parameterization/TypeInfo.swift b/Sources/Testing/Parameterization/TypeInfo.swift
@@ -45,11 +45,33 @@ public struct TypeInfo: Sendable {
     return nil
   }
 
-  init(fullyQualifiedName: String, unqualifiedName: String, mangledName: String?) {
+  /// Initialize an instance of this type with the specified names.
+  ///
+  /// - Parameters:
+  ///   - fullyQualifiedComponents: The fully-qualified name components of the
+  ///     type.
+  ///   - unqualified: The unqualified name of the type.
+  ///   - mangled: The mangled name of the type, if available.
+  init(fullyQualifiedNameComponents: [String], unqualifiedName: String, mangledName: String? = nil) {
     _kind = .nameOnly(
-      fullyQualifiedComponents: fullyQualifiedName.split(separator: ".").map(String.init),
+      fullyQualifiedComponents: fullyQualifiedNameComponents,
       unqualified: unqualifiedName,
-      mangled: mangledName
+      mangled: mangledName,
+    )
+  }
+
+  /// Initialize an instance of this type with the specified names.
+  ///
+  /// - Parameters:
+  ///   - fullyQualifiedName: The fully-qualified name of the type, with its
+  ///     components separated by a period character (`"."`).
+  ///   - unqualified: The unqualified name of the type.
+  ///   - mangled: The mangled name of the type, if available.
+  init(fullyQualifiedName: String, unqualifiedName: String, mangledName: String?) {
+    self.init(
+      fullyQualifiedNameComponents: fullyQualifiedName.split(separator: ".").map(String.init),
+      unqualifiedName: unqualifiedName,
+      mangledName: mangledName
     )
   }
 
@@ -246,44 +268,6 @@ func isClass(_ subclass: AnyClass, subclassOf superclass: AnyClass) -> Bool {
   }
 }
 
-// MARK: - Containing types
-
-extension TypeInfo {
-  /// An instance of this type representing the type immediately containing the
-  /// described type.
-  ///
-  /// For instance, given the following declaration in the `Example` module:
-  ///
-  /// ```swift
-  /// struct A {
-  ///   struct B {}
-  /// }
-  /// ```
-  ///
-  /// The value of this property for the type `A.B` would describe `A`, while
-  /// the value for `A` would be `nil` because it has no enclosing type.
-  var containingTypeInfo: Self? {
-    let fqnComponents = fullyQualifiedNameComponents
-    if fqnComponents.count > 2 { // the module is not a type
-      let fqn = fqnComponents.dropLast().joined(separator: ".")
-#if false // currently non-functional
-      if let type = _typeByName(fqn) {
-        return Self(describing: type)
-      }
-#endif
-      let name = fqnComponents[fqnComponents.count - 2]
-      return Self(fullyQualifiedName: fqn, unqualifiedName: name, mangledName: nil)
-    }
-    return nil
-  }
-
-  /// A sequence of instances of this type representing the types that
-  /// recursively contain it, starting with the immediate parent (if any.)
-  var allContainingTypeInfo: some Sequence<Self> {
-    sequence(first: self, next: \.containingTypeInfo).dropFirst()
-  }
-}
-
 // MARK: - CustomStringConvertible, CustomDebugStringConvertible, CustomTestStringConvertible
 
 extension TypeInfo: CustomStringConvertible, CustomDebugStringConvertible {
diff --git a/Sources/Testing/Running/Runner.Plan.swift b/Sources/Testing/Running/Runner.Plan.swift
@@ -154,6 +154,52 @@ extension Runner.Plan {
     }
   }
 
+  /// Recursively synthesize test instances representing suites for all missing
+  /// values in the specified test graph.
+  ///
+  /// - Parameters:
+  ///   - graph: The graph in which suites should be synthesized.
+  ///   - nameComponents: The name components of the suite to synthesize, based
+  ///     on the key path from the root node of the test graph to `graph`.
+  private static func _recursivelySynthesizeSuites(in graph: inout Graph<String, Test?>, nameComponents: [String] = []) {
+    // The recursive function. This is a local function to simplify the initial
+    // call which does not need to pass the `sourceLocation:` inout argument.
+    func synthesizeSuites(in graph: inout Graph<String, Test?>, nameComponents: [String] = [], sourceLocation: inout SourceLocation?) {
+      for (key, var childGraph) in graph.children {
+        synthesizeSuites(in: &childGraph, nameComponents: nameComponents + [key], sourceLocation: &sourceLocation)
+        graph.children[key] = childGraph
+      }
+
+      if let test = graph.value {
+        sourceLocation = test.sourceLocation
+      } else if let unqualifiedName = nameComponents.last, let sourceLocation {
+        // Don't synthesize suites representing modules.
+        if nameComponents.count <= 1 {
+          return
+        }
+
+        // Don't synthesize suites for nodes in the graph which are the
+        // immediate ancestor of a test function. That level of the hierarchy is
+        // used to disambiguate test functions which have equal names but
+        // different source locations.
+        if let firstChildTest = graph.children.values.first?.value, !firstChildTest.isSuite {
+          return
+        }
+
+        let typeInfo = TypeInfo(fullyQualifiedNameComponents: nameComponents, unqualifiedName: unqualifiedName)
+
+        // Note: When a suite is synthesized, it does not have an accurate
+        // source location, so we use the source location of a close descendant
+        // test. We do this instead of falling back to some "unknown"
+        // placeholder in an attempt to preserve the correct sort ordering.
+        graph.value = Test(traits: [], sourceLocation: sourceLocation, containingTypeInfo: typeInfo, isSynthesized: true)
+      }
+    }
+
+    var sourceLocation: SourceLocation?
+    synthesizeSuites(in: &graph, sourceLocation: &sourceLocation)
+  }
+
   /// Construct a graph of runner plan steps for the specified tests.
   ///
   /// - Parameters:
@@ -198,6 +244,9 @@ extension Runner.Plan {
       // and that is already guarded earlier in the SwiftPM entry point.
     }
 
+    // Synthesize suites for nodes in the test graph for which they are missing.
+    _recursivelySynthesizeSuites(in: &testGraph)
+
     // Recursively apply all recursive suite traits to children.
     //
     // This must be done _before_ calling `prepare(for:)` on the traits below.
diff --git a/Sources/Testing/Test+Discovery.swift b/Sources/Testing/Test+Discovery.swift
@@ -28,24 +28,7 @@ extension Test {
   /// All available ``Test`` instances in the process, according to the runtime.
   ///
   /// The order of values in this sequence is unspecified.
-  static var all: some Sequence<Test> {
-    get async {
-      // Convert the raw sequence of tests to a dictionary keyed by ID.
-      var result = await testsByID(_all)
-
-      // Ensure test suite types that don't have the @Suite attribute are still
-      // represented in the result.
-      _synthesizeSuiteTypes(into: &result)
-
-      return result.values
-    }
-  }
-
-  /// All available ``Test`` instances in the process, according to the runtime.
-  ///
-  /// The order of values in this sequence is unspecified. This sequence may
-  /// contain duplicates; callers should use ``all`` instead.
-  private static var _all: some Sequence<Self> {
+  static var all: some Sequence<Self> {
     get async {
       await withTaskGroup(of: [Self].self) { taskGroup in
         enumerateTypes(withNamesContaining: _testContainerTypeNameMagic) { _, type, _ in
@@ -60,53 +43,6 @@ extension Test {
       }
     }
   }
-
-  /// Create a dictionary mapping the IDs of a sequence of tests to those tests.
-  ///
-  /// - Parameters:
-  ///   - tests: The sequence to convert to a dictionary.
-  ///
-  /// - Returns: A dictionary containing `tests` keyed by those tests' IDs.
-  static func testsByID(_ tests: some Sequence<Self>) -> [ID: Self] {
-    [ID: Self](
-      tests.lazy.map { ($0.id, $0) },
-      uniquingKeysWith: { existing, _ in existing }
-    )
-  }
-
-  /// Synthesize any missing test suite types (that is, types containing test
-  /// content that do not have the `@Suite` attribute) and add them to a
-  /// dictionary of tests.
-  ///
-  /// - Parameters:
-  ///   - tests: A dictionary of tests to amend.
-  ///
-  /// - Returns: The number of key-value pairs added to `tests`.
-  @discardableResult private static func _synthesizeSuiteTypes(into tests: inout [ID: Self]) -> Int {
-    let originalCount = tests.count
-
-    // Find any instances of Test in the input that are *not* suites. We'll be
-    // checking the containing types of each one.
-    for test in tests.values where !test.isSuite {
-      guard let suiteTypeInfo = test.containingTypeInfo else {
-        continue
-      }
-      let suiteID = ID(typeInfo: suiteTypeInfo)
-      if tests[suiteID] == nil {
-        tests[suiteID] = Test(traits: [], sourceLocation: test.sourceLocation, containingTypeInfo: suiteTypeInfo, isSynthesized: true)
-
-        // Also synthesize any ancestral suites that don't have tests.
-        for ancestralSuiteTypeInfo in suiteTypeInfo.allContainingTypeInfo {
-          let ancestralSuiteID = ID(typeInfo: ancestralSuiteTypeInfo)
-          if tests[ancestralSuiteID] == nil {
-            tests[ancestralSuiteID] = Test(traits: [], sourceLocation: test.sourceLocation, containingTypeInfo: ancestralSuiteTypeInfo, isSynthesized: true)
-          }
-        }
-      }
-    }
-
-    return tests.count - originalCount
-  }
 }
 
 // MARK: -
diff --git a/Tests/TestingTests/MiscellaneousTests.swift b/Tests/TestingTests/MiscellaneousTests.swift
@@ -552,17 +552,6 @@ struct MiscellaneousTests {
     #expect(id.keyPathRepresentation == [""])
   }
 
-  @Test("Test.all deduping")
-  func allTestDeduping() {
-    let tests = [Test(name: "A") {}, Test(name: "B") {}, Test(name: "C") {}, Test(name: "D") {}, Test(name: "E") {}, Test(name: "F") {}, Test(name: "G") {},]
-    var duplicatedTests = tests
-    duplicatedTests += tests
-    duplicatedTests.shuffle()
-    let mappedTests = Test.testsByID(duplicatedTests)
-    #expect(mappedTests.count == tests.count)
-    #expect(mappedTests.values.allSatisfy { tests.contains($0) })
-  }
-
   @Test("failureBreakpoint() call")
   func failureBreakpointCall() {
     failureBreakpointValue = 1
diff --git a/Tests/TestingTests/PlanTests.swift b/Tests/TestingTests/PlanTests.swift
@@ -429,6 +429,104 @@ struct PlanTests {
     #expect(moduleGraph.children.count == 1)
   }
 
+  @Suite("Containing suite types without @Suite are synthesized")
+  struct ContainingSuiteSynthesis {
+    @Test("A test function inside a top-level implicit suite")
+    func oneImplicitParent() async throws {
+      let plan = await Runner.Plan(selecting: ImplicitParentSuite_A.self)
+      let testNames = plan.stepGraph.compactMap { $0.value }.map(\.test.name)
+      #expect(testNames == [
+        "ImplicitParentSuite_A",
+        "example()",
+      ])
+
+      let testFunction = try #require(plan.steps.map(\.test).first { $0.name == "example()" })
+      let implicitParentSuite_A = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitParentSuite_A" })
+      #expect(implicitParentSuite_A.sourceLocation == testFunction.sourceLocation)
+      #expect(implicitParentSuite_A.isSynthesized)
+    }
+
+    @Test("A test function in a type hierarchy where the nearest suite is explicit and outer ones are implicit", arguments: [
+      ImplicitGrandparentSuite_B.self,
+      ImplicitGrandparentSuite_B.ImplicitParentSuite_B.self,
+      ImplicitGrandparentSuite_B.ImplicitParentSuite_B.ExplicitChildSuite_B.self,
+    ] as [Any.Type])
+    func twoImplicitAncestorsButExplicitParent(suiteType: Any.Type) async throws {
+      let plan = await Runner.Plan(selecting: suiteType)
+      let testNames = plan.stepGraph.compactMap { $0.value }.map(\.test.name)
+      #expect(testNames == [
+        "ImplicitGrandparentSuite_B",
+        "ImplicitParentSuite_B",
+        "ExplicitChildSuite_B",
+        "example()",
+      ])
+
+      let testFunction = try #require(plan.steps.map(\.test).first { $0.name == "example()" })
+      let explicitChildSuite_B = try #require(plan.steps.map(\.test).first { $0.name == "ExplicitChildSuite_B" })
+      #expect(explicitChildSuite_B.sourceLocation != testFunction.sourceLocation)
+      #expect(!explicitChildSuite_B.isSynthesized)
+
+      let implicitParentSuite_B = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitParentSuite_B" })
+      #expect(implicitParentSuite_B.sourceLocation == explicitChildSuite_B.sourceLocation)
+      #expect(implicitParentSuite_B.isSynthesized)
+
+      let implicitGrandparentSuite_B = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitGrandparentSuite_B" })
+      #expect(implicitGrandparentSuite_B.sourceLocation == implicitParentSuite_B.sourceLocation)
+      #expect(implicitGrandparentSuite_B.isSynthesized)
+    }
+
+    @Test("A test function in a type hierarchy with both explicit and implicit suites")
+    func mixedAncestors() async throws {
+      let plan = await Runner.Plan(selecting: ExplicitGrandparentSuite_C.self)
+      let testNames = plan.stepGraph.compactMap { $0.value }.map(\.test.name)
+      #expect(testNames == [
+        "ExplicitGrandparentSuite_C",
+        "ImplicitParentSuite_C",
+        "ExplicitChildSuite_C",
+        "example()",
+      ])
+
+      let testFunction = try #require(plan.steps.map(\.test).first { $0.name == "example()" })
+      let explicitChildSuite_C = try #require(plan.steps.map(\.test).first { $0.name == "ExplicitChildSuite_C" })
+      #expect(explicitChildSuite_C.sourceLocation != testFunction.sourceLocation)
+      #expect(!explicitChildSuite_C.isSynthesized)
+
+      let implicitParentSuite_C = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitParentSuite_C" })
+      #expect(implicitParentSuite_C.sourceLocation == explicitChildSuite_C.sourceLocation)
+      #expect(implicitParentSuite_C.isSynthesized)
+
+      let explicitGrandparentSuite_C = try #require(plan.steps.map(\.test).first { $0.name == "ExplicitGrandparentSuite_C" })
+      #expect(explicitGrandparentSuite_C.sourceLocation != implicitParentSuite_C.sourceLocation)
+      #expect(!explicitGrandparentSuite_C.isSynthesized)
+    }
+
+    @Test("A test function in a type hierarchy with all implicit suites")
+    func allImplicitAncestors() async throws {
+      let plan = await Runner.Plan(selecting: ImplicitGrandparentSuite_D.self)
+      let testNames = plan.stepGraph.compactMap { $0.value }.map(\.test.name)
+      #expect(testNames == [
+        "ImplicitGrandparentSuite_D",
+        "ImplicitParentSuite_D",
+        "ImplicitChildSuite_D",
+        "ImplicitGrandchildSuite_D",
+        "example()",
+      ])
+
+      let testFunction = try #require(plan.steps.map(\.test).first { $0.name == "example()" })
+      let implicitGrandchildSuite_D = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitGrandchildSuite_D" })
+      #expect(implicitGrandchildSuite_D.sourceLocation == testFunction.sourceLocation)
+
+      let implicitChildSuite_D = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitChildSuite_D" })
+      #expect(implicitChildSuite_D.sourceLocation == implicitGrandchildSuite_D.sourceLocation)
+
+      let implicitParentSuite_D = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitParentSuite_D" })
+      #expect(implicitParentSuite_D.sourceLocation == implicitChildSuite_D.sourceLocation)
+
+      let implicitGrandparentSuite_D = try #require(plan.steps.map(\.test).first { $0.name == "ImplicitGrandparentSuite_D" })
+      #expect(implicitGrandparentSuite_D.sourceLocation == implicitParentSuite_D.sourceLocation)
+    }
+  }
+
 #if !SWT_NO_SNAPSHOT_TYPES
   @Test("Test cases of a disabled test are not evaluated")
   func disabledTestCases() async throws {
@@ -486,3 +584,42 @@ private struct BasicRecursiveTrait: SuiteTrait, TestTrait, CustomStringConvertib
     self.description = description
   }
 }
+
+// This fixture must not have an explicit `@Suite` attribute to validate suite
+// synthesis. Its children can be `.hidden`, though.
+fileprivate struct ImplicitParentSuite_A {
+  @Test(.hidden) func example() {}
+}
+
+fileprivate struct ImplicitGrandparentSuite_B {
+  // This fixture must not have an explicit `@Suite` attribute to validate suite
+  // synthesis. Its children can be `.hidden`, though.
+  struct ImplicitParentSuite_B {
+    @Suite(.hidden) struct ExplicitChildSuite_B {
+      @Test func example() {}
+    }
+  }
+}
+
+@Suite(.hidden) // This one intentionally _does_ have `@Suite`.
+fileprivate struct ExplicitGrandparentSuite_C {
+  // This fixture must not have an explicit `@Suite` attribute to validate suite
+  // synthesis. Its children can be `.hidden`, though.
+  struct ImplicitParentSuite_C {
+    @Suite struct ExplicitChildSuite_C {
+      @Test func example() {}
+    }
+  }
+}
+
+// These fixture suites must not have explicit `@Suite` attributes to validate
+// suite synthesis.
+fileprivate struct ImplicitGrandparentSuite_D {
+  struct ImplicitParentSuite_D {
+    struct ImplicitChildSuite_D {
+      struct ImplicitGrandchildSuite_D {
+        @Test(.hidden) func example() {}
+      }
+    }
+  }
+}
diff --git a/Tests/TestingTests/RunnerTests.swift b/Tests/TestingTests/RunnerTests.swift
