msgr: interceptor test step enum to ease readability

I've been reading messenger code and tests to help familarise myself
with it. The test Interceptor is a great idea and gives breakpoint and
action control for simulating different connection situations. But found
the breakpoint step numbers confusing to read. These breakpoint steps do
not completely correspond to steps, as they're placed in useful places
for testing.

This patch wraps the numbers up in an enum inside the Interceptor struct
then this is used to replace the numbers to make the code more readable.

I could probably extend this to using the same Interceptor::STEP enums
in ProtocolV2.cc but as an initial patch I just wanted to touch the test
code.
Though maybe I'll do that anyway, because there's no point only making
it 1/2 readable ;)

The Interceptor::STEP enum names I just came up with based on where they
turn up in ProtocolV2, please feel free to suggest better ones.

Signed-off-by: Matthew Oliver <moliver@suse.com>

diff --git a/src/msg/Messenger.h b/src/msg/Messenger.h
index cd209130599b1cfe041f538d34f95819baa4306c..46071f0ddd47968db1feb7a2bbc2f4cca3f993e3 100644 (file)
--- a/src/msg/Messenger.h
+++ b/src/msg/Messenger.h
@@ -59,6 +59,27 @@ struct Interceptor {
     STOP
   };
 
+  enum STEP {
+    START_CLIENT_BANNER_EXCHANGE = 1,
+    START_SERVER_BANNER_EXCHANGE,
+    BANNER_EXCHANGE_BANNER_CONNECTING,
+    BANNER_EXCHANGE,
+    HANDLE_PEER_BANNER_BANNER_CONNECTING,
+    HANDLE_PEER_BANNER,
+    HANDLE_PEER_BANNER_PAYLOAD_HELLO_CONNECTING,
+    HANDLE_PEER_BANNER_PAYLOAD,
+    SEND_AUTH_REQUEST,
+    HANDLE_AUTH_REQUEST_ACCEPTING_SIGN,
+    SEND_CLIENT_IDENTITY,
+    SEND_SERVER_IDENTITY,
+    SEND_RECONNECT,
+    SEND_RECONNECT_OK,
+    READY,
+    HANDLE_MESSAGE,
+    READ_MESSAGE_COMPLETE,
+    SESSION_RETRY
+  };
+
   virtual ~Interceptor() {}
   virtual ACTION intercept(Connection *conn, uint32_t step) = 0;
 };

diff --git a/src/test/msgr/test_msgr.cc b/src/test/msgr/test_msgr.cc
index 15f5d36f69d0449cac589f5e415e83d6e26d11e4..4ddcafabeb5cb7ceacbb39dd1fd24932f550a8f1 100644 (file)
--- a/src/test/msgr/test_msgr.cc
+++ b/src/test/msgr/test_msgr.cc
@@ -360,9 +360,9 @@ TEST_P(MessengerTest, ConnectionRaceTest) {
   client_msgr->start();
 
   // pause before sending client_ident message
-  cli_interceptor->breakpoint(11);
+  cli_interceptor->breakpoint(Interceptor::STEP::SEND_CLIENT_IDENTITY);
   // pause before sending client_ident message
-  srv_interceptor->breakpoint(11);
+  srv_interceptor->breakpoint(Interceptor::STEP::SEND_CLIENT_IDENTITY);
 
   ConnectionRef c2s = client_msgr->connect_to(server_msgr->get_mytype(),
                                                                            server_msgr->get_myaddrs());
@@ -374,17 +374,17 @@ TEST_P(MessengerTest, ConnectionRaceTest) {
   MPing *m2 = new MPing();
   ASSERT_EQ(s2c->send_message(m2), 0);
 
-  cli_interceptor->wait(11, c2s.get());
-  srv_interceptor->wait(11, s2c.get());
+  cli_interceptor->wait(Interceptor::STEP::SEND_CLIENT_IDENTITY, c2s.get());
+  srv_interceptor->wait(Interceptor::STEP::SEND_CLIENT_IDENTITY, s2c.get());
 
   // at this point both connections (A->B, B->A) are paused just before sending
   // the client_ident message.
 
-  cli_interceptor->remove_bp(11);
-  srv_interceptor->remove_bp(11);
+  cli_interceptor->remove_bp(Interceptor::STEP::SEND_CLIENT_IDENTITY);
+  srv_interceptor->remove_bp(Interceptor::STEP::SEND_CLIENT_IDENTITY);
 
-  cli_interceptor->proceed(11, Interceptor::ACTION::CONTINUE);
-  srv_interceptor->proceed(11, Interceptor::ACTION::CONTINUE);
+  cli_interceptor->proceed(Interceptor::STEP::SEND_CLIENT_IDENTITY, Interceptor::ACTION::CONTINUE);
+  srv_interceptor->proceed(Interceptor::STEP::SEND_CLIENT_IDENTITY, Interceptor::ACTION::CONTINUE);
 
   {
     std::unique_lock l{cli_dispatcher.lock};
@@ -449,46 +449,46 @@ TEST_P(MessengerTest, ConnectionRaceReuseBannerTest) {
   client_msgr->start();
 
   // pause before sending client_ident message
-  srv_interceptor->breakpoint(11);
+  srv_interceptor->breakpoint(Interceptor::STEP::SEND_CLIENT_IDENTITY);
 
   ConnectionRef s2c = server_msgr->connect_to(client_msgr->get_mytype(),
                                               client_msgr->get_myaddrs());
   MPing *m1 = new MPing();
   ASSERT_EQ(s2c->send_message(m1), 0);
 
-  srv_interceptor->wait(11);
-  srv_interceptor->remove_bp(11);
+  srv_interceptor->wait(Interceptor::STEP::SEND_CLIENT_IDENTITY);
+  srv_interceptor->remove_bp(Interceptor::STEP::SEND_CLIENT_IDENTITY);
 
   // pause before sending banner
-  cli_interceptor->breakpoint(3);
+  cli_interceptor->breakpoint(Interceptor::STEP::BANNER_EXCHANGE_BANNER_CONNECTING);
 
   ConnectionRef c2s = client_msgr->connect_to(server_msgr->get_mytype(),
                                               server_msgr->get_myaddrs());
   MPing *m2 = new MPing();
   ASSERT_EQ(c2s->send_message(m2), 0);
 
-  cli_interceptor->wait(3);
-  cli_interceptor->remove_bp(3);
+  cli_interceptor->wait(Interceptor::STEP::BANNER_EXCHANGE_BANNER_CONNECTING);
+  cli_interceptor->remove_bp(Interceptor::STEP::BANNER_EXCHANGE_BANNER_CONNECTING);
 
   // second connection is in BANNER_CONNECTING, ensure it stays so
   // and send client_ident
-  srv_interceptor->breakpoint(4);
-  srv_interceptor->proceed(11, Interceptor::ACTION::CONTINUE);
+  srv_interceptor->breakpoint(Interceptor::STEP::BANNER_EXCHANGE);
+  srv_interceptor->proceed(Interceptor::STEP::SEND_CLIENT_IDENTITY, Interceptor::ACTION::CONTINUE);
 
   // handle client_ident -- triggers reuse_connection() with exproto
   // in BANNER_CONNECTING
-  cli_interceptor->breakpoint(15);
-  cli_interceptor->proceed(3, Interceptor::ACTION::CONTINUE);
+  cli_interceptor->breakpoint(Interceptor::STEP::READY);
+  cli_interceptor->proceed(Interceptor::STEP::BANNER_EXCHANGE_BANNER_CONNECTING, Interceptor::ACTION::CONTINUE);
 
-  cli_interceptor->wait(15);
-  cli_interceptor->remove_bp(15);
+  cli_interceptor->wait(Interceptor::STEP::READY);
+  cli_interceptor->remove_bp(Interceptor::STEP::READY);
 
   // first connection is in READY
-  Connection *s2c_accepter = srv_interceptor->wait(4);
-  srv_interceptor->remove_bp(4);
+  Connection *s2c_accepter = srv_interceptor->wait(Interceptor::STEP::BANNER_EXCHANGE);
+  srv_interceptor->remove_bp(Interceptor::STEP::BANNER_EXCHANGE);
 
-  srv_interceptor->proceed(4, Interceptor::ACTION::CONTINUE);
-  cli_interceptor->proceed(15, Interceptor::ACTION::CONTINUE);
+  srv_interceptor->proceed(Interceptor::STEP::BANNER_EXCHANGE, Interceptor::ACTION::CONTINUE);
+  cli_interceptor->proceed(Interceptor::STEP::READY, Interceptor::ACTION::CONTINUE);
 
   {
     std::unique_lock l{cli_dispatcher.lock};
@@ -514,9 +514,9 @@ TEST_P(MessengerTest, ConnectionRaceReuseBannerTest) {
   EXPECT_FALSE(s2c_accepter->is_connected());
 
   // established exactly once, never faulted and reconnected
-  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), 1), 1u);
-  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), 13), 0u);
-  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), 15), 1u);
+  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::START_CLIENT_BANNER_EXCHANGE), 1u);
+  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SEND_RECONNECT), 0u);
+  EXPECT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::READY), 1u);
 
   client_msgr->shutdown();
   client_msgr->wait();
@@ -554,23 +554,23 @@ TEST_P(MessengerTest, MissingServerIdenTest) {
   client_msgr->add_dispatcher_head(&cli_dispatcher);
   client_msgr->start();
 
-  // pause before sending client_ident message
-  srv_interceptor->breakpoint(12);
+  // pause before sending server_ident message
+  srv_interceptor->breakpoint(Interceptor::STEP::SEND_SERVER_IDENTITY);
 
   ConnectionRef c2s = client_msgr->connect_to(server_msgr->get_mytype(),
                                                                            server_msgr->get_myaddrs());
   MPing *m1 = new MPing();
   ASSERT_EQ(c2s->send_message(m1), 0);
 
-  Connection *c2s_accepter = srv_interceptor->wait(12);
-  srv_interceptor->remove_bp(12);
+  Connection *c2s_accepter = srv_interceptor->wait(Interceptor::STEP::SEND_SERVER_IDENTITY);
+  srv_interceptor->remove_bp(Interceptor::STEP::SEND_SERVER_IDENTITY);
 
   // We inject a message from this side of the connection to force it to be
   // in standby when we inject the failure below
   MPing *m2 = new MPing();
   ASSERT_EQ(c2s_accepter->send_message(m2), 0);
 
-  srv_interceptor->proceed(12, Interceptor::ACTION::FAIL);
+  srv_interceptor->proceed(Interceptor::STEP::SEND_SERVER_IDENTITY, Interceptor::ACTION::FAIL);
 
   {
     std::unique_lock l{srv_dispatcher.lock};
@@ -632,21 +632,21 @@ TEST_P(MessengerTest, MissingServerIdenTest2) {
   client_msgr->add_dispatcher_head(&cli_dispatcher);
   client_msgr->start();
 
-  // pause before sending client_ident message
-  srv_interceptor->breakpoint(12);
+  // pause before sending server_ident message
+  srv_interceptor->breakpoint(Interceptor::STEP::SEND_SERVER_IDENTITY);
 
   ConnectionRef c2s = client_msgr->connect_to(server_msgr->get_mytype(),
                                                                            server_msgr->get_myaddrs());
 
-  Connection *c2s_accepter = srv_interceptor->wait(12);
-  srv_interceptor->remove_bp(12);
+  Connection *c2s_accepter = srv_interceptor->wait(Interceptor::STEP::SEND_SERVER_IDENTITY);
+  srv_interceptor->remove_bp(Interceptor::STEP::SEND_SERVER_IDENTITY);
 
   // We inject a message from this side of the connection to force it to be
   // in standby when we inject the failure below
   MPing *m2 = new MPing();
   ASSERT_EQ(c2s_accepter->send_message(m2), 0);
 
-  srv_interceptor->proceed(12, Interceptor::ACTION::FAIL);
+  srv_interceptor->proceed(Interceptor::STEP::SEND_SERVER_IDENTITY, Interceptor::ACTION::FAIL);
 
   {
     std::unique_lock l{cli_dispatcher.lock};
@@ -718,38 +718,38 @@ TEST_P(MessengerTest, ReconnectTest) {
   ASSERT_EQ(1u, static_cast<Session*>(c2s->get_priv().get())->get_count());
   ASSERT_TRUE(c2s->peer_is_osd());
 
-  cli_interceptor->breakpoint(16);
+  cli_interceptor->breakpoint(Interceptor::STEP::HANDLE_MESSAGE);
   
   MPing *m2 = new MPing();
   ASSERT_EQ(c2s->send_message(m2), 0);
 
-  cli_interceptor->wait(16, c2s.get());
-  cli_interceptor->remove_bp(16);
+  cli_interceptor->wait(Interceptor::STEP::HANDLE_MESSAGE, c2s.get());
+  cli_interceptor->remove_bp(Interceptor::STEP::HANDLE_MESSAGE);
 
   // at this point client and server are connected together
 
-  srv_interceptor->breakpoint(15);
+  srv_interceptor->breakpoint(Interceptor::STEP::READY);
 
   // failing client
-  cli_interceptor->proceed(16, Interceptor::ACTION::FAIL);
+  cli_interceptor->proceed(Interceptor::STEP::HANDLE_MESSAGE, Interceptor::ACTION::FAIL);
 
   MPing *m3 = new MPing();
   ASSERT_EQ(c2s->send_message(m3), 0);
 
-  Connection *c2s_accepter = srv_interceptor->wait(15);
+  Connection *c2s_accepter = srv_interceptor->wait(Interceptor::STEP::READY);
   // the srv end of theconnection is now paused at ready
   // this means that the reconnect was successful
-  srv_interceptor->remove_bp(15);
+  srv_interceptor->remove_bp(Interceptor::STEP::READY);
 
   ASSERT_TRUE(c2s_accepter->peer_is_client());
   // c2s_accepter sent 0 reconnect messages
-  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, 13), 0u);
+  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT), 0u);
   // c2s_accepter sent 1 reconnect_ok messages
-  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, 14), 1u);
+  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT_OK), 1u);
   // c2s sent 1 reconnect messages
-  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 13), 1u);
+  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SEND_RECONNECT), 1u);
   // c2s sent 0 reconnect_ok messages
-  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 14), 0u);
+  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SEND_RECONNECT_OK), 0u);
 
   srv_interceptor->proceed(15, Interceptor::ACTION::CONTINUE);
 
@@ -814,43 +814,43 @@ TEST_P(MessengerTest, ReconnectRaceTest) {
   ASSERT_EQ(1u, static_cast<Session*>(c2s->get_priv().get())->get_count());
   ASSERT_TRUE(c2s->peer_is_osd());
 
-  cli_interceptor->breakpoint(16);
+  cli_interceptor->breakpoint(Interceptor::STEP::HANDLE_MESSAGE);
   
   MPing *m2 = new MPing();
   ASSERT_EQ(c2s->send_message(m2), 0);
 
-  cli_interceptor->wait(16, c2s.get());
-  cli_interceptor->remove_bp(16);
+  cli_interceptor->wait(Interceptor::STEP::HANDLE_MESSAGE, c2s.get());
+  cli_interceptor->remove_bp(Interceptor::STEP::HANDLE_MESSAGE);
 
   // at this point client and server are connected together
 
   // force both client and server to race on reconnect
-  cli_interceptor->breakpoint(13);
-  srv_interceptor->breakpoint(13);
+  cli_interceptor->breakpoint(Interceptor::STEP::SEND_RECONNECT);
+  srv_interceptor->breakpoint(Interceptor::STEP::SEND_RECONNECT);
 
   // failing client
   // this will cause both client and server to reconnect at the same time
-  cli_interceptor->proceed(16, Interceptor::ACTION::FAIL);
+  cli_interceptor->proceed(Interceptor::STEP::HANDLE_MESSAGE, Interceptor::ACTION::FAIL);
 
   MPing *m3 = new MPing();
   ASSERT_EQ(c2s->send_message(m3), 0);
 
-  cli_interceptor->wait(13, c2s.get());
-  srv_interceptor->wait(13);
+  cli_interceptor->wait(Interceptor::STEP::SEND_RECONNECT, c2s.get());
+  srv_interceptor->wait(Interceptor::STEP::SEND_RECONNECT);
 
-  cli_interceptor->remove_bp(13);
-  srv_interceptor->remove_bp(13);
+  cli_interceptor->remove_bp(Interceptor::STEP::SEND_RECONNECT);
+  srv_interceptor->remove_bp(Interceptor::STEP::SEND_RECONNECT);
 
   // pause on "ready"
-  srv_interceptor->breakpoint(15);
+  srv_interceptor->breakpoint(Interceptor::STEP::READY);
 
-  cli_interceptor->proceed(13, Interceptor::ACTION::CONTINUE);
-  srv_interceptor->proceed(13, Interceptor::ACTION::CONTINUE);
+  cli_interceptor->proceed(Interceptor::STEP::SEND_RECONNECT, Interceptor::ACTION::CONTINUE);
+  srv_interceptor->proceed(Interceptor::STEP::SEND_RECONNECT, Interceptor::ACTION::CONTINUE);
 
-  Connection *c2s_accepter = srv_interceptor->wait(15);
+  Connection *c2s_accepter = srv_interceptor->wait(Interceptor::STEP::READY);
 
   // the server has reconnected and is "ready"
-  srv_interceptor->remove_bp(15);
+  srv_interceptor->remove_bp(Interceptor::STEP::READY);
 
   ASSERT_TRUE(c2s_accepter->peer_is_client());
   ASSERT_TRUE(c2s->peer_is_osd());
@@ -858,24 +858,24 @@ TEST_P(MessengerTest, ReconnectRaceTest) {
   // the server should win the reconnect race
 
   // c2s_accepter sent 1 or 2 reconnect messages
-  ASSERT_LT(srv_interceptor->count_step(c2s_accepter, 13), 3u);
-  ASSERT_GT(srv_interceptor->count_step(c2s_accepter, 13), 0u);
+  ASSERT_LT(srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT), 3u);
+  ASSERT_GT(srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT), 0u);
   // c2s_accepter sent 0 reconnect_ok messages
-  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, 14), 0u);
+  ASSERT_EQ(srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT_OK), 0u);
   // c2s sent 1 reconnect messages
-  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 13), 1u);
+  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SEND_RECONNECT), 1u);
   // c2s sent 1 reconnect_ok messages
-  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 14), 1u);
+  ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SEND_RECONNECT_OK), 1u);
 
-  if (srv_interceptor->count_step(c2s_accepter, 13) == 2) {
+  if (srv_interceptor->count_step(c2s_accepter, Interceptor::STEP::SEND_RECONNECT) == 2) {
     // if the server send the reconnect message two times then
     // the client must have sent a session retry message to the server
-    ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 18), 1u);
+    ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SESSION_RETRY), 1u);
   } else {
-    ASSERT_EQ(cli_interceptor->count_step(c2s.get(), 18), 0u);
+    ASSERT_EQ(cli_interceptor->count_step(c2s.get(), Interceptor::STEP::SESSION_RETRY), 0u);
   }
 
-  srv_interceptor->proceed(15, Interceptor::ACTION::CONTINUE);
+  srv_interceptor->proceed(Interceptor::STEP::READY, Interceptor::ACTION::CONTINUE);
 
   {
     std::unique_lock l{cli_dispatcher.lock};