Skip to content

Conversation

deepthidevaki
Copy link
Contributor

@deepthidevaki deepthidevaki commented Aug 26, 2025

Description

Due to migration that runs on cluster bootstrap, sometimes we may have snapshot after startup. So the pre-condition for this test that a snapshot does not exist might not always satisfy.

Since the snapshot at position 0 do not actually have the state changes from StreamProcessor, this test still has value. So we keep the test to cover such cases, but only remove the pre-condition.

Alternately, we can verify that if the snapshot exists it is at position 0. But I think we don't need to be that strict for this test.

Context: slack thread

Checklist

@github-actions github-actions bot added the component/zeebe Related to the Zeebe component/team label Aug 26, 2025
Due to migration that runs on cluster bootstrap, sometimes we may
have snapshot after startup. So the pre-condition for this test that a
snapshot does not exist might not always satisfy.

Since the snapshot at position 0 do not actually have the state changes from
StreamProcessor, this test still has value. So we keep the test to cover such cases,
but only remove the pre-condition.
@deepthidevaki deepthidevaki force-pushed the dd-8.6-flaky-no-snapshot-test branch from 3cf09cb to c0d0edb Compare August 26, 2025 08:00
@deepthidevaki deepthidevaki changed the title test: accept snapshot at position 0 for no snapshot update test test: remove pre-condition that snapshot do not exists Aug 26, 2025
@deepthidevaki deepthidevaki requested a review from npepinpe August 26, 2025 08:07
@deepthidevaki deepthidevaki marked this pull request as ready for review August 26, 2025 08:07
@deepthidevaki
Copy link
Contributor Author

I will port it to 8.7 and main as well.

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.

No blocker, just wondering if we want to be stricter in case the precondition changes again in the future.

@@ -46,7 +46,6 @@ void update(
state.withNetwork(network).withOldBroker().start(true);
final long processInstanceKey = testCase.setUp(state.client());
final long key = testCase.runBefore(state);
assertThat(state).hasNoSnapshotAvailable(1);
Copy link
Member

Choose a reason for hiding this comment

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

🤔 Should we assert it's at position 0? 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did that initially - 751d68c

But I think we don't need to be that strict. If you want, I can bring those changes back.

@npepinpe
Copy link
Member

I guess this has to be forward ported to 8.7 and 8.8 and main?

@deepthidevaki deepthidevaki added backport stable/8.7 Backport a pull request to stable/8.7 backport main Forward-port a pull request to main labels Aug 26, 2025
@deepthidevaki deepthidevaki added this pull request to the merge queue Aug 26, 2025
Merged via the queue into stable/8.6 with commit 651d9a6 Aug 26, 2025
63 checks passed
@deepthidevaki deepthidevaki deleted the dd-8.6-flaky-no-snapshot-test branch August 26, 2025 08:35
@backport-action
Copy link
Collaborator

Successfully created backport PR for main:

@backport-action
Copy link
Collaborator

Successfully created backport PR for stable/8.7:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport main Forward-port a pull request to main backport stable/8.7 Backport a pull request to stable/8.7 component/zeebe Related to the Zeebe component/team version:dryrun-8.8.0-alpha6-rc999 version:8.7.12 version:8.8.0-alpha8-rc1 version:8.8.0-alpha8
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants