Create new StreamPlatformExtension #10036

Zelldon · 2022-08-09T10:46:49Z

Description

This is an attempt to create a new StreamPlatformExtension, which should allow us to replace the TestStreams, StreamProcessorComposite, and StreamProcessorRule. This is something that would help us to continue with cleaning up the engine/stream processing code (and tests). There was a recent attempt to remove the event applies from the stream processing interface, #9985 but this failed since several tests have high dependencies on how the engine and related stuff is built. They are highly coupled.

This is just a first step and I would like to validate with you @saig0 and @npepinpe, since I think you both are quite familiar with JUnit 5 extensions and our old rules. (Note: I'm a total junit 5 noob please bare with me.)

What the PR does:

Introduce a new class StreamPlatform (combination of TestStreams and StreamComposite)
Introduce a new extension to create the StreamPlatform and make this available for tests
Created an example test which is a copy of StreamProcessorReplayModeTest.java

In the test class we can see how it would look like and what the stream platform could provide us. Instead of always registering mocked typed processors in streamProcessors tests we just have on mocked RecordProcessor which can be use for verifications. I think this would simplify our tests a lot, since it is currently a lot of boilter plate which is mostly for the tests not interesting at all.

Be aware that it currently only contains one test to show how it could look like and get early feedback.
Please take a look and let me know what you think.

Related issues

closes #

Definition of Done

Not all items need to be done depending on the issue and the pull request.

Code changes:

The changes are backwards compatibility with previous versions
If it fixes a bug then PRs are created to backport the fix to the last two minor versions. You can trigger a backport by assigning labels (e.g. backport stable/1.3) to the PR, in case that fails you need to create backports manually.

Testing:

There are unit/integration tests that verify all acceptance criterias of the issue
New tests are written to ensure backwards compatibility with further versions
The behavior is tested manually
The change has been verified by a QA run
The impact of the changes is verified by a benchmark

Documentation:

The documentation is updated (e.g. BPMN reference, configuration, examples, get-started guides, etc.)
New content is added to the release announcement
If the PR changes how BPMN processes are validated (e.g. support new BPMN element) then the Camunda modeling team should be informed to adjust the BPMN linting.

Please refer to our review guidelines.

Zelldon · 2022-08-09T10:52:18Z

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatformExtension.java

+    final var store = getStore(extensionContext);
+
+    return (StreamProcessorTestContext)
+        store.getOrComputeIfAbsent(FIELD_STATE, (key) -> new StreamProcessorTestContext());


Mostly copied from Peters https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/util/ZeebeStateExtension.java I figured out that it is useful for also closing resources

Zelldon · 2022-08-09T10:55:05Z

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatform.java

+  private final WriteActor writeActor = new WriteActor();
+  private final ZeebeDbFactory zeebeDbFactory;
+
+  public StreamPlatform(


Most parts are from TestStream and StreamProcessingComposite, I tried to clean up it already a bit

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatform.java

github-actions · 2022-08-09T11:02:38Z

Test Results

  813 files +    2   813 suites +2 1h 41m 53s ⏱️ + 5m 58s
6 117 tests +118 6 106 ✔️ +117 10 💤 ±0 1 ❌ +1
6 305 runs +118 6 294 ✔️ +117 10 💤 ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit 9728088. ± Comparison against base commit b8d1030.

♻️ This comment has been updated with latest results.

npepinpe · 2022-08-09T11:41:34Z

To clarify, what's the goal for this PR? If I understood correctly, you're blocked because the test classes are too tightly coupled with the engine API? I'm just trying to figure out what kind of feedback do you want 😄

Zelldon · 2022-08-09T11:52:40Z

To clarify, what's the goal for this PR? If I understood correctly, you're blocked because the test classes are too tightly coupled with the engine API? I'm just trying to figure out what kind of feedback do you want smile

Since I have zero experience with writing extensions I would like to have first general feedback: This looks like a valid extension or it is crap.

Furthemore, I would like to know whether you think this is something we want to use to test our StreamProcessor. I would like that we add some point test the stream processor separately from the engine, with just a mocked or dummy RecordProcessor, similar like what is done in the PR. To just validate the API and its guarantees, but without coupling to the real engine processing logic.

Blocked is a hard word I have also other things I can do but yes it would help to make progress in this part.

Zelldon · 2022-08-10T04:43:09Z

I see it as a first step in migrating our existing tests, so if you are ok with it and we merge it we can slowly migrate the other tests and delete at some point the old ones and hopefully at some point the old Rules. Of course, we can always iterate over the extension and StreamPlatform class (and we potentially also have to).

saig0

@Zelldon the extension looks good so far 👍

I think that we can clean it up here and there. Especially, the StreamPlatform looks very similar to our existing classes. But I'm fine with improving it over time when we migrate more tests and know what we really need.

Zelldon · 2022-08-15T06:14:32Z

Thanks @saig0 for your review :) Yes makes sense since it is a copy/combination of them. As I said I also see that as a first step and we can improve but I think this helps us already to get the stone rolling :)

Zelldon · 2022-08-15T06:14:36Z

bors r+

10036: Create new StreamPlatformExtension r=Zelldon a=Zelldon ## Description This is an attempt to create a new StreamPlatformExtension, which should allow us to replace the TestStreams, StreamProcessorComposite, and StreamProcessorRule. This is something that would help us to continue with cleaning up the engine/stream processing code (and tests). There was a recent attempt to remove the event applies from the stream processing interface, #9985 but this failed since several tests have high dependencies on how the engine and related stuff is built. They are highly coupled. This is just a first step and I would like to validate with you `@saig0` and `@npepinpe,` since I think you both are quite familiar with JUnit 5 extensions and our old rules. (Note: I'm a total junit 5 noob please bare with me.) _What the PR does:_ - Introduce a new class StreamPlatform (combination of [TestStreams](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/util/TestStreams.java) and [StreamComposite](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/util/StreamProcessingComposite.java)) - Introduce a new extension to create the StreamPlatform and make this available for tests - Created an example test which is a copy of [StreamProcessorReplayModeTest.java](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/processing/streamprocessor/StreamProcessorReplayModeTest.java) In the test class we can see how it would look like and what the stream platform could provide us. Instead of always registering mocked typed processors in streamProcessors tests we just have on mocked RecordProcessor which can be use for verifications. I think this would simplify our tests a lot, since it is currently a lot of boilter plate which is mostly for the tests not interesting at all. Be aware that it currently only contains one test to show how it could look like and get early feedback. Please take a look and let me know what you think.  ## Related issues  closes # Co-authored-by: Christopher Zell <zelldon91@googlemail.com>

zeebe-bors-camunda · 2022-08-15T06:34:05Z

Build failed:

Test summary

Zelldon · 2022-08-15T06:36:35Z

bors r+

10036: Create new StreamPlatformExtension r=Zelldon a=Zelldon ## Description This is an attempt to create a new StreamPlatformExtension, which should allow us to replace the TestStreams, StreamProcessorComposite, and StreamProcessorRule. This is something that would help us to continue with cleaning up the engine/stream processing code (and tests). There was a recent attempt to remove the event applies from the stream processing interface, #9985 but this failed since several tests have high dependencies on how the engine and related stuff is built. They are highly coupled. This is just a first step and I would like to validate with you `@saig0` and `@npepinpe,` since I think you both are quite familiar with JUnit 5 extensions and our old rules. (Note: I'm a total junit 5 noob please bare with me.) _What the PR does:_ - Introduce a new class StreamPlatform (combination of [TestStreams](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/util/TestStreams.java) and [StreamComposite](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/util/StreamProcessingComposite.java)) - Introduce a new extension to create the StreamPlatform and make this available for tests - Created an example test which is a copy of [StreamProcessorReplayModeTest.java](https://github.com/camunda/zeebe/blob/main/engine/src/test/java/io/camunda/zeebe/engine/processing/streamprocessor/StreamProcessorReplayModeTest.java) In the test class we can see how it would look like and what the stream platform could provide us. Instead of always registering mocked typed processors in streamProcessors tests we just have on mocked RecordProcessor which can be use for verifications. I think this would simplify our tests a lot, since it is currently a lot of boilter plate which is mostly for the tests not interesting at all. Be aware that it currently only contains one test to show how it could look like and get early feedback. Please take a look and let me know what you think.  ## Related issues  closes # Co-authored-by: Christopher Zell <zelldon91@googlemail.com>

zeebe-bors-camunda · 2022-08-15T07:23:16Z

Build failed:

Test summary

Zelldon · 2022-08-15T07:49:26Z

bors r+

zeebe-bors-camunda · 2022-08-15T08:05:04Z

Build succeeded:

test: create StreamPlatformExtension

ca16cb2

Zelldon requested review from saig0 and npepinpe August 9, 2022 10:47

test: remove unuse tests

541dd59

Zelldon commented Aug 9, 2022

View reviewed changes

github-advanced-security bot found potential problems Aug 9, 2022

View reviewed changes

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatform.java Fixed Show fixed Hide fixed

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatform.java Fixed Show fixed Hide fixed

engine/src/test/java/io/camunda/zeebe/engine/util/StreamPlatform.java Fixed Show fixed Hide fixed

style: apply style guide

7610bd5

Zelldon added 4 commits August 10, 2022 06:32

test: use simpler start method

34e3114

refactor: remove unused property

10a67f3

fix: set EventAppliers for now

1dfd5e1

refactor: clean up StreamPlatform

9728088

Zelldon marked this pull request as ready for review August 10, 2022 04:45

saig0 approved these changes Aug 15, 2022

View reviewed changes

Zelldon mentioned this pull request Aug 15, 2022

Remove StreamProcessorListener from engine #10035

Merged

10 tasks

zeebe-bors-camunda bot merged commit 7d89b5e into main Aug 15, 2022

zeebe-bors-camunda bot deleted the zell-new-tests branch August 15, 2022 08:05

Zelldon mentioned this pull request Aug 18, 2022

Proposal: Improve/Iterate over Engine Tests #10004

Open

This was referenced Sep 19, 2022

Test StreamProcessorReplayModeTest.shouldReplayMoreAfterResumed is broken #10027

Closed

Migrate the StreamProcessor tests to the new StreamPlatformExtension #10455

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create new StreamPlatformExtension #10036

Create new StreamPlatformExtension #10036

Zelldon commented Aug 9, 2022 •

edited

Zelldon Aug 9, 2022

Zelldon Aug 9, 2022

github-actions bot commented Aug 9, 2022 •

edited

npepinpe commented Aug 9, 2022

Zelldon commented Aug 9, 2022

Zelldon commented Aug 10, 2022

saig0 left a comment

Zelldon commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Create new StreamPlatformExtension #10036

Create new StreamPlatformExtension #10036

Conversation

Zelldon commented Aug 9, 2022 • edited

Description

Related issues

Definition of Done

Zelldon Aug 9, 2022

Choose a reason for hiding this comment

Zelldon Aug 9, 2022

Choose a reason for hiding this comment

github-actions bot commented Aug 9, 2022 • edited

Test Results

npepinpe commented Aug 9, 2022

Zelldon commented Aug 9, 2022

Zelldon commented Aug 10, 2022

saig0 left a comment

Choose a reason for hiding this comment

Zelldon commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Zelldon commented Aug 15, 2022

zeebe-bors-camunda bot commented Aug 15, 2022

Zelldon commented Aug 9, 2022 •

edited

github-actions bot commented Aug 9, 2022 •

edited