Develop to master (#6)

Sources/ChessKitEngine/Engine.swift

@@ -92,19 +92,15 @@ public final class Engine: Sendable {
     public func start(
         coreCount: Int? = nil,
         multipv: Int = 1
-    ) {
-        Task {
-            await engineConfigurationActor.setAsyncStream()
-            
-            setMessengerResponseHandler(coreCount: coreCount, multipv: multipv)
+    ) async {
+        //Setup async stream response if not already set.
+        await engineConfigurationActor.setAsyncStream()
+        
+        setMessengerResponseHandler(coreCount: coreCount, multipv: multipv)
+        messenger.start()
 
-            await MainActor.run {
-                messenger.start()
-            }
-
-            // start engine setup loop
-            await send(command: .uci)
-        }
+        // start engine setup loop
+        await send(command: .uci)
     }
 
     /// Stops the engine.

Sources/ChessKitEngineCore/EngineMessenger/EngineMessenger.mm

@@ -59,7 +59,13 @@ NSLock *_lock;
      object:_pipeReadHandle
     ];
 
-    [_pipeReadHandle readInBackgroundAndNotify];
+    dispatch_async(dispatch_get_main_queue(), ^{
+        //This has to run on a thread that has an active run loop
+        //otherwise we don't get notified when a read occurs.
+        //Since we are using async, the only active run loop we can
+        //guarentee to have an active run loop is the main thread.
+        [_pipeReadHandle readInBackgroundAndNotify];
+    });
 
     // set up write pipe
     _writePipe = [NSPipe pipe];

Tests/ChessKitEngineTests/EngineTests/BaseEngineTests.swift

@@ -23,6 +23,7 @@ import XCTest
 /// }
 /// ```
 ///
+//@TestsActor
 class BaseEngineTests: XCTestCase {
     
     override class var defaultTestSuite: XCTestSuite {
@@ -46,75 +47,97 @@ class BaseEngineTests: XCTestCase {
     override func tearDown() async throws {
         await engine.stop()
         engine = nil
-        try? await super.tearDown()
     }
     
-    func testEngineSetup() async {
+    func testEngineStart() async {
+        XCTAssert(!Thread.isMainThread, "Test must be run on a background thread")
+        XCTAssertNotNil(self.engine, "Failed to initialize engine")
+
         let expectation = self.expectation(
             description: "Expect engine \(engine.type.name) to start up."
         )
         
-        guard let engine = self.engine else {
-            XCTFail("Engine is nil")
-            return
-        }
+        await startEngine(expectation: expectation)
         
-        engine.start()
-        
-        Task{
-            for await response in await engine.responseStream! {
-                if case let .id(id) = response,
-                   case let .name(name) = id {
-                    let version = engine.type.version
-                    XCTAssertTrue(name.contains(version))
-                }
-                
-                let isRunning = await engine.isRunning
-                
-                if response == .readyok &&
-                    isRunning {
-                    expectation.fulfill()
-                }
-            }
-        }
         await fulfillment(of: [expectation], timeout: 5)
     }
     
     func testEngineStop() async {
+        XCTAssert(!Thread.isMainThread, "Test must be run on a background thread")
+        XCTAssertNotNil(self.engine, "Failed to initialize engine")
+        
         let expectationStartEngine = self.expectation(
             description: "Expect engine \(engine.type.name) to start up."
         )
-        
         let expectationStopEngine = self.expectation(
             description: "Expect engine \(engine.type.name) to stop gracefully."
         )
         
-        guard let engine = self.engine else {
-            XCTFail("Engine is nil")
-            return
-        }
+        await startEngine(expectation: expectationStartEngine)
         
-        engine.start()
-        
-        Task{
-            for await response in await engine.responseStream! {
-                let isRunning = await engine.isRunning
-                
-                if response == .readyok &&
-                    isRunning {
-                    expectationStartEngine.fulfill()
-                    break
-                }
-            }
-            
-            await engine.stop()
-            
-            if await !engine.isRunning,
-               await engine.responseStream == nil {
-                expectationStopEngine.fulfill()
-            }
-        }
+        await stopEngine(expectation: expectationStopEngine)
         
         await fulfillment(of: [expectationStartEngine, expectationStopEngine], timeout: 5)
     }
+    
+    func testEngineRestart() async {
+        XCTAssert(!Thread.isMainThread, "Test must be run on a background thread")
+        XCTAssertNotNil(self.engine, "Failed to initialize engine")
+
+        let expectationStartEngine = self.expectation(
+            description: "Expect engine \(engine.type.name) to start up."
+        )
+        let expectationStopEngine = self.expectation(
+            description: "Expect engine \(engine.type.name) to stop gracefully."
+        )
+        
+        expectationStartEngine.expectedFulfillmentCount = 2
+        
+        await startEngine(expectation: expectationStartEngine)
+        await stopEngine(expectation: expectationStopEngine)
+        await startEngine(expectation: expectationStartEngine)
+        
+        await fulfillment(of: [expectationStartEngine, expectationStopEngine], timeout: 5)
+    }
+    
+    
+    internal func stopEngine(expectation: XCTestExpectation) async {
+        await engine.stop()
+        
+        if await !engine.isRunning,
+           await engine.responseStream == nil {
+            expectation.fulfill()
+        }
+    }
+    
+    internal func startEngine(expectation: XCTestExpectation) async {
+        await engine.start()
+        
+        for await response in await engine.responseStream! {
+            if case let .id(id) = response,
+               case let .name(name) = id {
+                let version = engine.type.version
+                XCTAssertTrue(name.contains(version))
+            }
+            
+            let isRunning = await engine.isRunning
+            
+            if response == .readyok &&
+                isRunning {
+                expectation.fulfill()
+                break
+            }
+        }
+    }
+}
+
+//This actor's purpose is to ensure tests for the engine
+//class aren't running on main thread.
+//Since [EngineMessenger start] function now uses
+//`dispatch_async(dispatch_get_main_queue, (), ^{...});`
+//which is the main thread to listen for read notifications,
+//testing on main thread is counter productive.
+@globalActor
+actor TestsActor: GlobalActor {
+    static var shared = TestsActor()
 }

Tests/ChessKitEngineTests/EngineTests/Lc0Tests.swift

@@ -13,4 +13,29 @@ final class Lc0Tests: BaseEngineTests {
         super.setUp()
     }
 
+    override func testEngineRestart() async {
+        XCTAssert(!Thread.isMainThread, "Test must be run on a background thread")
+        XCTAssertNotNil(self.engine, "Failed to initialize engine")
+
+        let expectationStartEngine = self.expectation(
+            description: "Expect engine \(engine.type.name) to start up."
+        )
+        let expectationStopEngine = self.expectation(
+            description: "Expect engine \(engine.type.name) to stop gracefully."
+        )
+        
+        expectationStartEngine.expectedFulfillmentCount = 2
+        
+        await startEngine(expectation: expectationStartEngine)
+        await stopEngine(expectation: expectationStopEngine)
+        //LC0 has an internal mutex failure "Unhandled exception: mutex lock failed: Invalid argument"
+        //when trying to stop and start the engine too fast.
+        //Adding this 100 ms delay circumvent that issue.
+        //Once this issue is resolved, this override func
+        //can be removed and use the EngineRestart test on BeseEngineTests
+        try? await Task.sleep(for: .milliseconds(100))
+        await startEngine(expectation: expectationStartEngine)
+        
+        await fulfillment(of: [expectationStartEngine, expectationStopEngine], timeout: 5)
+    }
 }