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

Fix stress test expectation and bug in ColdTestPublisher #3700

Merged
merged 2 commits into from Feb 13, 2024
Merged

Conversation

OlegDokuka
Copy link
Contributor

This PR fixes an early termination bug in ColdTestPublisher which happens when request from the downstream subscriber races with new values and following termination which in rare case endups in termination without propagation of the remaining values. Also this PR improves expectation for the stress test where ColdTestPulisher takes a key role

closes #3633

Signed-off-by: Oleh Dokuka <odokuka@vmware.com>
@OlegDokuka OlegDokuka added the type/bug A general bug label Jan 22, 2024
@OlegDokuka OlegDokuka added this to the 3.4.36 milestone Jan 22, 2024
@OlegDokuka OlegDokuka requested a review from a team as a code owner January 22, 2024 13:12
Signed-off-by: Oleh Dokuka <odokuka@vmware.com>
@chemicL chemicL added type/test A change in test sources type/bug A general bug area/reactor-test This belongs to the reactor-test module and removed type/bug A general bug type/test A change in test sources labels Jan 23, 2024
Comment on lines 273 to 274
// Ignore, flaky test (https://github.com/reactor/reactor-core/issues/3633)
//@JCStressTest
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// Ignore, flaky test (https://github.com/reactor/reactor-core/issues/3633)
//@JCStressTest
@JCStressTest

Copy link
Member

Choose a reason for hiding this comment

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

This and the other tests can now have the ignores removed

Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

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

I realized that the tests are effectively disabled at the moment. As this PR aims to fix the flakiness, these tests can be re-enabled.

@chemicL chemicL merged commit 08f1eb0 into 3.4.x Feb 13, 2024
7 checks passed
@reactorbot
Copy link

@chemicL this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to main 🙇

@chemicL chemicL deleted the 3.4.x-fix-3633 branch February 13, 2024 11:10
chemicL added a commit that referenced this pull request Feb 13, 2024
@chemicL
Copy link
Member

chemicL commented Feb 13, 2024

The related tests can be uncommented as a follow-up.

chemicL added a commit that referenced this pull request Feb 13, 2024
chemicL added a commit that referenced this pull request Feb 13, 2024
This reverts commit 659b45c, reversing
changes made to 96ab48c.
chemicL added a commit that referenced this pull request Feb 19, 2024
This reverts commit c316327.
@violetagg violetagg changed the title fixes stress test expectation and bug in ColdTestPublisher Fix stress test expectation and bug in ColdTestPublisher Mar 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/reactor-test This belongs to the reactor-test module type/bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants