Skip to content

BAEL-5969 - Parallel Flux vs Flux in Project Reactor #18552

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

LordMaduz
Copy link
Contributor

  1. Implementation

Copy link
Collaborator

@theangrydev theangrydev left a comment

Choose a reason for hiding this comment

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

Make sure to apply the Baeldung formatting style - do you have the IDE profile imported already?

import java.util.concurrent.atomic.AtomicLong;

@Slf4j
public class ParallelFluxUnitTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is different about the two tests here, what are you trying to demonstrate, the same thing in each test or is there a different concept being communicated? I didn't understand the distinction

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First Test is to show Case General case of ParallelFlux where for computationally expensive tasks time taken is less than Flux Version. Also each element is processed once and emitted once.

Second Test is to show that the point that order of processing is not always same like that of Flux. We can't; guarantee the order of processing by default in ParallelFlux.

@Disabled("Manual test - takes time due to heavy computation")
@Test
public void givenFibonacciIndices_whenComputingWithFlux_thenCorrectResults() {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove newlines at the start of methods like this

}

@RepeatedTest(5)
public void givenListOfIds_whenComputingWithParallelFlux_OrderChanges() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a naming convention given_when_then

Suggested change
public void givenListOfIds_whenComputingWithParallelFlux_OrderChanges() {
public void givenListOfIds_whenComputingWithParallelFlux_thenOrderChanges() {

import java.util.concurrent.CopyOnWriteArrayList;

@Slf4j
public class ParallelFluxUnitTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public class ParallelFluxUnitTest {
public class ParallelFluxManualTest {

@Slf4j
public class ParallelFluxUnitTest {

@Disabled("Manual test - takes time due to heavy computation")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Disabled("Manual test - takes time due to heavy computation")

@Slf4j
public class FluxUnitTest {

@Disabled("Manual test - takes time due to heavy computation")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Disabled("Manual test - takes time due to heavy computation")

Don't do this - we have a naming convention instead - FluxManualTest - that will avoid it being run in CI

import java.time.Duration;

@Slf4j
public class FluxUnitTest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public class FluxUnitTest {
public class FluxManualTest {

.expectNextCount(3)
.verifyComplete();

System.out.println("ParallelFlux emitted order: " + emitted);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We have a convention to use loggers not System.out

Copy link
Collaborator

@theangrydev theangrydev left a comment

Choose a reason for hiding this comment

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

This turned out really good :)

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.

2 participants