Skip to content
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

Make ProcessScheduleServiceTest deterministic #11263

Merged
merged 4 commits into from
Dec 14, 2022

Conversation

deepthidevaki
Copy link
Contributor

@deepthidevaki deepthidevaki commented Dec 14, 2022

Description

Migrated ProcessScheduleServiceTest to use ControlledActorScheduler.
Except two tests shouldNotExecuteScheduledTaskIfNotInProcessingPhase and shouldExecuteScheduledTaskInProcessing, all other tests are deterministic and does not rely on timeouts. These two tests repeatedly submit tasks. So we cannot use workUntilDone to wait for all tasks to be executed. So they rely on timeouts and hopes that the expected task is executed within the timeout.

Related issues

closes #10887

@github-actions
Copy link
Contributor

Test Results

   963 files  ±    0     963 suites  ±0   2h 2m 26s ⏱️ + 8m 8s
7 714 tests +175  7 707 ✔️ +175  7 💤 ±0  0 ±0 
7 921 runs  +175  7 912 ✔️ +175  9 💤 ±0  0 ±0 

Results for commit 590466a. ± Comparison against base commit b91cdd3.

@deepthidevaki deepthidevaki marked this pull request as ready for review December 14, 2022 11:13
Copy link
Member

@npepinpe npepinpe left a comment

Choose a reason for hiding this comment

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

👍 Very nice! Can you share the modification to our team about the new helpers in ControlledActorScheduler and how they can be used, esp. when using timers?

I guess we should also backport this? Or is 8.1 not affected? The ControlledActorScheduler changes might be useful even in 8.0 as well, even if we don't have the engine refactoring parts.

@deepthidevaki
Copy link
Contributor Author

This should be backported. Atleast some of these tests are already in 8.1.

@deepthidevaki
Copy link
Contributor Author

bors merge

@zeebe-bors-camunda
Copy link
Contributor

Build succeeded:

@zeebe-bors-camunda zeebe-bors-camunda bot merged commit 68914b4 into main Dec 14, 2022
@zeebe-bors-camunda zeebe-bors-camunda bot deleted the dd-process-scheduling-actor-test branch December 14, 2022 13:27
@backport-action
Copy link
Collaborator

Backport failed for stable/8.1, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally.

git fetch origin stable/8.1
git worktree add -d .worktree/backport-11263-to-stable/8.1 origin/stable/8.1
cd .worktree/backport-11263-to-stable/8.1
git checkout -b backport-11263-to-stable/8.1
ancref=$(git merge-base b91cdd3bc4da5cf3518bef4566a9aa947746a82c 590466a53ec95496a2d43b8246e1c7bd4587de1d)
git cherry-pick -x $ancref..590466a53ec95496a2d43b8246e1c7bd4587de1d

zeebe-bors-camunda bot added a commit that referenced this pull request Dec 14, 2022
11266: [Backport/stable/8.1] Make ProcessSchedulerServiceTest deterministic r=npepinpe a=deepthidevaki

## Description

Backport #11263 

## Related issues

closes #10887 


Co-authored-by: Deepthi Devaki Akkoorath <deepthidevaki@gmail.com>
@Zelldon Zelldon added the version:8.1.6 Marks an issue as being completely or in parts released in 8.1.6 label Jan 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version:8.1.6 Marks an issue as being completely or in parts released in 8.1.6
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ProcessingScheduleServiceTest.shouldPreserveOrderingOfWritesEvenWithRetries is flaky
4 participants