-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(tests): stabilize flaky durable timeout tests #9035
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,11 +70,8 @@ def test_local_invoke_durable_function_timeout(self): | |
| example = DurableFunctionExamples.EXECUTION_TIMEOUT | ||
| function_name = example.function_name | ||
| execution_name = "executiontimeout-integration-test" | ||
| event_path = str(self.test_data_path / "durable" / "events" / "timeout_test_event.json") | ||
|
|
||
| command_list = self.get_invoke_command_list( | ||
| function_name, event_path=event_path, durable_execution_name=execution_name | ||
| ) | ||
| command_list = self.get_invoke_command_list(function_name, no_event=True, durable_execution_name=execution_name) | ||
|
|
||
| stdout, stderr, invoke_return_code = self.run_command_with_logging( | ||
| command_list, f"test_local_invoke_durable_function_{function_name}" | ||
|
|
@@ -84,7 +81,7 @@ def test_local_invoke_durable_function_timeout(self): | |
|
|
||
| # Assert invoke output shows timeout | ||
| execution_arn = self.assert_invoke_output( | ||
| stdout, input_data={"wait_seconds": 30}, execution_name=execution_name, expected_status="TIMED_OUT" | ||
| stdout, input_data={}, execution_name=execution_name, expected_status="TIMED_OUT" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Finding 2 of 3: this weakens the only durable invoke test that asserted a non-trivial input made it into the execution-history output. Before: After: If the goal is just stabilization, that's fine, but please consider keeping at least one durable test that asserts the input round-trips correctly through invoke → handler → execution history. |
||
| ) | ||
|
|
||
| # Get and verify execution history | ||
|
|
||
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| from typing import Any | ||
|
|
||
| from aws_durable_execution_sdk_python.context import DurableContext | ||
| from aws_durable_execution_sdk_python.execution import durable_execution | ||
| from aws_durable_execution_sdk_python.config import Duration | ||
|
|
||
|
|
||
| @durable_execution | ||
| def handler(event: Any, context: DurableContext) -> str: | ||
| context.wait(Duration.from_seconds(30), name="custom_wait") | ||
| return "Wait with name completed" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import logging | ||
| from typing import Any | ||
|
|
||
| from aws_durable_execution_sdk_python.config import WaitForCallbackConfig | ||
| from aws_durable_execution_sdk_python.context import ( | ||
| DurableContext, | ||
| WaitForCallbackContext, | ||
| ) | ||
| from aws_durable_execution_sdk_python.execution import durable_execution | ||
| from aws_durable_execution_sdk_python.config import Duration | ||
|
|
||
| logger = logging.getLogger() | ||
| logger.setLevel(logging.INFO) | ||
|
|
||
|
|
||
| def external_system_call(callback_id: str, _context: WaitForCallbackContext) -> None: | ||
| logger.info(f"Waiting for callback: {callback_id}") | ||
|
|
||
|
|
||
| @durable_execution | ||
| def handler(event: Any, context: DurableContext) -> str: | ||
| config = WaitForCallbackConfig( | ||
| timeout=Duration.from_seconds(5), | ||
| heartbeat_timeout=Duration.from_seconds(3), | ||
| ) | ||
|
|
||
| result = context.wait_for_callback( | ||
| external_system_call, name="external_call", config=config | ||
| ) | ||
|
|
||
| return f"External system result: {result}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finding 3 of 3: the
-e <event_path>code path for durable invokes is no longer exercised by the timeout test.It's still covered by
test_local_invoke_callback_heartbeat(line 181) which usesevent_path=..., so it's not a total loss — flagging mainly because the timeout flow was the test most likely to fail on a regression in event-file handling specific to durable invoke (e.g., payload encoding, file read path differences whenDurableExecutionNameis also set). Worth a sanity check that the heartbeat test would actually catch such a regression, since it doesn't assert on the event contents being honored either.