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
[#643] Tests for AMQP Adapter #660
[#643] Tests for AMQP Adapter #660
Conversation
* Rather, the setup manually instantiates the adapter and gives it a user defined ProtonServer. | ||
*/ | ||
@RunWith(VertxUnitRunner.class) | ||
public class VertxBasedAmqpProtocolAdapterWithoutClientTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be the unit tests, right? If so, can we please simply name this class VertxBasedAmqpProtocolAdapterTest
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it is. i rename it to distinguish it from the other test. will change it now
* as a Verticle. A ProtonClient, which simulates a device, is used to communicate with the adapter. | ||
*/ | ||
@RunWith(VertxUnitRunner.class) | ||
public class VertxBasedAmqpProtocolAdapterWithClientTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These seem to be functional tests running against a deployed version of the adapter. I would rather move these tests to the tests
module and run the tests against the Docker containers we start there. It would probably also make more sense to put it into a separate PR then...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i see. thanks!
@sophokles73: PR updated! |
return message; | ||
} | ||
|
||
private MessageSender givenAmessageSenderForAnyTenant() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should better be called givenATelemetrySenderForAnyTenant in order to point out that the sender indeed is a sender for telemetry messages.
public void uploadTelemetryMessageWithSettledDeliverySemantics() { | ||
// GIVEN an AMQP adapter with a configured server | ||
final VertxBasedAmqpProtocolAdapter adapter = givenAnAmqpAdapter(); | ||
final MessageSender sender = givenAmessageSenderForAnyTenant(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better call this variable telemetrySender
* AT_LEAST_ONCE delivery semantics. | ||
*/ | ||
@Test | ||
public void uploadTelemetryMessageWithNotSettledDeliverySemantics() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UnsettledDeliverySemantics
public void uploadTelemetryMessageWithNotSettledDeliverySemantics() { | ||
// GIVEN an adapter configured to use a user-define server. | ||
final VertxBasedAmqpProtocolAdapter adapter = givenAnAmqpAdapter(); | ||
final MessageSender sender = givenAmessageSenderForAnyTenant(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
telemetrySender
f51669d
to
dbe49c3
Compare
@sophokles73: thanks, just updated the PR! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…nticated devices) Signed-off-by: Alfusainey Jallow <alf.jallow@gmail.com>
dbe49c3
to
f12f6c2
Compare
thanks. i've squashed the commits into one |
@sophokles73: This PR test the functionality of the AMQP adapter (unauthenticated devices)