From ed9822a5e727dcaa2d39a5e0ea55b8552019a134 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Wed, 17 Jul 2024 10:31:16 -0400 Subject: [PATCH 1/3] Fix indentation bug in test --- tests/worker/test_workflow.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/worker/test_workflow.py b/tests/worker/test_workflow.py index 30b52fbec..a89f45dbc 100644 --- a/tests/worker/test_workflow.py +++ b/tests/worker/test_workflow.py @@ -5492,7 +5492,7 @@ async def _run_workflow_and_get_warning(self) -> bool: with pytest.raises(WorkflowFailureError) as err: await handle.result() - assert "workflow execution failed" in str(err.value).lower() + assert str(err.value).lower() == "workflow execution failed" unfinished_handler_warning_emitted = any( issubclass(w.category, self._unfinished_handler_warning_cls) From 895e3b13a5a1e8acc49def58c9445a3b4e22b46f Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Wed, 17 Jul 2024 10:42:19 -0400 Subject: [PATCH 2/3] Use pytest.raises() with minimal scope This is a style fix; none of these are incorrect AFAICS --- tests/worker/test_workflow.py | 111 +++++++++++++++++----------------- 1 file changed, 55 insertions(+), 56 deletions(-) diff --git a/tests/worker/test_workflow.py b/tests/worker/test_workflow.py index a89f45dbc..a91584df4 100644 --- a/tests/worker/test_workflow.py +++ b/tests/worker/test_workflow.py @@ -1113,37 +1113,37 @@ async def cancel_child(self) -> None: @pytest.mark.parametrize("use_execute", [True, False]) async def test_workflow_cancel_child_started(client: Client, use_execute: bool): async with new_worker(client, CancelChildWorkflow, LongSleepWorkflow) as worker: - with pytest.raises(WorkflowFailureError) as err: - # Start workflow - handle = await client.start_workflow( - CancelChildWorkflow.run, - use_execute, - id=f"workflow-{uuid.uuid4()}", - task_queue=worker.task_queue, - ) + # Start workflow + handle = await client.start_workflow( + CancelChildWorkflow.run, + use_execute, + id=f"workflow-{uuid.uuid4()}", + task_queue=worker.task_queue, + ) - # Wait until child started - async def child_started() -> bool: - try: - return await handle.query( - CancelChildWorkflow.ready - ) and await client.get_workflow_handle_for( - LongSleepWorkflow.run, # type: ignore[arg-type] - workflow_id=f"{handle.id}_child", - ).query(LongSleepWorkflow.started) - except RPCError as err: - # Ignore not-found or failed precondition because child may - # not have started yet - if ( - err.status == RPCStatusCode.NOT_FOUND - or err.status == RPCStatusCode.FAILED_PRECONDITION - ): - return False - raise + # Wait until child started + async def child_started() -> bool: + try: + return await handle.query( + CancelChildWorkflow.ready + ) and await client.get_workflow_handle_for( + LongSleepWorkflow.run, # type: ignore[arg-type] + workflow_id=f"{handle.id}_child", + ).query(LongSleepWorkflow.started) + except RPCError as err: + # Ignore not-found or failed precondition because child may + # not have started yet + if ( + err.status == RPCStatusCode.NOT_FOUND + or err.status == RPCStatusCode.FAILED_PRECONDITION + ): + return False + raise - await assert_eq_eventually(True, child_started) - # Send cancel signal and wait on the handle - await handle.signal(CancelChildWorkflow.cancel_child) + await assert_eq_eventually(True, child_started) + # Send cancel signal and wait on the handle + await handle.signal(CancelChildWorkflow.cancel_child) + with pytest.raises(WorkflowFailureError) as err: await handle.result() assert isinstance(err.value.cause, ChildWorkflowError) assert isinstance(err.value.cause.cause, CancelledError) @@ -2374,17 +2374,17 @@ async def test_workflow_already_started(client: Client, env: WorkflowEnvironment async with new_worker(client, LongSleepWorkflow) as worker: id = f"workflow-{uuid.uuid4()}" # Try to start it twice + await client.start_workflow( + LongSleepWorkflow.run, + id=id, + task_queue=worker.task_queue, + ) with pytest.raises(WorkflowAlreadyStartedError): await client.start_workflow( LongSleepWorkflow.run, id=id, task_queue=worker.task_queue, ) - await client.start_workflow( - LongSleepWorkflow.run, - id=id, - task_queue=worker.task_queue, - ) @workflow.defn @@ -3263,15 +3263,15 @@ async def test_workflow_custom_failure_converter(client: Client): client = Client(**config) # Run workflow and confirm error - with pytest.raises(WorkflowFailureError) as err: - async with new_worker( - client, CustomErrorWorkflow, activities=[custom_error_activity] - ) as worker: - handle = await client.start_workflow( - CustomErrorWorkflow.run, - id=f"workflow-{uuid.uuid4()}", - task_queue=worker.task_queue, - ) + async with new_worker( + client, CustomErrorWorkflow, activities=[custom_error_activity] + ) as worker: + handle = await client.start_workflow( + CustomErrorWorkflow.run, + id=f"workflow-{uuid.uuid4()}", + task_queue=worker.task_queue, + ) + with pytest.raises(WorkflowFailureError) as err: await handle.result() # Check error is as expected @@ -4606,13 +4606,13 @@ async def test_workflow_timeout_support(client: Client, approach: str): client, TimeoutSupportWorkflow, activities=[wait_cancel] ) as worker: # Run and confirm activity gets cancelled + handle = await client.start_workflow( + TimeoutSupportWorkflow.run, + approach, + id=f"workflow-{uuid.uuid4()}", + task_queue=worker.task_queue, + ) with pytest.raises(WorkflowFailureError) as err: - handle = await client.start_workflow( - TimeoutSupportWorkflow.run, - approach, - id=f"workflow-{uuid.uuid4()}", - task_queue=worker.task_queue, - ) await handle.result() assert isinstance(err.value.cause, ActivityError) assert isinstance(err.value.cause.cause, CancelledError) @@ -4946,8 +4946,8 @@ async def run(self, param: str) -> None: async def test_workflow_fail_on_bad_input(client: Client): - with pytest.raises(WorkflowFailureError) as err: - async with new_worker(client, FailOnBadInputWorkflow) as worker: + async with new_worker(client, FailOnBadInputWorkflow) as worker: + with pytest.raises(WorkflowFailureError) as err: await client.execute_workflow( "FailOnBadInputWorkflow", 123, @@ -5484,11 +5484,10 @@ async def _run_workflow_and_get_warning(self) -> bool: assert update_task with pytest.raises(RPCError) as update_err: await update_task - assert ( - update_err.value.status == RPCStatusCode.NOT_FOUND - and "workflow execution already completed" - in str(update_err.value).lower() - ) + assert update_err.value.status == RPCStatusCode.NOT_FOUND and ( + str(update_err.value).lower() + == "workflow execution already completed" + ) with pytest.raises(WorkflowFailureError) as err: await handle.result() From 7c8e774fd08a743d737d8edfa6f38cf4cd772767 Mon Sep 17 00:00:00 2001 From: Dan Davison Date: Wed, 17 Jul 2024 11:37:46 -0400 Subject: [PATCH 3/3] Fix assertions about client-side cancellation / failure errors --- tests/worker/test_workflow.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/worker/test_workflow.py b/tests/worker/test_workflow.py index a91584df4..6ce265536 100644 --- a/tests/worker/test_workflow.py +++ b/tests/worker/test_workflow.py @@ -5491,7 +5491,12 @@ async def _run_workflow_and_get_warning(self) -> bool: with pytest.raises(WorkflowFailureError) as err: await handle.result() - assert str(err.value).lower() == "workflow execution failed" + assert isinstance( + err.value.cause, + {"cancellation": CancelledError, "failure": ApplicationError}[ + self.workflow_termination_type + ], + ) unfinished_handler_warning_emitted = any( issubclass(w.category, self._unfinished_handler_warning_cls)