That's a great question! The real answer is that there was a bug in the logic which is now fixed. I noticed that the code got copied and I thought it's important to give examples on how to test state machines anyway (and how easy that is). The bug is now fixed, the state machine now has 100% coverage once this PR is in.
The first version of the state machine did track the resources correctly, so it would've never double closed anything, however, there was one bug: Whilst a write was ongoing (state .writing
), it used to transition straight to .error
emitting the .processingCompletedDiscardResources
Action
. That's incorrect, whilst a write is ongoing, we need to sit on the resources, wait until the write completes and then issue that Action
.
Remember, just because something's a state machine doesn't make it more correct. The benefits are that tests are easy, debugging is much much easier (all deterministic). The fix I needed to do was just
internal mutating func didError(_ error: Error) -> Action {
let oldState = self.state
self.state = .error(error)
switch oldState {
- case .idle, .error:
+ case .idle, .error, .errorWhilstWriting:
return Action(main: .nothingWeAreWaiting /* for a new request / the channel to go away */, callRead: false)
case .openingFile:
return Action(main: .processingCompletedDiscardResources(nil, error), callRead: false)
- case .readyToWrite(let fileHandle, _), .writing(let fileHandle, _):
+ case .readyToWrite(let fileHandle, _):
return Action(main: .processingCompletedDiscardResources(fileHandle, error), callRead: false)
+ case .writing(let fileHandle, _):
+ self.state = .errorWhilstWriting(fileHandle, error)
+ return Action(main: .nothingWeAreWaiting /* for the write to complete */, callRead: false)
}
}
and the corresponding test was also very straightforward:
func testErrorArrivesWhilstWriting() {
XCTAssertTrue(self.coordinator.shouldWeReadMoreDataFromNetwork())
self.coordinator.didReceiveRequestBegin(targetPath: "/test")
.assertOpenFile({ path in
XCTAssertEqual("/test", path)
})
.assertDoNotCallRead() // We're waiting for the file to open, so we better don't buffer more.
XCTAssertFalse(self.coordinator.shouldWeReadMoreDataFromNetwork())
self.coordinator.didOpenTargetFile(self.fakeFileHandle)
.assertCallRead() // Now, we want the body bytes
XCTAssertTrue(self.coordinator.shouldWeReadMoreDataFromNetwork())
self.coordinator.didReceiveRequestBodyBytes(self.byteX)
.assertStartWriting()
.assertDoNotCallRead() // We're now processing them, so let's wait again
// Now, we're currently `.writing`, so let's inject the error.
self.coordinator.didError(DummyError())
.assertNothing() // We should be told to do nothing and sit tight.
.assertDoNotCallRead()
// But now, we're done writing which should surface the error:
self.coordinator.didFinishWritingOneChunkToFile()
.assertDiscardResources({ fileHandle, error in
XCTAssertNotNil(fileHandle)
XCTAssert(error is DummyError)
})
}
Hope that helps .