Skip to content

Commit

Permalink
fix tests
Browse files Browse the repository at this point in the history
  • Loading branch information
empiredan committed Nov 21, 2024
1 parent bfaf2f6 commit 2a8df3c
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 29 deletions.
27 changes: 16 additions & 11 deletions src/meta/server_state.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ void server_state::query_configuration_by_index(const query_cfg_request &request
/*out*/ query_cfg_response &response)
{
zauto_read_lock l(_lock);
auto iter = _exist_apps.find(request.app_name.c_str());
auto iter = _exist_apps.find(request.app_name);
if (iter == _exist_apps.end()) {
response.err = ERR_OBJECT_NOT_FOUND;
return;
Expand Down Expand Up @@ -3024,22 +3024,26 @@ void server_state::set_app_envs(const app_env_rpc &env_rpc)
app_info ainfo;
std::string app_path;
{
zauto_read_lock l(_lock);

auto app = get_app(app_name);
FAIL_POINT_INJECT_NOT_RETURN_F("set_app_envs_failed", [app_name, this](std::string_view s) {
zauto_write_lock l(_lock);

FAIL_POINT_INJECT_NOT_RETURN_F("set_app_envs_failed", [&app](std::string_view s) {
if (s == "not_found") {
app.reset();
CHECK_EQ(_exist_apps.erase(app_name), 1);
return;
}

if (s == "dropping") {
app->status = app_status::AS_DROPPING;
const auto &iter = _exist_apps.find(app_name);
CHECK_TRUE(iter != _exist_apps.end());

iter->second->status = app_status::AS_DROPPING;
return;
}
});

zauto_read_lock l(_lock);

const auto &app = get_app(app_name);
if (!app) {
LOG_WARNING("set app envs failed since app_name({}) cannot be found", app_name);
env_rpc.response().err = ERR_APP_NOT_EXIST;
Expand Down Expand Up @@ -3068,15 +3072,16 @@ void server_state::set_app_envs(const app_env_rpc &env_rpc)
CHECK_EQ_MSG(ec, ERR_OK, "update app({}) info to remote storage failed", app_name);

zauto_write_lock l(_lock);
std::shared_ptr<app_state> app = get_app(app_name);

FAIL_POINT_INJECT_NOT_RETURN_F("set_app_envs_failed", [&app](std::string_view s) {
if (s == "dropped_after_update_remote_storage") {
app.reset();
FAIL_POINT_INJECT_NOT_RETURN_F("set_app_envs_failed", [app_name, this](std::string_view s) {
if (s == "dropped_after") {
CHECK_EQ(_exist_apps.erase(app_name), 1);
return;
}
});

std::shared_ptr<app_state> app = get_app(app_name);

// The table might be removed just before the callback function is invoked, thus we must
// check if this table still exists.
//
Expand Down
46 changes: 28 additions & 18 deletions src/meta/test/server_state_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class server_state_test
dsn::message_ptr binary_req(dsn::message_ex::create_request(RPC_CM_UPDATE_APP_ENV));
dsn::marshall(binary_req, request);
dsn::message_ex *recv_msg = create_corresponding_receive(binary_req);
return app_env_rpc(recv_msg); // don't need reply
return app_env_rpc(recv_msg); // Don't need to reply.
}

std::unique_ptr<meta_service> _ms;
Expand All @@ -220,7 +220,7 @@ void meta_service_test_app::app_envs_basic_test()
test.load_apps({"test_app1",
"test_set_app_envs_not_found",
"test_set_app_envs_dropping",
"test_set_app_envs_dropped_after_update_remote_storage"});
"test_set_app_envs_dropped_after"});

#define TEST_SET_APP_ENVS_FAILED(action, err_code) \
std::cout << "test server_state::set_app_envs(" #action ")..." << std::endl; \
Expand All @@ -244,7 +244,7 @@ void meta_service_test_app::app_envs_basic_test()

TEST_SET_APP_ENVS_FAILED(dropping, ERR_BUSY_DROPPING);

TEST_SET_APP_ENVS_FAILED(dropped_after_update_remote_storage, ERR_APP_DROPPED);
TEST_SET_APP_ENVS_FAILED(dropped_after, ERR_APP_DROPPED);

#undef TEST_SET_APP_ENVS_FAILED

Expand All @@ -264,6 +264,8 @@ void meta_service_test_app::app_envs_basic_test()

for (size_t idx = 0; idx < keys.size(); ++idx) {
const auto &key = keys[idx];

// Every env should be inserted.
ASSERT_EQ(1, app->envs.count(key));
ASSERT_EQ(values[idx], app->envs.at(key));
}
Expand All @@ -284,18 +286,21 @@ void meta_service_test_app::app_envs_basic_test()

for (size_t idx = 0; idx < keys.size(); ++idx) {
const std::string &key = keys[idx];
if (del_keys_set.count(key) >= 1) {
if (del_keys_set.count(key) > 0) {
// The env in `del_keys_set` should be deleted.
ASSERT_EQ(0, app->envs.count(key));
} else {
ASSERT_EQ(1, app->envs.count(key));
ASSERT_EQ(values[idx], app->envs.at(key));
continue;
}

// The env should still exist if it is not in `del_keys_set`.
ASSERT_EQ(1, app->envs.count(key));
ASSERT_EQ(values[idx], app->envs.at(key));
}
}

std::cout << "test server_state::clear_app_envs()..." << std::endl;
{
// test specify prefix
// Test specifying prefix.
{
configuration_update_app_env_request request;
request.__set_app_name("test_app1");
Expand All @@ -310,21 +315,25 @@ void meta_service_test_app::app_envs_basic_test()

for (size_t idx = 0; idx < keys.size(); ++idx) {
const std::string &key = keys[idx];
if (del_keys_set.count(key) <= 0) {
if (acquire_prefix(key) == clear_prefix) {
ASSERT_EQ(0, app->envs.count(key));
} else {
ASSERT_EQ(1, app->envs.count(key));
ASSERT_EQ(values[idx], app->envs.at(key));
}
} else {
// key already delete
if (del_keys_set.count(key) > 0) {
// The env should have been deleted during test for `del_app_envs`.
ASSERT_EQ(0, app->envs.count(key));
continue;
}

if (acquire_prefix(key) == clear_prefix) {
// The env with specified prefix should be deleted.
ASSERT_EQ(0, app->envs.count(key));
continue;
}

// Otherwise, the env should still exist.
ASSERT_EQ(1, app->envs.count(key));
ASSERT_EQ(values[idx], app->envs.at(key));
}
}

// test clear all
// Test clearing all.
{
configuration_update_app_env_request request;
request.__set_app_name("test_app1");
Expand All @@ -337,6 +346,7 @@ void meta_service_test_app::app_envs_basic_test()
const auto &app = test.get_app("test_app1");
ASSERT_TRUE(app);

// All envs should be cleared.
ASSERT_TRUE(app->envs.empty());
}
}
Expand Down

0 comments on commit 2a8df3c

Please sign in to comment.