Merge pull request #2057 from ahoppen/test-timeout

Add a few timeout checks to tests

Author: ahoppen

Sources/Diagnose/TraceFromSignpostsCommand.swift

@@ -15,8 +15,6 @@ import Foundation
 import RegexBuilder
 import SwiftExtensions
 
-import class TSCBasic.Process
-
 /// Shared instance of the regex that is used to extract Signpost lines from `log stream --signpost`.
 fileprivate struct LogParseRegex {
   @MainActor static let shared = LogParseRegex()

Sources/SKTestSupport/IndexedSingleSwiftFileTestProject.swift

@@ -13,6 +13,7 @@
 @_spi(Testing) import BuildSystemIntegration
 package import Foundation
 package import LanguageServerProtocol
+import SKLogging
 import SKOptions
 import SourceKitLSP
 import SwiftExtensions
@@ -126,7 +127,11 @@ package struct IndexedSingleSwiftFileTestProject {
 
     // Run swiftc to build the index store
     do {
-      try await Process.checkNonZeroExit(arguments: [swiftc.filePath] + compilerArguments)
+      let compilerArgumentsCopy = compilerArguments
+      let output = try await withTimeout(defaultTimeoutDuration) {
+        try await Process.checkNonZeroExit(arguments: [swiftc.filePath] + compilerArgumentsCopy)
+      }
+      logger.debug("swiftc output:\n\(output)")
     } catch {
       if !allowBuildFailure {
         throw error

Sources/SKTestSupport/MultiEntrySemaphore.swift

@@ -0,0 +1,52 @@
+//===----------------------------------------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2024 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SwiftExtensions
+import XCTest
+
+/// A semaphore that, once signaled, will pass on every `wait` call. Ie. the semaphore only needs to be signaled once
+/// and from that point onwards it can be acquired as many times as necessary.
+///
+/// Use cases of this are for example to delay indexing until a some other task has been performed. But once that is
+/// done, all index operations should be able to run, not just one.
+package final class MultiEntrySemaphore: Sendable {
+  private let name: String
+  private let signaled = AtomicBool(initialValue: false)
+
+  package init(name: String) {
+    self.name = name
+  }
+
+  package func signal() {
+    signaled.value = true
+  }
+
+  package func waitOrThrow() async throws {
+    do {
+      try await repeatUntilExpectedResult(sleepInterval: .seconds(0.01)) { signaled.value }
+    } catch {
+      struct TimeoutError: Error, CustomStringConvertible {
+        let name: String
+        var description: String { "\(name) timed out" }
+      }
+      throw TimeoutError(name: "\(name) timed out")
+    }
+  }
+
+  package func waitOrXCTFail(file: StaticString = #filePath, line: UInt = #line) async {
+    do {
+      try await waitOrThrow()
+    } catch {
+      XCTFail("\(error)", file: file, line: line)
+    }
+  }
+}

Sources/SKTestSupport/RepeatUntilExpectedResult.swift

@@ -21,7 +21,7 @@ import XCTest
 ///
 /// `sleepInterval` is the duration to wait before re-executing the body.
 package func repeatUntilExpectedResult(
-  timeout: Duration = .seconds(defaultTimeout),
+  timeout: Duration = defaultTimeoutDuration,
   sleepInterval: Duration = .seconds(1),
   _ body: () async throws -> Bool,
   file: StaticString = #filePath,

Sources/SKTestSupport/SkipUnless.swift

@@ -222,19 +222,22 @@ package actor SkipUnless {
           throw FailedToCrateInputFileError()
         }
         // If we can't compile for wasm, this fails complaining that it can't find the stdlib for wasm.
-        let process = Process(
-          args: try swiftFrontend.filePath,
-          "-typecheck",
-          try input.filePath,
-          "-triple",
-          "wasm32-unknown-none-wasm",
-          "-enable-experimental-feature",
-          "Embedded",
-          "-Xcc",
-          "-fdeclspec"
-        )
-        try process.launch()
-        let result = try await process.waitUntilExit()
+        let result = try await withTimeout(defaultTimeoutDuration) {
+          try await Process.run(
+            arguments: [
+              try swiftFrontend.filePath,
+              "-typecheck",
+              try input.filePath,
+              "-triple",
+              "wasm32-unknown-none-wasm",
+              "-enable-experimental-feature",
+              "Embedded",
+              "-Xcc",
+              "-fdeclspec",
+            ],
+            workingDirectory: nil
+          )
+        }
         if result.exitStatus == .terminated(code: 0) {
           return .featureSupported
         }
@@ -266,7 +269,7 @@ package actor SkipUnless {
               sourcekitd.keys.useNewAPI: 1
             ],
           ]),
-          timeout: .seconds(defaultTimeout),
+          timeout: defaultTimeoutDuration,
           fileContents: nil
         )
         return response[sourcekitd.keys.useNewAPI] == 1

Sources/SKTestSupport/SwiftPMDependencyProject.swift

@@ -13,6 +13,7 @@
 package import Foundation
 import LanguageServerProtocolExtensions
 import SwiftExtensions
+import TSCExtensions
 import XCTest
 
 import struct TSCBasic.AbsolutePath
@@ -45,11 +46,12 @@ package class SwiftPMDependencyProject {
     }
     // We can't use `workingDirectory` because Amazon Linux doesn't support working directories (or at least
     // TSCBasic.Process doesn't support working directories on Amazon Linux)
-    let process = TSCBasic.Process(
-      arguments: [try git.filePath, "-C", try workingDirectory.filePath] + arguments
-    )
-    try process.launch()
-    let processResult = try await process.waitUntilExit()
+    let processResult = try await withTimeout(defaultTimeoutDuration) {
+      try await TSCBasic.Process.run(
+        arguments: [try git.filePath, "-C", try workingDirectory.filePath] + arguments,
+        workingDirectory: nil
+      )
+    }
     guard processResult.exitStatus == .terminated(code: 0) else {
       throw Error.processedTerminatedWithNonZeroExitCode(processResult)
     }

Sources/SKTestSupport/SwiftPMTestProject.swift

@@ -12,6 +12,7 @@
 
 package import Foundation
 package import LanguageServerProtocol
+import SKLogging
 package import SKOptions
 package import SourceKitLSP
 import SwiftExtensions
@@ -260,7 +261,16 @@ package class SwiftPMTestProject: MultiFileTestProject {
         "-Xswiftc", "-module-cache-path", "-Xswiftc", try globalModuleCache.filePath,
       ]
     }
-    try await Process.checkNonZeroExit(arguments: arguments)
+    let argumentsCopy = arguments
+    let output = try await withTimeout(defaultTimeoutDuration) {
+      try await Process.checkNonZeroExit(arguments: argumentsCopy)
+    }
+    logger.debug(
+      """
+      'swift build' output: 
+      \(output)
+      """
+    )
   }
 
   /// Resolve package dependencies for the package at `path`.
@@ -274,6 +284,14 @@ package class SwiftPMTestProject: MultiFileTestProject {
       "resolve",
       "--package-path", try path.filePath,
     ]
-    try await Process.checkNonZeroExit(arguments: arguments)
+    let output = try await withTimeout(defaultTimeoutDuration) {
+      try await Process.checkNonZeroExit(arguments: arguments)
+    }
+    logger.debug(
+      """
+      'swift package resolve' output: 
+      \(output)
+      """
+    )
   }
 }

Sources/SKTestSupport/TestSourceKitLSPClient.swift

@@ -197,7 +197,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
     preInitialization?(self)
     if initialize {
       let capabilities = capabilities
-      try await withTimeout(.seconds(defaultTimeout)) {
+      try await withTimeout(defaultTimeoutDuration) {
         _ = try await self.send(
           InitializeRequest(
             processId: nil,
@@ -270,7 +270,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
   ///
   /// - Note: This also returns any notifications sent before the call to
   ///   `nextNotification`.
-  package func nextNotification(timeout: Duration = .seconds(defaultTimeout)) async throws -> any NotificationType {
+  package func nextNotification(timeout: Duration = defaultTimeoutDuration) async throws -> any NotificationType {
     return try await notifications.next(timeout: timeout)
   }
 
@@ -279,7 +279,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
   /// If the next notification is not a `PublishDiagnosticsNotification`, this
   /// methods throws.
   package func nextDiagnosticsNotification(
-    timeout: Duration = .seconds(defaultTimeout)
+    timeout: Duration = defaultTimeoutDuration
   ) async throws -> PublishDiagnosticsNotification {
     guard !usePullDiagnostics else {
       struct PushDiagnosticsError: Error, CustomStringConvertible {
@@ -295,7 +295,7 @@ package final class TestSourceKitLSPClient: MessageHandler, Sendable {
   package func nextNotification<ExpectedNotificationType: NotificationType>(
     ofType: ExpectedNotificationType.Type,
     satisfying predicate: (ExpectedNotificationType) throws -> Bool = { _ in true },
-    timeout: Duration = .seconds(defaultTimeout)
+    timeout: Duration = defaultTimeoutDuration
   ) async throws -> ExpectedNotificationType {
     while true {
       let nextNotification = try await nextNotification(timeout: timeout)

Sources/SKTestSupport/Timeouts.swift

@@ -22,3 +22,5 @@ package let defaultTimeout: TimeInterval = {
   }
   return 180
 }()
+
+package var defaultTimeoutDuration: Duration { .seconds(defaultTimeout) }

Sources/SourceKitLSP/Swift/DocumentFormatting.swift

@@ -196,7 +196,9 @@ extension SwiftLanguageService {
     writeStream.send(snapshot.text)
     try writeStream.close()
 
-    let result = try await process.waitUntilExitStoppingProcessOnTaskCancellation()
+    let result = try await withTimeout(.seconds(60)) {
+      try await process.waitUntilExitStoppingProcessOnTaskCancellation()
+    }
     guard result.exitStatus == .terminated(code: 0) else {
       let swiftFormatErrorMessage: String
       switch result.stderrOutput {

Tests/SourceKitDTests/SourceKitDTests.swift

@@ -84,15 +84,15 @@ final class SourceKitDTests: XCTestCase {
       keys.compilerArgs: args,
     ])
 
-    _ = try await sourcekitd.send(req, timeout: .seconds(defaultTimeout), fileContents: nil)
+    _ = try await sourcekitd.send(req, timeout: defaultTimeoutDuration, fileContents: nil)
 
     try await fulfillmentOfOrThrow([expectation1, expectation2])
 
     let close = sourcekitd.dictionary([
       keys.request: sourcekitd.requests.editorClose,
       keys.name: path,
     ])
-    _ = try await sourcekitd.send(close, timeout: .seconds(defaultTimeout), fileContents: nil)
+    _ = try await sourcekitd.send(close, timeout: defaultTimeoutDuration, fileContents: nil)
   }
 }
 

Tests/SourceKitLSPTests/BackgroundIndexingTests.swift

@@ -1335,14 +1335,24 @@ final class BackgroundIndexingTests: XCTestCase {
     //  - The user runs `swift package update`
     //  - This updates `Package.resolved`, which we watch
     //  - We reload the package, which updates `Dependency.swift` in `.build/index-build/checkouts`, which we also watch.
-    try await Process.run(
-      arguments: [
-        unwrap(ToolchainRegistry.forTesting.default?.swift?.filePath),
-        "package", "update",
-        "--package-path", project.scratchDirectory.filePath,
-      ],
-      workingDirectory: nil
+    let projectURL = project.scratchDirectory
+    let packageUpdateOutput = try await withTimeout(defaultTimeoutDuration) {
+      try await Process.run(
+        arguments: [
+          unwrap(ToolchainRegistry.forTesting.default?.swift?.filePath),
+          "package", "update",
+          "--package-path", projectURL.filePath,
+        ],
+        workingDirectory: nil
+      )
+    }
+    logger.debug(
+      """
+      'swift package update' output:
+      \(packageUpdateOutput)
+      """
     )
+
     XCTAssertNotEqual(try String(contentsOf: packageResolvedURL, encoding: .utf8), originalPackageResolvedContents)
     project.testClient.send(
       DidChangeWatchedFilesNotification(changes: [
@@ -1901,12 +1911,10 @@ final class BackgroundIndexingTests: XCTestCase {
   }
 
   func testIsIndexingRequest() async throws {
-    let checkedIsIndexStatus = AtomicBool(initialValue: false)
+    let checkedIsIndexStatus = MultiEntrySemaphore(name: "Checked is index status")
     let hooks = Hooks(
       indexHooks: IndexHooks(updateIndexStoreTaskDidStart: { task in
-        while !checkedIsIndexStatus.value {
-          try? await Task.sleep(for: .seconds(0.1))
-        }
+        await checkedIsIndexStatus.waitOrXCTFail()
       })
     )
     let project = try await SwiftPMTestProject(
@@ -1920,7 +1928,7 @@ final class BackgroundIndexingTests: XCTestCase {
     )
     let isIndexingResponseWhileIndexing = try await project.testClient.send(IsIndexingRequest())
     XCTAssert(isIndexingResponseWhileIndexing.indexing)
-    checkedIsIndexStatus.value = true
+    checkedIsIndexStatus.signal()
 
     try await repeatUntilExpectedResult {
       try await project.testClient.send(IsIndexingRequest()).indexing == false

Tests/SourceKitLSPTests/CompilationDatabaseTests.swift

@@ -65,21 +65,12 @@ final class CompilationDatabaseTests: XCTestCase {
       DocumentHighlight(range: positions["5️⃣"]..<positions["6️⃣"], kind: .text),
     ]
 
-    var didReceiveCorrectHighlight = false
-
     // Updating the build settings takes a few seconds.
     // Send highlight requests every second until we receive correct results.
-    for _ in 0..<30 {
+    try await repeatUntilExpectedResult {
       let postChangeHighlightResponse = try await project.testClient.send(highlightRequest)
-
-      if postChangeHighlightResponse == expectedPostEditHighlight {
-        didReceiveCorrectHighlight = true
-        break
-      }
-      try await Task.sleep(for: .seconds(1))
+      return postChangeHighlightResponse == expectedPostEditHighlight
     }
-
-    XCTAssert(didReceiveCorrectHighlight)
   }
 
   func testJSONCompilationDatabaseWithDifferentToolchainsForSwift() async throws {

Tests/SourceKitLSPTests/PullDiagnosticsTests.swift

@@ -279,20 +279,13 @@ final class PullDiagnosticsTests: XCTestCase {
   }
 
   func testDiagnosticsWaitForDocumentToBePrepared() async throws {
-    let diagnosticRequestSent = AtomicBool(initialValue: false)
+    let diagnosticRequestSent = MultiEntrySemaphore(name: "Diagnostic request sent")
     var testHooks = Hooks()
     testHooks.indexHooks.preparationTaskDidStart = { @Sendable taskDescription in
       // Only start preparation after we sent the diagnostic request. In almost all cases, this should not give
       // preparation enough time to finish before the diagnostic request is handled unless we wait for preparation in
       // the diagnostic request.
-      while diagnosticRequestSent.value == false {
-        do {
-          try await Task.sleep(for: .seconds(0.01))
-        } catch {
-          XCTFail("Did not expect sleep to fail")
-          break
-        }
-      }
+      await diagnosticRequestSent.waitOrXCTFail()
     }
 
     let project = try await SwiftPMTestProject(
@@ -331,7 +324,7 @@ final class PullDiagnosticsTests: XCTestCase {
       XCTAssertEqual(diagnostics.success?.fullReport?.items, [])
       receivedDiagnostics.fulfill()
     }
-    diagnosticRequestSent.value = true
+    diagnosticRequestSent.signal()
     try await fulfillmentOfOrThrow([receivedDiagnostics])
   }