Fix two flakyOutOfProcessReferenceResolver tests (#374)

* Fix flaky test by handling partial response data chunks.

* Fix another flaky test

* Reduce the risk of a deadlock when reading additional response chunks

Author: d-ronnqvist

Sources/SwiftDocC/Infrastructure/External Data/OutOfProcessReferenceResolver.swift

@@ -437,7 +437,7 @@ private class LongRunningProcess: ExternalLinkResolving {
 
         try process.run()
 
-        let errorReadSource = DispatchSource.makeReadSource(fileDescriptor: errorOutput.fileHandleForReading.fileDescriptor)
+        let errorReadSource = DispatchSource.makeReadSource(fileDescriptor: errorOutput.fileHandleForReading.fileDescriptor, queue: .main)
         errorReadSource.setEventHandler { [errorOutput] in
             let data = errorOutput.fileHandleForReading.availableData
             let errorMessage = String(data: data, encoding: .utf8)
@@ -462,23 +462,39 @@ private class LongRunningProcess: ExternalLinkResolving {
 
     func sendAndWait<Request: Codable & CustomStringConvertible, Response: Codable>(request: Request?) throws -> Response {
         if let request = request {
-            guard let requestString = String(
-                    data: try JSONEncoder().encode(request), encoding: .utf8)?.appending("\n"),
+            guard let requestString = String(data: try JSONEncoder().encode(request), encoding: .utf8)?.appending("\n"),
                   let requestData = requestString.data(using: .utf8)
             else {
                 throw OutOfProcessReferenceResolver.Error.unableToEncodeRequestToClient(requestDescription: request.description)
             }
             input.fileHandleForWriting.write(requestData)
         }
-        let response = output.fileHandleForReading.availableData
+        var response = output.fileHandleForReading.availableData
         guard !response.isEmpty else {
             throw OutOfProcessReferenceResolver.Error.processDidExit(code: Int(process.terminationStatus))
         }
 
-        do {
-            return try JSONDecoder().decode(Response.self, from: response)
-        } catch {
-            throw OutOfProcessReferenceResolver.Error.unableToDecodeResponseFromClient(response, error)
+        // It's not guaranteed that the full response will be available all at once.
+        while true {
+            // If a pipe is empty, checking `availableData` will block until there is new data to read.
+            do {
+                // To avoid blocking forever we check if the response can be decoded after each chunk of data.
+                return try JSONDecoder().decode(Response.self, from: response)
+            } catch {
+                if case DecodingError.dataCorrupted = error,     // If the data wasn't valid JSON, read more data and try to decode it again.
+                    response.count.isMultiple(of: Int(PIPE_BUF)) // To reduce the risk of deadlocking, check that bytes so far is a multiple of the pipe buffer size.
+                {
+                    let moreResponseData = output.fileHandleForReading.availableData
+                    guard !moreResponseData.isEmpty else {
+                        throw OutOfProcessReferenceResolver.Error.processDidExit(code: Int(process.terminationStatus))
+                    }
+                    response += moreResponseData
+                    continue
+                }
+            
+                // Other errors are re-thrown as wrapped errors.
+                throw OutOfProcessReferenceResolver.Error.unableToDecodeResponseFromClient(response, error)
+            }
         }
     }
 

Tests/SwiftDocCUtilitiesTests/OutOfProcessReferenceResolverTests.swift

@@ -145,27 +145,25 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
 
     func testResolvingTopicLinkProcess() throws {
         #if os(macOS)
-        throw XCTSkip("This test is flaky (rdar://91678333)")
-        
-//        try assertResolvesTopicLink(makeResolver: { testMetadata in
-//            let temporaryFolder = try createTemporaryDirectory()
-//            let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
-//            
-//            let encodedMetadata = try String(data: JSONEncoder().encode(testMetadata), encoding: .utf8)!
-//            
-//            try """
-//            #!/bin/bash
-//            echo '{"bundleIdentifier":"com.test.bundle"}'       # Write this resolver's bundle identifier
-//            read                                                # Wait for docc to send a topic URL
-//            echo '{"resolvedInformation":\(encodedMetadata)}'   # Respond with the test metadata (above)
-//            """.write(to: executableLocation, atomically: true, encoding: .utf8)
-//            
-//            // `0o0700` is `-rwx------` (read, write, & execute only for owner)
-//            try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
-//            XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
-//             
-//            return try OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: { _ in })
-//        })
+        try assertResolvesTopicLink(makeResolver: { testMetadata in
+            let temporaryFolder = try createTemporaryDirectory()
+            let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
+            
+            let encodedMetadata = try String(data: JSONEncoder().encode(testMetadata), encoding: .utf8)!
+            
+            try """
+            #!/bin/bash
+            echo '{"bundleIdentifier":"com.test.bundle"}'       # Write this resolver's bundle identifier
+            read                                                # Wait for docc to send a topic URL
+            echo '{"resolvedInformation":\(encodedMetadata)}'   # Respond with the test metadata (above)
+            """.write(to: executableLocation, atomically: true, encoding: .utf8)
+            
+            // `0o0700` is `-rwx------` (read, write, & execute only for owner)
+            try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
+            XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
+             
+            return try OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: { _ in })
+        })
 
         #endif
     }
@@ -370,34 +368,32 @@ class OutOfProcessReferenceResolverTests: XCTestCase {
     }
 
     func testForwardsErrorOutputProcess() throws {
-        throw XCTSkip("This test is flaky rdar://88498898")
-
-//        #if os(macOS)
-//        let temporaryFolder = try createTemporaryDirectory()
-//        
-//        let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
-//        try """
-//        #!/bin/bash
-//        echo "Some error output" 1>&2                   # Write to stderr
-//        echo '{"bundleIdentifier":"com.test.bundle"}'   # Write this resolver's bundle identifier
-//        read                                            # Wait for docc to send a topic URL
-//        """.write(to: executableLocation, atomically: true, encoding: .utf8)
-//        
-//        // `0o0700` is `-rwx------` (read, write, & execute only for owner)
-//        try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
-//        XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
-//         
-//        let didReadErrorOutputExpectation = AsyncronousExpectation(description: "Did read forwarded error output.")
-//        DispatchQueue.global().async {
-//            let resolver = try? OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: {
-//                errorMessage in
-//                XCTAssertEqual(errorMessage, "Some error output\n")
-//                didReadErrorOutputExpectation.fulfill()
-//            })
-//            XCTAssertEqual(resolver?.bundleIdentifier, "com.test.bundle")
-//        }
-//        XCTAssertNotEqual(didReadErrorOutputExpectation.wait(timeout: 20.0), .timedOut)
-//        #endif
+        #if os(macOS)
+        let temporaryFolder = try createTemporaryDirectory()
+        
+        let executableLocation = temporaryFolder.appendingPathComponent("link-resolver-executable")
+        try """
+        #!/bin/bash
+        echo '{"bundleIdentifier":"com.test.bundle"}'   # Write this resolver's bundle identifier
+        echo "Some error output" 1>&2                   # Write to stderr
+        read                                            # Wait for docc to send a topic URL
+        """.write(to: executableLocation, atomically: true, encoding: .utf8)
+        
+        // `0o0700` is `-rwx------` (read, write, & execute only for owner)
+        try FileManager.default.setAttributes([.posixPermissions: 0o0700], ofItemAtPath: executableLocation.path)
+        XCTAssert(FileManager.default.isExecutableFile(atPath: executableLocation.path))
+         
+        let didReadErrorOutputExpectation = expectation(description: "Did read forwarded error output.")
+        
+        let resolver = try? OutOfProcessReferenceResolver(processLocation: executableLocation, errorOutputHandler: {
+            errorMessage in
+            XCTAssertEqual(errorMessage, "Some error output\n")
+            didReadErrorOutputExpectation.fulfill()
+        })
+        XCTAssertEqual(resolver?.bundleIdentifier, "com.test.bundle")
+        
+        wait(for: [didReadErrorOutputExpectation], timeout: 20.0)
+        #endif
     }
 
     func assertForwardsResolverErrors(resolver: OutOfProcessReferenceResolver) throws {