From 9fdb586592e24fbd42a42b1ea8a06fcb4a6dbf93 Mon Sep 17 00:00:00 2001 From: bfredl Date: Wed, 18 Jan 2023 11:54:49 +0100 Subject: fix(unittests): do not consider process crash to be a success unittests relied on the exact setup of coredumps on CI to detect process crashing, and otherwise completely discarded errors. Dectect child process failure reliably using process status, so that unittests actually work locally as well. --- test/unit/helpers.lua | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'test/unit/helpers.lua') diff --git a/test/unit/helpers.lua b/test/unit/helpers.lua index 29ea0235be..4576edf436 100644 --- a/test/unit/helpers.lua +++ b/test/unit/helpers.lua @@ -471,8 +471,9 @@ else close = ffi.C.close, wait = function(pid) ffi.errno(0) + local stat_loc = ffi.new('int[1]', {0}) while true do - local r = ffi.C.waitpid(pid, nil, ffi.C.kPOSIXWaitWUNTRACED) + local r = ffi.C.waitpid(pid, stat_loc, ffi.C.kPOSIXWaitWUNTRACED) if r == -1 then local err = ffi.errno(0) if err == ffi.C.kPOSIXErrnoECHILD then @@ -485,6 +486,7 @@ else assert(r == pid) end end + return stat_loc[0] end, exit = ffi.C._exit, } @@ -730,18 +732,22 @@ local function check_child_err(rd) end end -local function itp_parent(rd, pid, allow_failure) - local err, emsg = pcall(check_child_err, rd) - sc.wait(pid) +local function itp_parent(rd, pid, allow_failure, location) + local ok, emsg = pcall(check_child_err, rd) + local status = sc.wait(pid) sc.close(rd) - if not err then + if not ok then if allow_failure then - io.stderr:write('Errorred out:\n' .. tostring(emsg) .. '\n') + io.stderr:write('Errorred out ('..status..'):\n' .. tostring(emsg) .. '\n') os.execute([[ sh -c "source ci/common/test.sh check_core_dumps --delete \"]] .. Paths.test_luajit_prg .. [[\""]]) else - error(emsg) + error(tostring(emsg)..'\nexit code: '..status) + end + elseif status ~= 0 then + if not allow_failure then + error("child process errored out with status "..status.."!\n\n"..location) end end end @@ -758,6 +764,11 @@ local function gen_itp(it) -- FIXME Fix tests with this true return end + + -- Pre-emptively calculating error location, wasteful, ugh! + -- But the way this code messes around with busted implies the real location is strictly + -- not available in the parent when an actual error occurs. so we have to do this here. + local location = debug.traceback() it(name, function() local rd, wr = sc.pipe() child_pid = sc.fork() @@ -768,7 +779,7 @@ local function gen_itp(it) sc.close(wr) local saved_child_pid = child_pid child_pid = nil - itp_parent(rd, saved_child_pid, allow_failure) + itp_parent(rd, saved_child_pid, allow_failure, location) end end) end -- cgit From 847a1507aaf0877cda253ee4fdcb036fcd67816b Mon Sep 17 00:00:00 2001 From: bfredl Date: Wed, 18 Jan 2023 12:12:13 +0100 Subject: fix(unittest): delete unused duplicated code YAGNI. These were disabled 5 years ago in lint commit 29ed5b3a39abb84d6af602b2d0a7680d9dab381c --- test/unit/helpers.lua | 192 ++++++++++++++++++++------------------------------ 1 file changed, 78 insertions(+), 114 deletions(-) (limited to 'test/unit/helpers.lua') diff --git a/test/unit/helpers.lua b/test/unit/helpers.lua index 4576edf436..14d8eda357 100644 --- a/test/unit/helpers.lua +++ b/test/unit/helpers.lua @@ -7,9 +7,6 @@ local global_helpers = require('test.helpers') local assert = require('luassert') local say = require('say') -local posix = nil -local syscall = nil - local check_cores = global_helpers.check_cores local dedent = global_helpers.dedent local neq = global_helpers.neq @@ -373,124 +370,91 @@ local function to_cstr(string) return cstr(#string + 1, string) end -local sc - -if posix ~= nil then - sc = { - fork = posix.fork, - pipe = posix.pipe, - read = posix.read, - write = posix.write, - close = posix.close, - wait = posix.wait, - exit = posix._exit, - } -elseif syscall ~= nil then - sc = { - fork = syscall.fork, - pipe = function() - local ret = {syscall.pipe()} - return ret[3], ret[4] - end, - read = function(rd, len) - return rd:read(nil, len) - end, - write = function(wr, s) - return wr:write(s) - end, - close = function(p) - return p:close() - end, - wait = syscall.wait, - exit = syscall.exit, - } -else - cimport_immediate('./test/unit/fixtures/posix.h') - sc = { - fork = function() - return tonumber(ffi.C.fork()) - end, - pipe = function() - local ret = ffi.new('int[2]', {-1, -1}) - ffi.errno(0) - local res = ffi.C.pipe(ret) - if (res ~= 0) then +cimport_immediate('./test/unit/fixtures/posix.h') +local sc = { + fork = function() + return tonumber(ffi.C.fork()) + end, + pipe = function() + local ret = ffi.new('int[2]', {-1, -1}) + ffi.errno(0) + local res = ffi.C.pipe(ret) + if (res ~= 0) then + local err = ffi.errno(0) + assert(res == 0, ("pipe() error: %u: %s"):format( + err, ffi.string(ffi.C.strerror(err)))) + end + assert(ret[0] ~= -1 and ret[1] ~= -1) + return ret[0], ret[1] + end, + read = function(rd, len) + local ret = ffi.new('char[?]', len, {0}) + local total_bytes_read = 0 + ffi.errno(0) + while total_bytes_read < len do + local bytes_read = tonumber(ffi.C.read( + rd, + ffi.cast('void*', ret + total_bytes_read), + len - total_bytes_read)) + if bytes_read == -1 then local err = ffi.errno(0) - assert(res == 0, ("pipe() error: %u: %s"):format( - err, ffi.string(ffi.C.strerror(err)))) - end - assert(ret[0] ~= -1 and ret[1] ~= -1) - return ret[0], ret[1] - end, - read = function(rd, len) - local ret = ffi.new('char[?]', len, {0}) - local total_bytes_read = 0 - ffi.errno(0) - while total_bytes_read < len do - local bytes_read = tonumber(ffi.C.read( - rd, - ffi.cast('void*', ret + total_bytes_read), - len - total_bytes_read)) - if bytes_read == -1 then - local err = ffi.errno(0) - if err ~= ffi.C.kPOSIXErrnoEINTR then - assert(false, ("read() error: %u: %s"):format( - err, ffi.string(ffi.C.strerror(err)))) - end - elseif bytes_read == 0 then - break - else - total_bytes_read = total_bytes_read + bytes_read + if err ~= ffi.C.kPOSIXErrnoEINTR then + assert(false, ("read() error: %u: %s"):format( + err, ffi.string(ffi.C.strerror(err)))) end + elseif bytes_read == 0 then + break + else + total_bytes_read = total_bytes_read + bytes_read end - return ffi.string(ret, total_bytes_read) - end, - write = function(wr, s) - local wbuf = to_cstr(s) - local total_bytes_written = 0 - ffi.errno(0) - while total_bytes_written < #s do - local bytes_written = tonumber(ffi.C.write( - wr, - ffi.cast('void*', wbuf + total_bytes_written), - #s - total_bytes_written)) - if bytes_written == -1 then - local err = ffi.errno(0) - if err ~= ffi.C.kPOSIXErrnoEINTR then - assert(false, ("write() error: %u: %s ('%s')"):format( - err, ffi.string(ffi.C.strerror(err)), s)) - end - elseif bytes_written == 0 then - break - else - total_bytes_written = total_bytes_written + bytes_written + end + return ffi.string(ret, total_bytes_read) + end, + write = function(wr, s) + local wbuf = to_cstr(s) + local total_bytes_written = 0 + ffi.errno(0) + while total_bytes_written < #s do + local bytes_written = tonumber(ffi.C.write( + wr, + ffi.cast('void*', wbuf + total_bytes_written), + #s - total_bytes_written)) + if bytes_written == -1 then + local err = ffi.errno(0) + if err ~= ffi.C.kPOSIXErrnoEINTR then + assert(false, ("write() error: %u: %s ('%s')"):format( + err, ffi.string(ffi.C.strerror(err)), s)) end + elseif bytes_written == 0 then + break + else + total_bytes_written = total_bytes_written + bytes_written end - return total_bytes_written - end, - close = ffi.C.close, - wait = function(pid) - ffi.errno(0) - local stat_loc = ffi.new('int[1]', {0}) - while true do - local r = ffi.C.waitpid(pid, stat_loc, ffi.C.kPOSIXWaitWUNTRACED) - if r == -1 then - local err = ffi.errno(0) - if err == ffi.C.kPOSIXErrnoECHILD then - break - elseif err ~= ffi.C.kPOSIXErrnoEINTR then - assert(false, ("waitpid() error: %u: %s"):format( - err, ffi.string(ffi.C.strerror(err)))) - end - else - assert(r == pid) + end + return total_bytes_written + end, + close = ffi.C.close, + wait = function(pid) + ffi.errno(0) + local stat_loc = ffi.new('int[1]', {0}) + while true do + local r = ffi.C.waitpid(pid, stat_loc, ffi.C.kPOSIXWaitWUNTRACED) + if r == -1 then + local err = ffi.errno(0) + if err == ffi.C.kPOSIXErrnoECHILD then + break + elseif err ~= ffi.C.kPOSIXErrnoEINTR then + assert(false, ("waitpid() error: %u: %s"):format( + err, ffi.string(ffi.C.strerror(err)))) end + else + assert(r == pid) end - return stat_loc[0] - end, - exit = ffi.C._exit, - } -end + end + return stat_loc[0] + end, + exit = ffi.C._exit, +} local function format_list(lst) local ret = '' -- cgit