Skip to content

Commit 8472141

Browse files
sfan5sfan5
authoredDec 18, 2021
Restructure devtest's unittests and run them in CI (#11859)
·
5.11.05.5.0
1 parent 1c5ece8 commit 8472141

File tree

12 files changed

+289
-72
lines changed

12 files changed

+289
-72
lines changed
 

Diff for: ‎.github/workflows/build.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ jobs:
9393
run: |
9494
./bin/minetest --run-unittests
9595
96-
- name: Integration test
96+
- name: Integration test + devtest
9797
run: |
9898
./util/test_multiplayer.sh
9999

Diff for: ‎games/devtest/mods/unittests/crafting.lua

+5-13
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1+
dofile(core.get_modpath(core.get_current_modname()) .. "/crafting_prepare.lua")
2+
13
-- Test minetest.clear_craft function
24
local function test_clear_craft()
3-
minetest.log("info", "[unittests] Testing minetest.clear_craft")
45
-- Clearing by output
56
minetest.register_craft({
67
output = "foo",
@@ -22,11 +23,10 @@ local function test_clear_craft()
2223
minetest.clear_craft({recipe={{"foo", "bar"}}})
2324
assert(minetest.get_all_craft_recipes("foo") == nil)
2425
end
26+
unittests.register("test_clear_craft", test_clear_craft)
2527

2628
-- Test minetest.get_craft_result function
2729
local function test_get_craft_result()
28-
minetest.log("info", "[unittests] Testing minetest.get_craft_result")
29-
3030
-- normal
3131
local input = {
3232
method = "normal",
@@ -107,14 +107,6 @@ local function test_get_craft_result()
107107
assert(output.item)
108108
minetest.log("info", "[unittests] unrepairable tool crafting output.item:to_table(): "..dump(output.item:to_table()))
109109
-- unrepairable tool must not yield any output
110-
assert(output.item:get_name() == "")
111-
110+
assert(output.item:is_empty())
112111
end
113-
114-
function unittests.test_crafting()
115-
test_clear_craft()
116-
test_get_craft_result()
117-
minetest.log("action", "[unittests] Crafting tests passed!")
118-
return true
119-
end
120-
112+
unittests.register("test_get_craft_result", test_get_craft_result)

Diff for: ‎games/devtest/mods/unittests/init.lua

+190-9
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,199 @@
11
unittests = {}
22

3+
unittests.list = {}
4+
5+
-- name: Name of the test
6+
-- func:
7+
-- for sync: function(player, pos), should error on failure
8+
-- for async: function(callback, player, pos)
9+
-- MUST call callback() or callback("error msg") in case of error once test is finished
10+
-- this means you cannot use assert() in the test implementation
11+
-- opts: {
12+
-- player = false, -- Does test require a player?
13+
-- map = false, -- Does test require map access?
14+
-- async = false, -- Does the test run asynchronously? (read notes above!)
15+
-- }
16+
function unittests.register(name, func, opts)
17+
local def = table.copy(opts or {})
18+
def.name = name
19+
def.func = func
20+
table.insert(unittests.list, def)
21+
end
22+
23+
function unittests.on_finished(all_passed)
24+
-- free to override
25+
end
26+
27+
-- Calls invoke with a callback as argument
28+
-- Suspends coroutine until that callback is called
29+
-- Return values are passed through
30+
local function await(invoke)
31+
local co = coroutine.running()
32+
assert(co)
33+
local called_early = true
34+
invoke(function(...)
35+
if called_early == true then
36+
called_early = {...}
37+
else
38+
coroutine.resume(co, ...)
39+
end
40+
end)
41+
if called_early ~= true then
42+
-- callback was already called before yielding
43+
return unpack(called_early)
44+
end
45+
called_early = nil
46+
return coroutine.yield()
47+
end
48+
49+
function unittests.run_one(idx, counters, out_callback, player, pos)
50+
local def = unittests.list[idx]
51+
if not def.player then
52+
player = nil
53+
elseif player == nil then
54+
out_callback(false)
55+
return false
56+
end
57+
if not def.map then
58+
pos = nil
59+
elseif pos == nil then
60+
out_callback(false)
61+
return false
62+
end
63+
64+
local tbegin = core.get_us_time()
65+
local function done(status, err)
66+
local tend = core.get_us_time()
67+
local ms_taken = (tend - tbegin) / 1000
68+
69+
if not status then
70+
core.log("error", err)
71+
end
72+
print(string.format("[%s] %s - %dms",
73+
status and "PASS" or "FAIL", def.name, ms_taken))
74+
counters.time = counters.time + ms_taken
75+
counters.total = counters.total + 1
76+
if status then
77+
counters.passed = counters.passed + 1
78+
end
79+
end
80+
81+
if def.async then
82+
core.log("info", "[unittest] running " .. def.name .. " (async)")
83+
def.func(function(err)
84+
done(err == nil, err)
85+
out_callback(true)
86+
end, player, pos)
87+
else
88+
core.log("info", "[unittest] running " .. def.name)
89+
local status, err = pcall(def.func, player, pos)
90+
done(status, err)
91+
out_callback(true)
92+
end
93+
94+
return true
95+
end
96+
97+
local function wait_for_player(callback)
98+
if #core.get_connected_players() > 0 then
99+
return callback(core.get_connected_players()[1])
100+
end
101+
local first = true
102+
core.register_on_joinplayer(function(player)
103+
if first then
104+
callback(player)
105+
first = false
106+
end
107+
end)
108+
end
109+
110+
local function wait_for_map(player, callback)
111+
local check = function()
112+
if core.get_node_or_nil(player:get_pos()) ~= nil then
113+
callback()
114+
else
115+
minetest.after(0, check)
116+
end
117+
end
118+
check()
119+
end
120+
121+
function unittests.run_all()
122+
-- This runs in a coroutine so it uses await().
123+
local counters = { time = 0, total = 0, passed = 0 }
124+
125+
-- Run standalone tests first
126+
for idx = 1, #unittests.list do
127+
local def = unittests.list[idx]
128+
def.done = await(function(cb)
129+
unittests.run_one(idx, counters, cb, nil, nil)
130+
end)
131+
end
132+
133+
-- Wait for a player to join, run tests that require a player
134+
local player = await(wait_for_player)
135+
for idx = 1, #unittests.list do
136+
local def = unittests.list[idx]
137+
if not def.done then
138+
def.done = await(function(cb)
139+
unittests.run_one(idx, counters, cb, player, nil)
140+
end)
141+
end
142+
end
143+
144+
-- Wait for the world to generate/load, run tests that require map access
145+
await(function(cb)
146+
wait_for_map(player, cb)
147+
end)
148+
local pos = vector.round(player:get_pos())
149+
for idx = 1, #unittests.list do
150+
local def = unittests.list[idx]
151+
if not def.done then
152+
def.done = await(function(cb)
153+
unittests.run_one(idx, counters, cb, player, pos)
154+
end)
155+
end
156+
end
157+
158+
-- Print stats
159+
assert(#unittests.list == counters.total)
160+
print(string.rep("+", 80))
161+
print(string.format("Unit Test Results: %s",
162+
counters.total == counters.passed and "PASSED" or "FAILED"))
163+
print(string.format(" %d / %d failed tests.",
164+
counters.total - counters.passed, counters.total))
165+
print(string.format(" Testing took %dms total.", counters.time))
166+
print(string.rep("+", 80))
167+
unittests.on_finished(counters.total == counters.passed)
168+
return counters.total == counters.passed
169+
end
170+
171+
--------------
172+
3173
local modpath = minetest.get_modpath("unittests")
4-
dofile(modpath .. "/random.lua")
174+
dofile(modpath .. "/misc.lua")
5175
dofile(modpath .. "/player.lua")
6-
dofile(modpath .. "/crafting_prepare.lua")
7176
dofile(modpath .. "/crafting.lua")
8177
dofile(modpath .. "/itemdescription.lua")
9178

10-
if minetest.settings:get_bool("devtest_unittests_autostart", false) then
11-
unittests.test_random()
12-
unittests.test_crafting()
13-
unittests.test_short_desc()
14-
minetest.register_on_joinplayer(function(player)
15-
unittests.test_player(player)
179+
--------------
180+
181+
if core.settings:get_bool("devtest_unittests_autostart", false) then
182+
core.after(0, function()
183+
coroutine.wrap(unittests.run_all)()
16184
end)
185+
else
186+
minetest.register_chatcommand("unittests", {
187+
privs = {basic_privs=true},
188+
description = "Runs devtest unittests (may modify player or map state)",
189+
func = function(name, param)
190+
unittests.on_finished = function(ok)
191+
core.chat_send_player(name,
192+
(ok and "All tests passed." or "There were test failures.") ..
193+
" Check the console for detailed output.")
194+
end
195+
coroutine.wrap(unittests.run_all)()
196+
return true, ""
197+
end,
198+
})
17199
end
18-

Diff for: ‎games/devtest/mods/unittests/itemdescription.lua

+2-1
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ minetest.register_chatcommand("item_description", {
2525
end
2626
})
2727

28-
function unittests.test_short_desc()
28+
local function test_short_desc()
2929
local function get_short_description(item)
3030
return ItemStack(item):get_short_description()
3131
end
@@ -49,3 +49,4 @@ function unittests.test_short_desc()
4949

5050
return true
5151
end
52+
unittests.register("test_short_desc", test_short_desc)

Diff for: ‎games/devtest/mods/unittests/misc.lua

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
local function test_random()
2+
-- Try out PseudoRandom
3+
local pseudo = PseudoRandom(13)
4+
assert(pseudo:next() == 22290)
5+
assert(pseudo:next() == 13854)
6+
end
7+
unittests.register("test_random", test_random)
8+
9+
local function test_dynamic_media(cb, player)
10+
if core.get_player_information(player:get_player_name()).protocol_version < 40 then
11+
core.log("warning", "test_dynamic_media: Client too old, skipping test.")
12+
return cb()
13+
end
14+
15+
-- Check that the client acknowledges media transfers
16+
local path = core.get_worldpath() .. "/test_media.obj"
17+
local f = io.open(path, "w")
18+
f:write("# contents don't matter\n")
19+
f:close()
20+
21+
local call_ok = false
22+
local ok = core.dynamic_add_media({
23+
filepath = path,
24+
to_player = player:get_player_name(),
25+
}, function(name)
26+
if not call_ok then
27+
cb("impossible condition")
28+
end
29+
cb()
30+
end)
31+
if not ok then
32+
return cb("dynamic_add_media() returned error")
33+
end
34+
call_ok = true
35+
36+
-- if the callback isn't called this test will just hang :shrug:
37+
end
38+
unittests.register("test_dynamic_media", test_dynamic_media, {async=true, player=true})

Diff for: ‎games/devtest/mods/unittests/player.lua

+20-26
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,21 @@
22
-- HP Change Reasons
33
--
44
local expect = nil
5+
minetest.register_on_player_hpchange(function(player, hp, reason)
6+
if expect == nil then
7+
return
8+
end
9+
10+
for key, value in pairs(reason) do
11+
assert(expect[key] == value)
12+
end
13+
for key, value in pairs(expect) do
14+
assert(reason[key] == value)
15+
end
16+
17+
expect = nil
18+
end)
19+
520
local function run_hpchangereason_tests(player)
621
local old_hp = player:get_hp()
722

@@ -20,7 +35,11 @@ local function run_hpchangereason_tests(player)
2035

2136
player:set_hp(old_hp)
2237
end
38+
unittests.register("test_hpchangereason", run_hpchangereason_tests, {player=true})
2339

40+
--
41+
-- Player meta
42+
--
2443
local function run_player_meta_tests(player)
2544
local meta = player:get_meta()
2645
meta:set_string("foo", "bar")
@@ -48,29 +67,4 @@ local function run_player_meta_tests(player)
4867
assert(meta:get_string("foo") == "")
4968
assert(meta:equals(meta2))
5069
end
51-
52-
function unittests.test_player(player)
53-
minetest.register_on_player_hpchange(function(player, hp, reason)
54-
if not expect then
55-
return
56-
end
57-
58-
for key, value in pairs(reason) do
59-
assert(expect[key] == value)
60-
end
61-
62-
for key, value in pairs(expect) do
63-
assert(reason[key] == value)
64-
end
65-
66-
expect = nil
67-
end)
68-
69-
run_hpchangereason_tests(player)
70-
run_player_meta_tests(player)
71-
local msg = "Player tests passed for player '"..player:get_player_name().."'!"
72-
minetest.chat_send_all(msg)
73-
minetest.log("action", "[unittests] "..msg)
74-
return true
75-
end
76-
70+
unittests.register("test_player_meta", run_player_meta_tests, {player=true})

Diff for: ‎games/devtest/mods/unittests/random.lua

-10
This file was deleted.

Diff for: ‎src/script/cpp_api/s_base.h

+9
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,15 @@ class ScriptApiBase : protected LuaHelper {
125125
friend class ModApiEnvMod;
126126
friend class LuaVoxelManip;
127127

128+
/*
129+
Subtle edge case with coroutines: If for whatever reason you have a
130+
method in a subclass that's called from existing lua_CFunction
131+
(any of the l_*.cpp files) then make it static and take the lua_State*
132+
as an argument. This is REQUIRED because getStack() will not return the
133+
correct state if called inside coroutines.
134+
135+
Also note that src/script/common/ is the better place for such helpers.
136+
*/
128137
lua_State* getStack()
129138
{ return m_luastack; }
130139

Diff for: ‎src/script/cpp_api/s_server.cpp

+2-4
Original file line numberDiff line numberDiff line change
@@ -198,10 +198,8 @@ std::string ScriptApiServer::formatChatMessage(const std::string &name,
198198
return ret;
199199
}
200200

201-
u32 ScriptApiServer::allocateDynamicMediaCallback(int f_idx)
201+
u32 ScriptApiServer::allocateDynamicMediaCallback(lua_State *L, int f_idx)
202202
{
203-
lua_State *L = getStack();
204-
205203
if (f_idx < 0)
206204
f_idx = lua_gettop(L) + f_idx + 1;
207205

@@ -235,7 +233,7 @@ u32 ScriptApiServer::allocateDynamicMediaCallback(int f_idx)
235233

236234
void ScriptApiServer::freeDynamicMediaCallback(u32 token)
237235
{
238-
lua_State *L = getStack();
236+
SCRIPTAPI_PRECHECKHEADER
239237

240238
verbosestream << "freeDynamicMediaCallback(" << token << ")" << std::endl;
241239

Diff for: ‎src/script/cpp_api/s_server.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class ScriptApiServer
5151
const std::string &password);
5252

5353
/* dynamic media handling */
54-
u32 allocateDynamicMediaCallback(int f_idx);
54+
static u32 allocateDynamicMediaCallback(lua_State *L, int f_idx);
5555
void freeDynamicMediaCallback(u32 token);
5656
void on_dynamic_media_added(u32 token, const char *playername);
5757

Diff for: ‎src/script/lua_api/l_server.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -496,7 +496,7 @@ int ModApiServer::l_dynamic_add_media(lua_State *L)
496496

497497
CHECK_SECURE_PATH(L, filepath.c_str(), false);
498498

499-
u32 token = server->getScriptIface()->allocateDynamicMediaCallback(2);
499+
u32 token = server->getScriptIface()->allocateDynamicMediaCallback(L, 2);
500500

501501
bool ok = server->dynamicAddMedia(filepath, token, to_player, ephemeral);
502502
if (!ok)

Diff for: ‎util/test_multiplayer.sh

+20-6
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ waitfor () {
2020
}
2121

2222
gdbrun () {
23-
gdb -q -ex 'set confirm off' -ex 'r' -ex 'bt' -ex 'quit' --args "$@"
23+
gdb -q -batch -ex 'set confirm off' -ex 'r' -ex 'bt' --args "$@"
2424
}
2525

2626
[ -e $minetest ] || { echo "executable $minetest missing"; exit 1; }
@@ -33,28 +33,42 @@ printf '%s\n' >$testspath/client1.conf \
3333
enable_{sound,minimap,shaders}=false
3434

3535
printf '%s\n' >$testspath/server.conf \
36-
max_block_send_distance=1
36+
max_block_send_distance=1 devtest_unittests_autostart=true
3737

3838
cat >$worldpath/worldmods/test/init.lua <<"LUA"
3939
core.after(0, function()
4040
io.close(io.open(core.get_worldpath() .. "/startup", "w"))
4141
end)
42-
core.register_on_joinplayer(function(player)
43-
io.close(io.open(core.get_worldpath() .. "/player_joined", "w"))
42+
local function callback(test_ok)
43+
if not test_ok then
44+
io.close(io.open(core.get_worldpath() .. "/test_failure", "w"))
45+
end
46+
io.close(io.open(core.get_worldpath() .. "/done", "w"))
4447
core.request_shutdown("", false, 2)
45-
end)
48+
end
49+
if core.settings:get_bool("devtest_unittests_autostart") then
50+
unittests.on_finished = callback
51+
else
52+
core.register_on_joinplayer(function() callback(true) end)
53+
end
4654
LUA
55+
printf '%s\n' >$worldpath/worldmods/test/mod.conf \
56+
name=test optional_depends=unittests
4757

4858
echo "Starting server"
4959
gdbrun $minetest --server --config $conf_server --world $worldpath --gameid $gameid 2>&1 | sed -u 's/^/(server) /' &
5060
waitfor $worldpath/startup
5161

5262
echo "Starting client"
5363
gdbrun $minetest --config $conf_client1 --go --address 127.0.0.1 2>&1 | sed -u 's/^/(client) /' &
54-
waitfor $worldpath/player_joined
64+
waitfor $worldpath/done
5565

5666
echo "Waiting for client and server to exit"
5767
wait
5868

69+
if [ -f $worldpath/test_failure ]; then
70+
echo "There were test failures."
71+
exit 1
72+
fi
5973
echo "Success"
6074
exit 0

0 commit comments

Comments
 (0)
Please sign in to comment.