Skip to content

Conversation

@tconley1428
Copy link
Contributor

What was changed

Why?

Checklist

  1. Closes

  2. How was this tested:

  1. Any docs updates needed?

@tconley1428 tconley1428 changed the title Run the test a lot Fix flaky patch test Sep 23, 2025
@tconley1428 tconley1428 marked this pull request as ready for review September 23, 2025 19:34
@tconley1428 tconley1428 requested a review from a team as a code owner September 23, 2025 19:34
@tconley1428 tconley1428 changed the title Fix flaky patch test Fix flaky test: test_workflow_patch_memoized Sep 23, 2025
PatchMemoizedWorkflowPatched.waiting_signal
)

await assert_eq_eventually(True, waiting_signal)
Copy link
Contributor

@dandavison dandavison Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be beyond the call of duty, but it looks to me like this test was written before update existed and that a better way to implement this nowadays would be

    @workflow.update
    async def wait_until_waiting_signal_is_true(self) -> None:
        await workflow.wait_condition(lambda: self._waiting_signal)

...

        # Need to wait until it has gotten halfway through
        await pre_patch_handle.execute_update(
            PatchMemoizedWorkflowUnpatched.wait_until_waiting_signal_is_true
        )

[Tested and that seems to work]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair, but I'll leave it as long as it is working.

@tconley1428 tconley1428 merged commit 6dd59dd into main Sep 24, 2025
24 of 28 checks passed
@tconley1428 tconley1428 deleted the workflow_patch_memoize_flake branch September 24, 2025 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants