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 new stream typed queue #282

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hanzo2001
Copy link

This contribution derives from issue (feature request) #281

This PR is not ready to be merged. There are many things to look out for and the code style is not compliant.

Also, I cannot seem to pass a test for plugin com.github.fridujo:rabbitmq-mock-alpakka-amqp-integration

[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 11.138 s <<< FAILURE! - in com.github.fridujo.rabbitmq.mock.integration.alpakka.AmqpConnectorsTest
[ERROR] publishAndConsumeRpcWithoutAutoAck  Time elapsed: 10.053 s  <<< FAILURE!
java.lang.AssertionError: assertion failed: timeout (4999314094 nanoseconds) during expectMsgClass waiting for interface akka.stream.testkit.TestSubscriber$SubscriberEvent
        at com.github.fridujo.rabbitmq.mock.integration.alpakka.AmqpConnectorsTest.publishAndConsumeRpcWithoutAutoAck(AmqpConnectorsTest.java:121)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   AmqpConnectorsTest.publishAndConsumeRpcWithoutAutoAck:121 assertion failed: timeout (4999314094 nanoseconds) during expectMsgClass waiting for interface akka.stream.testkit.TestSubscriber$SubscriberEvent
[INFO] 
[ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0

@ledoyen
Copy link
Contributor

ledoyen commented Apr 1, 2022

Hi, thanks for the interest you take on the project.

Please note that I have very few time to support it and my answers could be very (very) long to arrive.

A quick feedback on your early proposal though:

  • I think that most of the modifications could be done in MockQueue to avoid splitting logic
    • In order to do that, the concept of "storage behavior" needs to be extracted in a new abstraction
    • To add stream support then add a new implementation of this abstraction
    • This should simplify the change and the need for decorators
  • It could be nice for this feature to introduce something that I wanted to do a long time ago: play integration tests twice with a MockConnectionfactory and a real ConnectionFactory using testcontainers to verify that the same behavior is observed

Interesting feature though, surely driven from Kafka success 😄

@ledoyen ledoyen added the enhancement New feature or request label Apr 1, 2022
@hanzo2001
Copy link
Author

I will try to improve my PR but what you are suggesting may take a while. I like refactorings but I usually break a lot more than I fix and sometimes the business routines become far too abstract for the next round of improvements.

I'll try to improve this on the next weekend

@ledoyen ledoyen force-pushed the master branch 2 times, most recently from aaf8dde to a763be2 Compare October 18, 2022 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants