From 61a57336920e0f64361a4c48215b12969c3a1eff Mon Sep 17 00:00:00 2001 From: SmallJoker Date: Thu, 29 Sep 2022 22:16:29 +0200 Subject: [PATCH] Unittest: Add inventory callback tests --- builtin/game/tests/test_moveaction.lua | 84 ++++++++++++++++++ src/script/cpp_api/s_base.h | 1 + src/server.h | 3 + src/server/serverinventorymgr.h | 7 +- src/unittest/mock_inventorymanager.h | 17 ++-- src/unittest/mock_server.h | 2 + src/unittest/test_moveaction.cpp | 114 ++++++++++++++++++------- 7 files changed, 187 insertions(+), 41 deletions(-) create mode 100644 builtin/game/tests/test_moveaction.lua diff --git a/builtin/game/tests/test_moveaction.lua b/builtin/game/tests/test_moveaction.lua new file mode 100644 index 000000000..476579348 --- /dev/null +++ b/builtin/game/tests/test_moveaction.lua @@ -0,0 +1,84 @@ +-- Table to keep track of callback executions +-- [i + 0] = count of expected patterns of index (i + 1) +-- [i + 1] = pattern to check +local PATTERN_NORMAL = { 4, "allow_%w", 2, "on_take", 1, "on_put", 1 } +local PATTERN_SWAP = { 8, "allow_%w", 4, "on_take", 2, "on_put", 2 } +local exec_listing = {} -- List of logged callbacks (e.g. "on_take", "allow_put") + +-- Checks whether the logged callbacks equal the expected pattern +core.__helper_check_callbacks = function(expect_swap) + local exec_pattern = expect_swap and PATTERN_SWAP or PATTERN_NORMAL + local ok = #exec_listing == exec_pattern[1] + if ok then + local list_index = 1 + for i = 2, #exec_pattern, 2 do + for n = 1, exec_pattern[i + 1] do + -- Match the list for "n" occurrences of the wanted callback name pattern + ok = exec_listing[list_index]:find(exec_pattern[i]) + list_index = list_index + 1 + if not ok then break end + end + if not ok then break end + end + end + + if not ok then + print("Execution order mismatch!") + print("Expected patterns: ", dump(exec_pattern)) + print("Got list: ", dump(exec_listing)) + end + exec_listing = {} + return ok +end + +-- Uncomment the other line for easier callback debugging +local log = function(...) end +--local log = print + +minetest.register_allow_player_inventory_action(function(_, action, inv, info) + log("\tallow " .. action, info.count or info.stack:to_string()) + + if action == "move" then + -- testMoveFillStack + return info.count + end + + if action == "take" or action == "put" then + assert(not info.stack:is_empty(), "Stack empty in: " .. action) + + -- testMoveUnallowed + -- testSwapFromUnallowed + -- testSwapToUnallowed + if info.stack:get_name() == "default:takeput_deny" then + return 0 + end + + -- testMovePartial + if info.stack:get_name() == "default:takeput_max_5" then + return 5 + end + + -- testCallbacks + if info.stack:get_name():find("default:takeput_cb_%d") then + -- Log callback as executed + table.insert(exec_listing, "allow_" .. action) + return -- Unlimited + end + end + + return -- Unlimited +end) + +minetest.register_on_player_inventory_action(function(_, action, inv, info) + log("\ton " .. action, info.count or info.stack:to_string()) + + if action == "take" or action == "put" then + assert(not info.stack:is_empty(), action) + + if info.stack:get_name():find("default:takeput_cb_%d") then + -- Log callback as executed + table.insert(exec_listing, "on_" .. action) + return + end + end +end) diff --git a/src/script/cpp_api/s_base.h b/src/script/cpp_api/s_base.h index cacc79a83..ac74c2e68 100644 --- a/src/script/cpp_api/s_base.h +++ b/src/script/cpp_api/s_base.h @@ -146,6 +146,7 @@ protected: friend class ModApiBase; friend class ModApiEnv; friend class LuaVoxelManip; + friend class TestMoveAction; // needs getStack() /* Subtle edge case with coroutines: If for whatever reason you have a diff --git a/src/server.h b/src/server.h index 9f9f08ac7..bd1547238 100644 --- a/src/server.h +++ b/src/server.h @@ -434,7 +434,10 @@ public: private: friend class EmergeThread; friend class RemoteClient; + + // unittest classes friend class TestServerShutdownState; + friend class TestMoveAction; struct ShutdownState { friend class TestServerShutdownState; diff --git a/src/server/serverinventorymgr.h b/src/server/serverinventorymgr.h index b54f6edd6..9b654c507 100644 --- a/src/server/serverinventorymgr.h +++ b/src/server/serverinventorymgr.h @@ -40,8 +40,9 @@ public: m_env = env; } - Inventory *getInventory(const InventoryLocation &loc); - void setInventoryModified(const InventoryLocation &loc); + // virtual: Overwritten by MockInventoryManager for the unittests + virtual Inventory *getInventory(const InventoryLocation &loc); + virtual void setInventoryModified(const InventoryLocation &loc); // Creates or resets inventory Inventory *createDetachedInventory(const std::string &name, IItemDefManager *idef, @@ -52,7 +53,7 @@ public: void sendDetachedInventories(const std::string &peer_name, bool incremental, std::function apply_cb); -private: +protected: struct DetachedInventory { std::unique_ptr inventory; diff --git a/src/unittest/mock_inventorymanager.h b/src/unittest/mock_inventorymanager.h index dc75490de..bdf75296a 100644 --- a/src/unittest/mock_inventorymanager.h +++ b/src/unittest/mock_inventorymanager.h @@ -17,11 +17,15 @@ with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ -#include -#include -#include +#pragma once -class MockInventoryManager : public InventoryManager +#include "gamedef.h" +#include "inventory.h" +#include "server/serverinventorymgr.h" + +class ServerEnvironment; + +class MockInventoryManager : public ServerInventoryManager { public: MockInventoryManager(IGameDef *gamedef) : @@ -29,14 +33,17 @@ public: p2(gamedef->getItemDefManager()) {}; - virtual Inventory* getInventory(const InventoryLocation &loc){ + Inventory *getInventory(const InventoryLocation &loc) override + { if (loc.type == InventoryLocation::PLAYER && loc.name == "p1") return &p1; if (loc.type == InventoryLocation::PLAYER && loc.name == "p2") return &p2; return nullptr; } + void setInventoryModified(const InventoryLocation &loc) override {} Inventory p1; Inventory p2; + }; diff --git a/src/unittest/mock_server.h b/src/unittest/mock_server.h index 4c0e5fe16..062303a93 100644 --- a/src/unittest/mock_server.h +++ b/src/unittest/mock_server.h @@ -17,6 +17,8 @@ with this program; if not, write to the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +#pragma once + #include class MockServer : public Server diff --git a/src/unittest/test_moveaction.cpp b/src/unittest/test_moveaction.cpp index 62a5237a3..af0a88df8 100644 --- a/src/unittest/test_moveaction.cpp +++ b/src/unittest/test_moveaction.cpp @@ -24,6 +24,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "mock_serveractiveobject.h" #include "scripting_server.h" +#include "server/mods.h" class TestMoveAction : public TestBase @@ -39,43 +40,31 @@ public: void testMoveSomewhere(ServerActiveObject *obj, IGameDef *gamedef); void testMoveUnallowed(ServerActiveObject *obj, IGameDef *gamedef); void testMovePartial(ServerActiveObject *obj, IGameDef *gamedef); + void testSwap(ServerActiveObject *obj, IGameDef *gamedef); void testSwapFromUnallowed(ServerActiveObject *obj, IGameDef *gamedef); void testSwapToUnallowed(ServerActiveObject *obj, IGameDef *gamedef); + + void testCallbacks(ServerActiveObject *obj, Server *server); + void testCallbacksSwap(ServerActiveObject *obj, Server *server); }; static TestMoveAction g_test_instance; -const static char *helper_lua_src = R"( -core.register_allow_player_inventory_action(function(player, action, inventory, info) - if action == "move" then - return info.count - end - - if info.stack:get_name() == "default:water" then - return 0 - end - if info.stack:get_name() == "default:lava" then - return 5 - end - - return info.stack:get_count() -end) -)"; - void TestMoveAction::runTests(IGameDef *gamedef) { MockServer server(getTestTempDirectory()); - const auto helper_lua = getTestTempFile(); - std::ofstream ofs(helper_lua, std::ios::out | std::ios::binary); - ofs << helper_lua_src; - ofs.close(); - ServerScripting server_scripting(&server); try { - server_scripting.loadMod(Server::getBuiltinLuaPath() + DIR_DELIM "init.lua", BUILTIN_MOD_NAME); - server_scripting.loadMod(helper_lua, BUILTIN_MOD_NAME); + // FIXME: When removing the line below, the unittest does NOT crash + // (but it should) when running all unittests in order or registration. + // Some Lua API functions used in builtin require the Mgr to be present. + server.m_modmgr = std::make_unique(server.m_path_world); + + std::string builtin = Server::getBuiltinLuaPath() + DIR_DELIM; + server_scripting.loadMod(builtin + "init.lua", BUILTIN_MOD_NAME); + server_scripting.loadMod(builtin + "game" DIR_DELIM "tests" DIR_DELIM "test_moveaction.lua", BUILTIN_MOD_NAME); } catch (ModError &e) { // Print backtrace in case of syntax errors rawstream << e.what() << std::endl; @@ -83,6 +72,8 @@ void TestMoveAction::runTests(IGameDef *gamedef) return; } + server.m_script = &server_scripting; + MetricsBackend mb; ServerEnvironment server_env(nullptr, &server_scripting, &server, "", &mb); MockServerActiveObject obj(&server_env); @@ -92,9 +83,15 @@ void TestMoveAction::runTests(IGameDef *gamedef) TEST(testMoveSomewhere, &obj, gamedef); TEST(testMoveUnallowed, &obj, gamedef); TEST(testMovePartial, &obj, gamedef); + TEST(testSwap, &obj, gamedef); TEST(testSwapFromUnallowed, &obj, gamedef); TEST(testSwapToUnallowed, &obj, gamedef); + + TEST(testCallbacks, &obj, &server); + TEST(testCallbacksSwap, &obj, &server); + + server.m_script = nullptr; // Do not free stack memory } static ItemStack parse_itemstack(const char *s) @@ -165,12 +162,12 @@ void TestMoveAction::testMoveUnallowed(ServerActiveObject *obj, IGameDef *gamede { MockInventoryManager inv(gamedef); - inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:water 50")); + inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_deny 50")); inv.p2.addList("main", 10); apply_action("Move 20 player:p1 main 0 player:p2 main 0", &inv, obj, gamedef); - UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:water 50"); + UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:takeput_deny 50"); UASSERT(inv.p2.getList("main")->getItem(0).empty()) } @@ -178,13 +175,14 @@ void TestMoveAction::testMovePartial(ServerActiveObject *obj, IGameDef *gamedef) { MockInventoryManager inv(gamedef); - inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:lava 50")); + inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_max_5 50")); inv.p2.addList("main", 10); + // Lua: limited to 5 per transaction apply_action("Move 20 player:p1 main 0 player:p2 main 0", &inv, obj, gamedef); - UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:lava 45"); - UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:lava 5"); + UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:takeput_max_5 45"); + UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:takeput_max_5 5"); } void TestMoveAction::testSwap(ServerActiveObject *obj, IGameDef *gamedef) @@ -204,12 +202,12 @@ void TestMoveAction::testSwapFromUnallowed(ServerActiveObject *obj, IGameDef *ga { MockInventoryManager inv(gamedef); - inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:water 50")); + inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_deny 50")); inv.p2.addList("main", 10)->addItem(0, parse_itemstack("default:brick 60")); apply_action("Move 50 player:p1 main 0 player:p2 main 0", &inv, obj, gamedef); - UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:water 50"); + UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:takeput_deny 50"); UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:brick 60"); } @@ -218,10 +216,60 @@ void TestMoveAction::testSwapToUnallowed(ServerActiveObject *obj, IGameDef *game MockInventoryManager inv(gamedef); inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:stone 50")); - inv.p2.addList("main", 10)->addItem(0, parse_itemstack("default:water 60")); + inv.p2.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_deny 60")); apply_action("Move 50 player:p1 main 0 player:p2 main 0", &inv, obj, gamedef); UASSERT(inv.p1.getList("main")->getItem(0).getItemString() == "default:stone 50"); - UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:water 60"); + UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:takeput_deny 60"); +} + +static bool check_function(lua_State *L, bool expect_swap) +{ + bool ok = false; + int error_handler = PUSH_ERROR_HANDLER(L); + + lua_getglobal(L, "core"); + lua_getfield(L, -1, "__helper_check_callbacks"); + lua_pushboolean(L, expect_swap); + int result = lua_pcall(L, 1, 1, error_handler); + if (result == 0) + ok = lua_toboolean(L, -1); + else + errorstream << lua_tostring(L, -1) << std::endl; // Error msg + + lua_settop(L, 0); + return ok; +} + +void TestMoveAction::testCallbacks(ServerActiveObject *obj, Server *server) +{ + server->m_inventory_mgr = std::make_unique(server); + MockInventoryManager &inv = *(MockInventoryManager *)server->getInventoryMgr(); + + inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_cb_1 10")); + inv.p2.addList("main", 10); + + apply_action("Move 10 player:p1 main 0 player:p2 main 1", &inv, obj, server); + + // Expecting no swap. 4 callback executions in total. See Lua file for details. + UASSERT(check_function(server->getScriptIface()->getStack(), false)); + + server->m_inventory_mgr.reset(); +} + +void TestMoveAction::testCallbacksSwap(ServerActiveObject *obj, Server *server) +{ + server->m_inventory_mgr = std::make_unique(server); + MockInventoryManager &inv = *(MockInventoryManager *)server->getInventoryMgr(); + + inv.p1.addList("main", 10)->addItem(0, parse_itemstack("default:takeput_cb_2 50")); + inv.p2.addList("main", 10)->addItem(1, parse_itemstack("default:takeput_cb_1 10")); + + apply_action("Move 10 player:p1 main 0 player:p2 main 1", &inv, obj, server); + + // Expecting swap. 8 callback executions in total. See Lua file for details. + UASSERT(check_function(server->getScriptIface()->getStack(), true)); + + server->m_inventory_mgr.reset(); }