]> git.lizzy.rs Git - dragonfireclient.git/commitdiff
Restructure devtest's unittests and run them in CI (#11859)
authorsfan5 <sfan5@live.de>
Sat, 18 Dec 2021 19:36:43 +0000 (20:36 +0100)
committerGitHub <noreply@github.com>
Sat, 18 Dec 2021 19:36:43 +0000 (20:36 +0100)
12 files changed:
.github/workflows/build.yml
games/devtest/mods/unittests/crafting.lua
games/devtest/mods/unittests/init.lua
games/devtest/mods/unittests/itemdescription.lua
games/devtest/mods/unittests/misc.lua [new file with mode: 0644]
games/devtest/mods/unittests/player.lua
games/devtest/mods/unittests/random.lua [deleted file]
src/script/cpp_api/s_base.h
src/script/cpp_api/s_server.cpp
src/script/cpp_api/s_server.h
src/script/lua_api/l_server.cpp
util/test_multiplayer.sh

index 417b4f65024aba44304466623199fd0a93bc9da2..af1de15ec068bdc90fce556e042712964435b0d4 100644 (file)
@@ -93,7 +93,7 @@ jobs:
         run: |
           ./bin/minetest --run-unittests
 
-      - name: Integration test
+      - name: Integration test + devtest
         run: |
           ./util/test_multiplayer.sh
 
index eff13ce090e009cc215a5777f50b8fd1c5fac6b7..8c16d3efb3f884d4c80a5681450e42254360536c 100644 (file)
@@ -1,6 +1,7 @@
+dofile(core.get_modpath(core.get_current_modname()) .. "/crafting_prepare.lua")
+
 -- Test minetest.clear_craft function
 local function test_clear_craft()
-       minetest.log("info", "[unittests] Testing minetest.clear_craft")
        -- Clearing by output
        minetest.register_craft({
                output = "foo",
@@ -22,11 +23,10 @@ local function test_clear_craft()
        minetest.clear_craft({recipe={{"foo", "bar"}}})
        assert(minetest.get_all_craft_recipes("foo") == nil)
 end
+unittests.register("test_clear_craft", test_clear_craft)
 
 -- Test minetest.get_craft_result function
 local function test_get_craft_result()
-       minetest.log("info", "[unittests] Testing minetest.get_craft_result")
-
        -- normal
        local input = {
                method = "normal",
@@ -107,14 +107,6 @@ local function test_get_craft_result()
        assert(output.item)
        minetest.log("info", "[unittests] unrepairable tool crafting output.item:to_table(): "..dump(output.item:to_table()))
        -- unrepairable tool must not yield any output
-       assert(output.item:get_name() == "")
-
+       assert(output.item:is_empty())
 end
-
-function unittests.test_crafting()
-       test_clear_craft()
-       test_get_craft_result()
-       minetest.log("action", "[unittests] Crafting tests passed!")
-       return true
-end
-
+unittests.register("test_get_craft_result", test_get_craft_result)
index 12c67f78beb072958a35297d72d82862548e3de8..0754d507f9416a49e3532de3373ce30a9eb32f45 100644 (file)
 unittests = {}
 
+unittests.list = {}
+
+-- name: Name of the test
+-- func:
+--   for sync: function(player, pos), should error on failure
+--   for async: function(callback, player, pos)
+--     MUST call callback() or callback("error msg") in case of error once test is finished
+--     this means you cannot use assert() in the test implementation
+-- opts: {
+--   player = false, -- Does test require a player?
+--   map = false, -- Does test require map access?
+--   async = false, -- Does the test run asynchronously? (read notes above!)
+-- }
+function unittests.register(name, func, opts)
+       local def = table.copy(opts or {})
+       def.name = name
+       def.func = func
+       table.insert(unittests.list, def)
+end
+
+function unittests.on_finished(all_passed)
+       -- free to override
+end
+
+-- Calls invoke with a callback as argument
+-- Suspends coroutine until that callback is called
+-- Return values are passed through
+local function await(invoke)
+       local co = coroutine.running()
+       assert(co)
+       local called_early = true
+       invoke(function(...)
+               if called_early == true then
+                       called_early = {...}
+               else
+                       coroutine.resume(co, ...)
+               end
+       end)
+       if called_early ~= true then
+               -- callback was already called before yielding
+               return unpack(called_early)
+       end
+       called_early = nil
+       return coroutine.yield()
+end
+
+function unittests.run_one(idx, counters, out_callback, player, pos)
+       local def = unittests.list[idx]
+       if not def.player then
+               player = nil
+       elseif player == nil then
+               out_callback(false)
+               return false
+       end
+       if not def.map then
+               pos = nil
+       elseif pos == nil then
+               out_callback(false)
+               return false
+       end
+
+       local tbegin = core.get_us_time()
+       local function done(status, err)
+               local tend = core.get_us_time()
+               local ms_taken = (tend - tbegin) / 1000
+
+               if not status then
+                       core.log("error", err)
+               end
+               print(string.format("[%s] %s - %dms",
+                       status and "PASS" or "FAIL", def.name, ms_taken))
+               counters.time = counters.time + ms_taken
+               counters.total = counters.total + 1
+               if status then
+                       counters.passed = counters.passed + 1
+               end
+       end
+
+       if def.async then
+               core.log("info", "[unittest] running " .. def.name .. " (async)")
+               def.func(function(err)
+                       done(err == nil, err)
+                       out_callback(true)
+               end, player, pos)
+       else
+               core.log("info", "[unittest] running " .. def.name)
+               local status, err = pcall(def.func, player, pos)
+               done(status, err)
+               out_callback(true)
+       end
+       
+       return true
+end
+
+local function wait_for_player(callback)
+       if #core.get_connected_players() > 0 then
+               return callback(core.get_connected_players()[1])
+       end
+       local first = true
+       core.register_on_joinplayer(function(player)
+               if first then
+                       callback(player)
+                       first = false
+               end
+       end)
+end
+
+local function wait_for_map(player, callback)
+       local check = function()
+               if core.get_node_or_nil(player:get_pos()) ~= nil then
+                       callback()
+               else
+                       minetest.after(0, check)
+               end
+       end
+       check()
+end
+
+function unittests.run_all()
+       -- This runs in a coroutine so it uses await().
+       local counters = { time = 0, total = 0, passed = 0 }
+
+       -- Run standalone tests first
+       for idx = 1, #unittests.list do
+               local def = unittests.list[idx]
+               def.done = await(function(cb)
+                       unittests.run_one(idx, counters, cb, nil, nil)
+               end)
+       end
+
+       -- Wait for a player to join, run tests that require a player
+       local player = await(wait_for_player)
+       for idx = 1, #unittests.list do
+               local def = unittests.list[idx]
+               if not def.done then
+                       def.done = await(function(cb)
+                               unittests.run_one(idx, counters, cb, player, nil)
+                       end)
+               end
+       end
+
+       -- Wait for the world to generate/load, run tests that require map access
+       await(function(cb)
+               wait_for_map(player, cb)
+       end)
+       local pos = vector.round(player:get_pos())
+       for idx = 1, #unittests.list do
+               local def = unittests.list[idx]
+               if not def.done then
+                       def.done = await(function(cb)
+                               unittests.run_one(idx, counters, cb, player, pos)
+                       end)
+               end
+       end
+
+       -- Print stats
+       assert(#unittests.list == counters.total)
+       print(string.rep("+", 80))
+       print(string.format("Unit Test Results: %s",
+       counters.total == counters.passed and "PASSED" or "FAILED"))
+       print(string.format("    %d / %d failed tests.",
+       counters.total - counters.passed, counters.total))
+       print(string.format("    Testing took %dms total.", counters.time))
+       print(string.rep("+", 80))
+       unittests.on_finished(counters.total == counters.passed)
+       return counters.total == counters.passed
+end
+
+--------------
+
 local modpath = minetest.get_modpath("unittests")
-dofile(modpath .. "/random.lua")
+dofile(modpath .. "/misc.lua")
 dofile(modpath .. "/player.lua")
-dofile(modpath .. "/crafting_prepare.lua")
 dofile(modpath .. "/crafting.lua")
 dofile(modpath .. "/itemdescription.lua")
 
-if minetest.settings:get_bool("devtest_unittests_autostart", false) then
-       unittests.test_random()
-       unittests.test_crafting()
-       unittests.test_short_desc()
-       minetest.register_on_joinplayer(function(player)
-               unittests.test_player(player)
+--------------
+
+if core.settings:get_bool("devtest_unittests_autostart", false) then
+       core.after(0, function()
+               coroutine.wrap(unittests.run_all)()
        end)
+else
+       minetest.register_chatcommand("unittests", {
+               privs = {basic_privs=true},
+               description = "Runs devtest unittests (may modify player or map state)",
+               func = function(name, param)
+                       unittests.on_finished = function(ok)
+                               core.chat_send_player(name,
+                                       (ok and "All tests passed." or "There were test failures.") ..
+                                       " Check the console for detailed output.")
+                       end
+                       coroutine.wrap(unittests.run_all)()
+                       return true, ""
+               end,
+       })
 end
-
index d6ee6551affbce9a35a235e9d9577b00c5ed8699..dc62de7f0afb6eeb1eff93f9423a718fdbc1b97c 100644 (file)
@@ -25,7 +25,7 @@ minetest.register_chatcommand("item_description", {
        end
 })
 
-function unittests.test_short_desc()
+local function test_short_desc()
        local function get_short_description(item)
                return ItemStack(item):get_short_description()
        end
@@ -49,3 +49,4 @@ function unittests.test_short_desc()
 
        return true
 end
+unittests.register("test_short_desc", test_short_desc)
diff --git a/games/devtest/mods/unittests/misc.lua b/games/devtest/mods/unittests/misc.lua
new file mode 100644 (file)
index 0000000..cf4f92c
--- /dev/null
@@ -0,0 +1,38 @@
+local function test_random()
+       -- Try out PseudoRandom
+       local pseudo = PseudoRandom(13)
+       assert(pseudo:next() == 22290)
+       assert(pseudo:next() == 13854)
+end
+unittests.register("test_random", test_random)
+
+local function test_dynamic_media(cb, player)
+       if core.get_player_information(player:get_player_name()).protocol_version < 40 then
+               core.log("warning", "test_dynamic_media: Client too old, skipping test.")
+               return cb()
+       end
+
+       -- Check that the client acknowledges media transfers
+       local path = core.get_worldpath() .. "/test_media.obj"
+       local f = io.open(path, "w")
+       f:write("# contents don't matter\n")
+       f:close()
+
+       local call_ok = false
+       local ok = core.dynamic_add_media({
+               filepath = path,
+               to_player = player:get_player_name(),
+       }, function(name)
+               if not call_ok then
+                       cb("impossible condition")
+               end
+               cb()
+       end)
+       if not ok then
+               return cb("dynamic_add_media() returned error")
+       end
+       call_ok = true
+
+       -- if the callback isn't called this test will just hang :shrug:
+end
+unittests.register("test_dynamic_media", test_dynamic_media, {async=true, player=true})
index 4a681310dc0acd3402e9e7de8bb96a366b867cc0..fa0557960580020ebcbb0871d9923afe94c2d5be 100644 (file)
@@ -2,6 +2,21 @@
 -- HP Change Reasons
 --
 local expect = nil
+minetest.register_on_player_hpchange(function(player, hp, reason)
+       if expect == nil then
+               return
+       end
+
+       for key, value in pairs(reason) do
+               assert(expect[key] == value)
+       end
+       for key, value in pairs(expect) do
+               assert(reason[key] == value)
+       end
+
+       expect = nil
+end)
+
 local function run_hpchangereason_tests(player)
        local old_hp = player:get_hp()
 
@@ -20,7 +35,11 @@ local function run_hpchangereason_tests(player)
 
        player:set_hp(old_hp)
 end
+unittests.register("test_hpchangereason", run_hpchangereason_tests, {player=true})
 
+--
+-- Player meta
+--
 local function run_player_meta_tests(player)
        local meta = player:get_meta()
        meta:set_string("foo", "bar")
@@ -48,29 +67,4 @@ local function run_player_meta_tests(player)
        assert(meta:get_string("foo") == "")
        assert(meta:equals(meta2))
 end
-
-function unittests.test_player(player)
-       minetest.register_on_player_hpchange(function(player, hp, reason)
-               if not expect then
-                       return
-               end
-
-               for key, value in pairs(reason) do
-                       assert(expect[key] == value)
-               end
-
-               for key, value in pairs(expect) do
-                       assert(reason[key] == value)
-               end
-
-               expect = nil
-       end)
-
-       run_hpchangereason_tests(player)
-       run_player_meta_tests(player)
-       local msg = "Player tests passed for player '"..player:get_player_name().."'!"
-       minetest.chat_send_all(msg)
-       minetest.log("action", "[unittests] "..msg)
-       return true
-end
-
+unittests.register("test_player_meta", run_player_meta_tests, {player=true})
diff --git a/games/devtest/mods/unittests/random.lua b/games/devtest/mods/unittests/random.lua
deleted file mode 100644 (file)
index f94f0a8..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-function unittests.test_random()
-       -- Try out PseudoRandom
-       minetest.log("action", "[unittests] Testing PseudoRandom ...")
-       local pseudo = PseudoRandom(13)
-       assert(pseudo:next() == 22290)
-       assert(pseudo:next() == 13854)
-       minetest.log("action", "[unittests] PseudoRandom test passed!")
-       return true
-end
-
index 06df2abe3945860647ee5e62911930ef7755a377..244d81605901b407fba659c5ef7148d3831bffd0 100644 (file)
@@ -125,6 +125,15 @@ class ScriptApiBase : protected LuaHelper {
        friend class ModApiEnvMod;
        friend class LuaVoxelManip;
 
+       /*
+               Subtle edge case with coroutines: If for whatever reason you have a
+               method in a subclass that's called from existing lua_CFunction
+               (any of the l_*.cpp files) then make it static and take the lua_State*
+               as an argument. This is REQUIRED because getStack() will not return the
+               correct state if called inside coroutines.
+
+               Also note that src/script/common/ is the better place for such helpers.
+       */
        lua_State* getStack()
                { return m_luastack; }
 
index 6ddb2630d45048aa1b64b9e718066497454f283f..c255b0c71cfb6119c81cb8f2e26f971c19ab2ddd 100644 (file)
@@ -198,10 +198,8 @@ std::string ScriptApiServer::formatChatMessage(const std::string &name,
        return ret;
 }
 
-u32 ScriptApiServer::allocateDynamicMediaCallback(int f_idx)
+u32 ScriptApiServer::allocateDynamicMediaCallback(lua_State *L, int f_idx)
 {
-       lua_State *L = getStack();
-
        if (f_idx < 0)
                f_idx = lua_gettop(L) + f_idx + 1;
 
@@ -235,7 +233,7 @@ u32 ScriptApiServer::allocateDynamicMediaCallback(int f_idx)
 
 void ScriptApiServer::freeDynamicMediaCallback(u32 token)
 {
-       lua_State *L = getStack();
+       SCRIPTAPI_PRECHECKHEADER
 
        verbosestream << "freeDynamicMediaCallback(" << token << ")" << std::endl;
 
index c5c3d559606723006e5f1a2b9e9e64c6e2d39fd1..58c8c0e481cec9c97c4f6386fffa18a0b8ce2d22 100644 (file)
@@ -51,7 +51,7 @@ class ScriptApiServer
                const std::string &password);
 
        /* dynamic media handling */
-       u32 allocateDynamicMediaCallback(int f_idx);
+       static u32 allocateDynamicMediaCallback(lua_State *L, int f_idx);
        void freeDynamicMediaCallback(u32 token);
        void on_dynamic_media_added(u32 token, const char *playername);
 
index 82a6920701799a904d4279697e88e58e9a325c3c..88ab5e16b4b8dc41a51c8bcd24c1e28bce052a91 100644 (file)
@@ -496,7 +496,7 @@ int ModApiServer::l_dynamic_add_media(lua_State *L)
 
        CHECK_SECURE_PATH(L, filepath.c_str(), false);
 
-       u32 token = server->getScriptIface()->allocateDynamicMediaCallback(2);
+       u32 token = server->getScriptIface()->allocateDynamicMediaCallback(L, 2);
 
        bool ok = server->dynamicAddMedia(filepath, token, to_player, ephemeral);
        if (!ok)
index 9fb894a3032a4bb2e8abbd976d545c926172c84f..5ffc044e06bc099d153df8381f7ccc6897e1185c 100755 (executable)
@@ -20,7 +20,7 @@ waitfor () {
 }
 
 gdbrun () {
-       gdb -q -ex 'set confirm off' -ex 'r' -ex 'bt' -ex 'quit' --args "$@"
+       gdb -q -batch -ex 'set confirm off' -ex 'r' -ex 'bt' --args "$@"
 }
 
 [ -e $minetest ] || { echo "executable $minetest missing"; exit 1; }
@@ -33,17 +33,27 @@ printf '%s\n' >$testspath/client1.conf \
        enable_{sound,minimap,shaders}=false
 
 printf '%s\n' >$testspath/server.conf \
-       max_block_send_distance=1
+       max_block_send_distance=1 devtest_unittests_autostart=true
 
 cat >$worldpath/worldmods/test/init.lua <<"LUA"
 core.after(0, function()
        io.close(io.open(core.get_worldpath() .. "/startup", "w"))
 end)
-core.register_on_joinplayer(function(player)
-       io.close(io.open(core.get_worldpath() .. "/player_joined", "w"))
+local function callback(test_ok)
+       if not test_ok then
+               io.close(io.open(core.get_worldpath() .. "/test_failure", "w"))
+       end
+       io.close(io.open(core.get_worldpath() .. "/done", "w"))
        core.request_shutdown("", false, 2)
-end)
+end
+if core.settings:get_bool("devtest_unittests_autostart") then
+       unittests.on_finished = callback
+else
+       core.register_on_joinplayer(function() callback(true) end)
+end
 LUA
+printf '%s\n' >$worldpath/worldmods/test/mod.conf \
+       name=test optional_depends=unittests
 
 echo "Starting server"
 gdbrun $minetest --server --config $conf_server --world $worldpath --gameid $gameid 2>&1 | sed -u 's/^/(server) /' &
@@ -51,10 +61,14 @@ waitfor $worldpath/startup
 
 echo "Starting client"
 gdbrun $minetest --config $conf_client1 --go --address 127.0.0.1 2>&1 | sed -u 's/^/(client) /' &
-waitfor $worldpath/player_joined
+waitfor $worldpath/done
 
 echo "Waiting for client and server to exit"
 wait
 
+if [ -f $worldpath/test_failure ]; then
+       echo "There were test failures."
+       exit 1
+fi
 echo "Success"
 exit 0