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

Add missing java api for StreamTestKit #1186

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Add missing java api for StreamTestKit #1186

merged 3 commits into from
Mar 20, 2024

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Mar 10, 2024

Motivation:
Add more java native api to StreamTestKit.

refs:#970

was: #1049

continue the work of @naosense

@He-Pin He-Pin force-pushed the streamKit branch 2 times, most recently from 812f1ea to 650ef9d Compare March 16, 2024 10:18
@He-Pin He-Pin marked this pull request as ready for review March 16, 2024 10:19
@@ -136,15 +136,15 @@ class StreamTestKitSpec extends PekkoSpec {
// It also needs to be dilated since the testkit will dilate the timeout
// accordingly to `-Dpekko.test.timefactor` value.
val initialDelay = (timeout * 2).dilated
val pf: PartialFunction[Any, Unit] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

these changes seem quite small compared to the StreamTestKit changes

is it possible to test all the new functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I'm a little busy at $Work right now, testing all these new methods seems like a lot of work, and as we can see, the changes are mostly small, just forwarding/converting.

@@ -1242,8 +1242,7 @@ public void mustBeAbleToUseMerge3() {
.mergeAll(Arrays.asList(sourceB, sourceC), false)
.runWith(TestSink.probe(system), system);
sub.expectSubscription().request(9);
sub.expectNextUnorderedN(Util.immutableSeq(Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9)))
.expectComplete();
sub.expectNextUnorderedN(Arrays.asList(1, 2, 3, 4, 5, 6, 7, 8, 9)).expectComplete();
Copy link
Member Author

Choose a reason for hiding this comment

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

Then, no Util.immutableSeq is needed

Copy link
Contributor

@mdedetrich mdedetrich left a comment

Choose a reason for hiding this comment

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

This is getting a lgtm. Adding tests for these functions are not necessary since they are basic wrappers over existing functions which are already being tested and this has also historically been the case for Akka as well

@He-Pin
Copy link
Member Author

He-Pin commented Mar 20, 2024

@pjfanning @Roiocam WDYT, should it be included in m1 release?

@pjfanning
Copy link
Contributor

Lgtm

Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

Lgtm

@He-Pin
Copy link
Member Author

He-Pin commented Mar 20, 2024

Thanks

@He-Pin He-Pin merged commit 55477ac into apache:main Mar 20, 2024
18 checks passed
@He-Pin He-Pin deleted the streamKit branch March 20, 2024 15:31
@He-Pin He-Pin modified the milestones: 1.1.0-M2, 1.1.0-M1 Mar 21, 2024
@pjfanning pjfanning added the late-release-note late breaking changes that will require release notes changes label Mar 24, 2024
@pjfanning pjfanning removed the late-release-note late breaking changes that will require release notes changes label May 7, 2024
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.

4 participants