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 ambiguous naming on Integration Tests Sniffer API (next_upstream_message vs next_downstream_message) #1236

Merged
merged 1 commit into from
Oct 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 17 additions & 17 deletions roles/tests-integration/tests/common/sniffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ enum SnifferError {
/// The downstream (or client) role connects to the [`Sniffer`] `listening_address` and the
/// [`Sniffer`] connects to the `upstream` server. This way, the Sniffer can intercept messages sent
/// between the downstream and upstream roles. The downstream will send its messages to the
/// [`Sniffer`] which will save those in the `downstream_messages` aggregator and forward them to
/// the upstream role. When a response is received it is saved in `upstream_messages` and
/// forwarded to the downstream role. Both `downstream_messages` and `upstream_messages` can be
/// [`Sniffer`] which will save those in the `messages_from_downstream` aggregator and forward them to
/// the upstream role. When a response is received it is saved in `messages_from_upstream` and
/// forwarded to the downstream role. Both `messages_from_downstream` and `messages_from_upstream` can be
/// accessed as FIFO queues.
///
/// It is useful for testing purposes, as it allows to assert that the roles have sent specific
Expand All @@ -48,8 +48,8 @@ enum SnifferError {
pub struct Sniffer {
listening_address: SocketAddr,
upstream_address: SocketAddr,
downstream_messages: MessagesAggregator,
upstream_messages: MessagesAggregator,
messages_from_downstream: MessagesAggregator,
messages_from_upstream: MessagesAggregator,
}

impl Sniffer {
Expand All @@ -59,8 +59,8 @@ impl Sniffer {
Self {
listening_address,
upstream_address,
downstream_messages: MessagesAggregator::new(),
upstream_messages: MessagesAggregator::new(),
messages_from_downstream: MessagesAggregator::new(),
messages_from_upstream: MessagesAggregator::new(),
}
}

Expand All @@ -80,8 +80,8 @@ impl Sniffer {
)
.await
.expect("Failed to create upstream");
let downstream_messages = self.downstream_messages.clone();
let upstream_messages = self.upstream_messages.clone();
let downstream_messages = self.messages_from_downstream.clone();
let upstream_messages = self.messages_from_upstream.clone();
let _ = select! {
r = Self::recv_from_down_send_to_up(downstream_receiver, upstream_sender, downstream_messages) => r,
r = Self::recv_from_up_send_to_down(upstream_receiver, downstream_sender, upstream_messages) => r,
Expand All @@ -95,8 +95,8 @@ impl Sniffer {
/// This can be used to assert that the downstream sent:
/// - specific message types
/// - specific message fields
pub fn next_downstream_message(&self) -> Option<(MsgType, AnyMessage<'static>)> {
self.downstream_messages.next_message()
pub fn next_message_from_downstream(&self) -> Option<(MsgType, AnyMessage<'static>)> {
self.messages_from_downstream.next_message()
}

/// Returns the oldest message sent by upstream.
Expand All @@ -106,8 +106,8 @@ impl Sniffer {
/// This can be used to assert that the upstream sent:
/// - specific message types
/// - specific message fields
pub fn next_upstream_message(&self) -> Option<(MsgType, AnyMessage<'static>)> {
self.upstream_messages.next_message()
pub fn next_message_from_upstream(&self) -> Option<(MsgType, AnyMessage<'static>)> {
self.messages_from_upstream.next_message()
}

async fn create_downstream(
Expand Down Expand Up @@ -433,17 +433,17 @@ impl Drop for Sniffer {
std::panic::set_hook(Box::new(|_| {
println!();
}));
if !self.downstream_messages.is_empty() {
if !self.messages_from_downstream.is_empty() {
println!(
"You didn't handle all downstream messages: {:?}",
self.downstream_messages
self.messages_from_downstream
);
panic!();
}
if !self.upstream_messages.is_empty() {
if !self.messages_from_upstream.is_empty() {
println!(
"You didn't handle all upstream messages: {:?}",
self.upstream_messages
self.messages_from_upstream
);
panic!();
}
Expand Down
16 changes: 11 additions & 5 deletions roles/tests-integration/tests/pool_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ async fn success_pool_template_provider_connection() {
// macro can take any number of arguments after the message argument, but the order is
// important where a property should be followed by its value.
assert_common_message!(
&sniffer.next_downstream_message(),
&sniffer.next_message_from_downstream(),
SetupConnection,
protocol,
Protocol::TemplateDistributionProtocol,
Expand All @@ -33,8 +33,14 @@ async fn success_pool_template_provider_connection() {
max_version,
2
);
assert_common_message!(&sniffer.next_upstream_message(), SetupConnectionSuccess);
assert_tp_message!(&sniffer.next_downstream_message(), CoinbaseOutputDataSize);
assert_tp_message!(&sniffer.next_upstream_message(), NewTemplate);
assert_tp_message!(sniffer.next_upstream_message(), SetNewPrevHash);
assert_common_message!(
&sniffer.next_message_from_upstream(),
SetupConnectionSuccess
);
assert_tp_message!(
&sniffer.next_message_from_downstream(),
CoinbaseOutputDataSize
);
assert_tp_message!(&sniffer.next_message_from_upstream(), NewTemplate);
assert_tp_message!(sniffer.next_message_from_upstream(), SetNewPrevHash);
}
Loading