Restructure devtest's unittests and run them in CI (#11859)

Author: sfan5

.github/workflows/build.yml

@@ -93,7 +93,7 @@ jobs:
         run: |
           ./bin/minetest --run-unittests
 
-      - name: Integration test
+      - name: Integration test + devtest
         run: |
           ./util/test_multiplayer.sh
 

games/devtest/mods/unittests/crafting.lua

@@ -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)

games/devtest/mods/unittests/init.lua

@@ -1,18 +1,199 @@
 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
-

games/devtest/mods/unittests/itemdescription.lua

@@ -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)

games/devtest/mods/unittests/misc.lua

@@ -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})

games/devtest/mods/unittests/player.lua

@@ -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})