Unittest: Add inventory callback tests

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)

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

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;

src/server/serverinventorymgr.h

@@ -40,8 +40,9 @@ public:
 		m_env = env;
 	}
 
-	Inventory *getInventory(const InventoryLocation &loc);
+	// virtual: Overwritten by MockInventoryManager for the unittests
-	void setInventoryModified(const InventoryLocation &loc);
+	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<void(const std::string &, Inventory *)> apply_cb);
 
-private:
+protected:
 	struct DetachedInventory
 	{
 		std::unique_ptr<Inventory> inventory;

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 <gamedef.h>
+#pragma once
-#include <inventory.h>
-#include <inventorymanager.h>
 
-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;
+
 };

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 <server.h>
 
 class MockServer : public Server

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);
+		// FIXME: When removing the line below, the unittest does NOT crash
-		server_scripting.loadMod(helper_lua, BUILTIN_MOD_NAME);
+		// (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<ServerModManager>(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.p1.getList("main")->getItem(0).getItemString() == "default:takeput_max_5 45");
-	UASSERT(inv.p2.getList("main")->getItem(0).getItemString() == "default:lava 5");
+	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<MockInventoryManager>(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<MockInventoryManager>(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();
 }