From efd5374a09736c3c2bb0ead081afefe7fbfd6a3b Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 5 Apr 2023 13:31:01 -0400 Subject: [PATCH 1/2] Fixed runSideEffect key in Worker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Changed WorkerWorkflow to use it’s state as the key for the runSideEffect call so that if the state changes the worker is re-run. Updated unit tests to test for this issue of the side effect not running. Added unit tests for testing that updating the key in the render call causes the worker to run. --- WorkflowConcurrency/Sources/Worker.swift | 2 +- WorkflowConcurrency/Tests/WorkerTests.swift | 137 ++++++++++++++++++-- 2 files changed, 130 insertions(+), 9 deletions(-) diff --git a/WorkflowConcurrency/Sources/Worker.swift b/WorkflowConcurrency/Sources/Worker.swift index 0c5a2d7eb..d8e1935d2 100644 --- a/WorkflowConcurrency/Sources/Worker.swift +++ b/WorkflowConcurrency/Sources/Worker.swift @@ -61,7 +61,7 @@ struct WorkerWorkflow: Workflow { func render(state: State, context: RenderContext) -> Rendering { let logger = WorkerLogger() let sink = context.makeOutputSink() - context.runSideEffect(key: "") { lifetime in + context.runSideEffect(key: state) { lifetime in let send: @MainActor(Output) -> Void = sink.send let task = Task { logger.logStarted() diff --git a/WorkflowConcurrency/Tests/WorkerTests.swift b/WorkflowConcurrency/Tests/WorkerTests.swift index a80995620..7cc333eb6 100644 --- a/WorkflowConcurrency/Tests/WorkerTests.swift +++ b/WorkflowConcurrency/Tests/WorkerTests.swift @@ -22,7 +22,7 @@ import XCTest class WorkerTests: XCTestCase { func testWorkerOutput() { let host = WorkflowHost( - workflow: TaskTestWorkerWorkflow(key: "") + workflow: TaskTestWorkerWorkflow(key: "", initialState: 0) ) let expectation = XCTestExpectation() @@ -38,8 +38,120 @@ class WorkerTests: XCTestCase { disposable?.dispose() } + func testWorkflowUpdate() { + // Create the workflow which causes the TaskTestWorker to run. + let host = WorkflowHost( + workflow: TaskTestWorkerWorkflow(key: "", initialState: 0) + ) + + var expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called after the TaskTestWorker runs and + // updates the state. + var disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Test to make sure the initial state of the workflow is correct. + XCTAssertEqual(0, host.rendering.value) + + // Wait for the worker to run. + wait(for: [expectation], timeout: 1.0) + // Test to make sure the rendering after the worker runs is correct. + XCTAssertEqual(1, host.rendering.value) + + disposable?.dispose() + + expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called after the workflow is updated. + // After the host is updated with a new workflow instance the + // initial state should be 1. + disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Updated the workflow to a new initial state. + host.update(workflow: TaskTestWorkerWorkflow(key: "", initialState: 1)) + + // Wait for the workflow to render after being updated. + wait(for: [expectation], timeout: 1.0) + // Test to make sure the rendering matches the initial state. + XCTAssertEqual(1, host.rendering.value) + + expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called when the worker runs. + // The worker isEquivalent is false because we have changed the initialState. + disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Wait for the worker to trigger a rendering. + wait(for: [expectation], timeout: 1.0) + // Check to make sure the rendering is correct. + XCTAssertEqual(2, host.rendering.value) + } + + func testWorkflowKeyChange() { + // Create the workflow which causes the TaskTestWorker to run. + let host = WorkflowHost( + workflow: TaskTestWorkerWorkflow(key: "", initialState: 0) + ) + + var expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called after the TaskTestWorker runs and + // updates the state. + var disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Test to make sure the initial state of the workflow is correct. + XCTAssertEqual(0, host.rendering.value) + + // Wait for the worker to run. + wait(for: [expectation], timeout: 1.0) + // Test to make sure the rendering after the worker runs is correct. + XCTAssertEqual(1, host.rendering.value) + + disposable?.dispose() + + expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called after the workflow is updated. + // After the host is updated with a new workflow instance the + // initial state should be 1. + disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Update the workflow to a new key which should force the worker to run. + host.update(workflow: TaskTestWorkerWorkflow(key: "key", initialState: 0)) + + // Wait for the workflow to render after being updated. + wait(for: [expectation], timeout: 1.0) + // Test to make sure the rendering matches the existing state + // since the inititalState didn't change. + XCTAssertEqual(1, host.rendering.value) + + expectation = XCTestExpectation() + // Set to observe renderings + // This expectation should be called when the worker runs. + // The worker should run because the key was changed for the workflow. + disposable = host.rendering.signal.observeValues { rendering in + expectation.fulfill() + } + + // Wait for the worker to trigger a rendering. + wait(for: [expectation], timeout: 1.0) + // Check to make sure the rendering is correct. + // The worker adds one to the initialState so this should be 1. + XCTAssertEqual(1, host.rendering.value) + } + func testExpectedWorker() { - TaskTestWorkerWorkflow(key: "123") + TaskTestWorkerWorkflow(key: "123", initialState: 0) .renderTester() .expectWorkflow( type: WorkerWorkflow.self, @@ -127,11 +239,11 @@ private struct TaskTestWorkerWorkflow: Workflow { typealias Rendering = Int let key: String - - func makeInitialState() -> Int { 0 } + let initialState: Int + func makeInitialState() -> Int { initialState } func render(state: Int, context: RenderContext) -> Int { - TaskTestWorker() + TaskTestWorker(initialState: initialState) .mapOutput { output in AnyWorkflowAction { state in state = output @@ -141,18 +253,27 @@ private struct TaskTestWorkerWorkflow: Workflow { .running(in: context, key: key) return state } + + func workflowDidChange(from previousWorkflow: TaskTestWorkerWorkflow, state: inout Int) { + if previousWorkflow.initialState != initialState { + state = initialState + } + } } private struct TaskTestWorker: Worker { typealias Output = Int - + let initialState: Int + func run() async -> Int { do { try await Task.sleep(nanoseconds: 10000000) } catch {} - return 1 + return initialState + 1 } - func isEquivalent(to otherWorker: TaskTestWorker) -> Bool { true } + func isEquivalent(to otherWorker: TaskTestWorker) -> Bool { + return otherWorker.initialState == initialState + } } From 99e314d5fee0eed29bf43655c455a9a2424a13a6 Mon Sep 17 00:00:00 2001 From: Mark Johnson Date: Wed, 5 Apr 2023 15:44:55 -0400 Subject: [PATCH 2/2] Changed initial state to be more obvious Per CR changed intial state of the updated workflow to be a more obvious state change --- WorkflowConcurrency/Tests/WorkerTests.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/WorkflowConcurrency/Tests/WorkerTests.swift b/WorkflowConcurrency/Tests/WorkerTests.swift index 7cc333eb6..b9010da83 100644 --- a/WorkflowConcurrency/Tests/WorkerTests.swift +++ b/WorkflowConcurrency/Tests/WorkerTests.swift @@ -72,12 +72,12 @@ class WorkerTests: XCTestCase { } // Updated the workflow to a new initial state. - host.update(workflow: TaskTestWorkerWorkflow(key: "", initialState: 1)) + host.update(workflow: TaskTestWorkerWorkflow(key: "", initialState: 7)) // Wait for the workflow to render after being updated. wait(for: [expectation], timeout: 1.0) // Test to make sure the rendering matches the initial state. - XCTAssertEqual(1, host.rendering.value) + XCTAssertEqual(7, host.rendering.value) expectation = XCTestExpectation() // Set to observe renderings @@ -90,7 +90,7 @@ class WorkerTests: XCTestCase { // Wait for the worker to trigger a rendering. wait(for: [expectation], timeout: 1.0) // Check to make sure the rendering is correct. - XCTAssertEqual(2, host.rendering.value) + XCTAssertEqual(8, host.rendering.value) } func testWorkflowKeyChange() {