Skip to content

Commit

Permalink
Allow tcp_client::init to fail correctly
Browse files Browse the repository at this point in the history
sentinel_2pc::controller::init contains a bug that makes it impossible to
trigger the error "Failed to start coordinator client" (see GitHub Issue mit-dci#131).
This commit fixes the bug using the method described in Issue mit-dci#131.  It
adds a new unit test to test triggering the error, and also modifies a
different unit test to account for behavior that's expected after the bug fix.

Signed-off-by: Michael L. Szulczewski <[email protected]>
  • Loading branch information
mszulcz-mitre committed Aug 16, 2022
1 parent 0a5635a commit e2b351d
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/util/rpc/tcp_client.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ namespace cbdc::rpc {
/// starts the response handler thread.
/// \return true.
[[nodiscard]] auto init() -> bool {
if(!m_net.cluster_connect(m_server_endpoints, false)) {
if(!m_net.cluster_connect(m_server_endpoints, true)) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/rpc/tcp_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ TEST(tcp_rpc_test, send_fail_test) {
auto client = cbdc::rpc::tcp_client<request, response>(
{{cbdc::network::localhost, 55555},
{cbdc::network::localhost, 55556}});
ASSERT_TRUE(client.init());
ASSERT_FALSE(client.init());

auto req = request{0};
auto resp = client.call(req);
Expand Down
57 changes: 57 additions & 0 deletions tests/unit/sentinel_2pc/controller_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,60 @@ TEST_F(sentinel_2pc_test, tx_validation_test) {
});
ASSERT_TRUE(res);
}

TEST_F(sentinel_2pc_test, bad_coordinator_endpoint) {
// Replace the valid coordinator endpoint defined in the fixture
// with an invalid endpoint.
m_opts.m_coordinator_endpoints.clear();
const auto bad_coordinator_ep
= std::make_pair("abcdefg", m_coordinator_port);
m_opts.m_coordinator_endpoints.resize(1);
m_opts.m_coordinator_endpoints[0].push_back(bad_coordinator_ep);

// Initialize a new controller with the invalid coordinator endpoint.
auto ctl = std::make_unique<cbdc::sentinel_2pc::controller>(0,
m_opts,
m_logger);

// Check that the controller with the invalid coordinator endpoint
// fails to initialize correctly.
ASSERT_FALSE(ctl->init());
}

TEST_F(sentinel_2pc_test, bad_sentinel_client_endpoint) {
// Test that a sentinel client fails to initialize
// when given a bad endpoint.
constexpr auto bad_endpoint = std::make_pair("abcdefg", m_sentinel_port);
const std::vector<cbdc::network::endpoint_t> bad_endpoints{bad_endpoint};
auto client = cbdc::sentinel::rpc::client(bad_endpoints, m_logger);
ASSERT_FALSE(client.init());

// Test that the controller fails to initialize when given a bad endpoint
// for a sentinel client.
m_opts.m_sentinel_endpoints.emplace_back(bad_endpoint);
auto ctl = std::make_unique<cbdc::sentinel_2pc::controller>(0,
m_opts,
m_logger);
ASSERT_FALSE(ctl->init());
}

TEST_F(sentinel_2pc_test, bad_rpc_server_endpoint) {
// The sentinel endpoint defined below (which corresponds to sentinel_id
// also defined below) is used by the sentinel_2pc controller to initialize
// an rpc server. Replacing the valid endpoint defined in the fixture with
// an invalid endpoint should cause the rpc server to fail to initialize.
m_opts.m_sentinel_endpoints.clear();
constexpr auto bad_endpoint = std::make_pair("abcdefg", m_sentinel_port);
m_opts.m_sentinel_endpoints.resize(1);
m_opts.m_sentinel_endpoints.emplace_back(bad_endpoint);

// Initialize a new controller with the invalid endpoint for the server.
constexpr uint32_t sentinel_id = 0;
const auto ctl
= std::make_unique<cbdc::sentinel_2pc::controller>(sentinel_id,
m_opts,
m_logger);

// Check that the controller with the invalid endpoint fails to initialize.
ASSERT_FALSE(ctl->init());
}

0 comments on commit e2b351d

Please sign in to comment.