Skip to content

Commit

Permalink
Fix saving artifacts
Browse files Browse the repository at this point in the history
> What's the problem?

When creating the logic for saving artifacts [1] a global variable has
been created as `current_test` in the `runner.lua` module. Luatest has
collected all the tests and  runs a simple loop. At the beginning of
each iteration the current test is written to the `current_test'.

Server saved the value of the current test. However at some point the
current test in the global variable and in the server object were
different (the server stored information about the previous test). Also
server does not store information about which test it was used in.

> What is the solution?

Redesign the logic of working with artifacts: now each test knows which
server is linked to it, and each server knows about the test.

For example:

    foo = Server:new()
    foo:start()

    -- only `foo` artifacts
    g.test_with_foo = function()
        foo:exec(...)
    end

    -- no server artifacts
    g.test_without_servers = function()
        ...
    end

[1] #296

Helped-by: Igor Munkin <imun@tarantool.org>

Close #299
  • Loading branch information
Oleg Chaplashkin authored and ylobankov committed Aug 28, 2023
1 parent 3fa719d commit 251b35f
Show file tree
Hide file tree
Showing 12 changed files with 344 additions and 31 deletions.
11 changes: 9 additions & 2 deletions luatest/output/junit.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local utils = require('luatest.utils')
local ROCK_VERSION = require('luatest.VERSION')

-- See directory junitxml for more information about the junit format
Expand All @@ -20,17 +21,23 @@ function Output.xml_c_data_escape(str)
end

function Output.node_status_xml(node)
local artifacts = ''
if utils.table_len(node.servers) > 0 then
for _, server in pairs(node.servers) do
artifacts = ('%s%s -> %s'):format(artifacts, server.alias, server.artifacts)
end
end
if node:is('error') then
return table.concat(
{' <error type="', Output.xml_escape(node.message), '">\n',
' <![CDATA[', Output.xml_c_data_escape(node.trace), ']]>\n',
' <artifacts>', Output.xml_escape(node.artifacts or ''), '</artifacts>\n',
' <artifacts>', Output.xml_escape(artifacts), '</artifacts>\n',
' </error>\n'})
elseif node:is('fail') then
return table.concat(
{' <failure type="', Output.xml_escape(node.message), '">\n',
' <![CDATA[', Output.xml_c_data_escape(node.trace), ']]>\n',
' <artifacts>', Output.xml_escape(node.artifacts or ''), '</artifacts>\n',
' <artifacts>', Output.xml_escape(artifacts), '</artifacts>\n',
' </failure>\n'})
elseif node:is('skip') then
return table.concat({' <skipped>', Output.xml_escape(node.message or ''),'</skipped>\n'})
Expand Down
8 changes: 6 additions & 2 deletions luatest/output/tap.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local utils = require('luatest.utils')
-- For a good reference for TAP format, check: http://testanything.org/tap-specification.html
local Output = require('luatest.output.generic'):new_class()

Expand Down Expand Up @@ -28,8 +29,11 @@ function Output.mt:update_status(node)
end
if (node:is('fail') or node:is('error')) and self.verbosity >= self.class.VERBOSITY.VERBOSE then
print(prefix .. node.trace:gsub('\n', '\n' .. prefix))
if node.artifacts then
print(prefix .. node.artifacts:gsub('\n', '\n' .. prefix))
if utils.table_len(node.servers) > 0 then
print(prefix .. 'artifacts:')
for _, server in pairs(node.servers) do
print(('%s%s -> %s'):format(prefix, server.alias, server.artifacts))
end
end
end
end
Expand Down
9 changes: 6 additions & 3 deletions luatest/output/text.lua
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
local utils = require('luatest.utils')
local Output = require('luatest.output.generic'):new_class()

Output.BOLD_CODE = '\x1B[1m'
Expand Down Expand Up @@ -60,10 +61,12 @@ function Output.mt:display_one_failed_test(index, fail) -- luacheck: no unused
print(index..") " .. fail.name .. self.class.ERROR_COLOR_CODE)
print(fail.message .. self.class.RESET_TERM)
print(fail.trace)
if fail.artifacts then
print(fail.artifacts)
if utils.table_len(fail.servers) > 0 then
print('artifacts:')
for _, server in pairs(fail.servers) do
print(('\t%s -> %s'):format(server.alias, server.artifacts))
end
end
print()
end

function Output.mt:display_errored_tests()
Expand Down
11 changes: 10 additions & 1 deletion luatest/runner.lua
Original file line number Diff line number Diff line change
Expand Up @@ -413,17 +413,19 @@ end
function Runner.mt:run_tests(tests_list)
-- Make seed for ordering not affect other random numbers.
math.randomseed(os.time())
rawset(_G, 'current_test', {value = nil})
for _ = 1, self.exe_repeat_group or 1 do
local last_group
for _, test in ipairs(tests_list) do
rawset(_G, 'current_test', test)
if last_group ~= test.group then
if last_group then
rawget(_G, 'current_test').value = nil
self:end_group(last_group)
end
self:start_group(test.group)
last_group = test.group
end
rawget(_G, 'current_test').value = test
self:run_test(test)
if self.result.aborted then
break
Expand All @@ -444,6 +446,13 @@ end
function Runner.mt:invoke_test_function(test)
local err = self:protected_call(test.group, test.method, test.name)
self:update_status(test, err)
if not test:is('success') then
if utils.table_len(test.servers) > 0 then
for _, server in pairs(test.servers) do
server:save_artifacts()
end
end
end
end

function Runner.mt:find_test(groups, name)
Expand Down
53 changes: 39 additions & 14 deletions luatest/server.lua
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,27 @@ function Server:new(object, extra)
object = utils.merge(object, extra)
self:inherit(object)
object:initialize()

-- Each method of the server instance will be overridden by a new function
-- in which the association of the current test and server is performed first
-- and then the method itself.
-- It solves the problem when the server is not used in the test (should not
-- save artifacts) and when used.
for k, v in pairs(self) do
if type(v) == 'function' then
object[k] = function(...)
local t = rawget(_G, 'current_test')
if t and t.value then
t = t.value
if not object.tests[t.name] then
object.tests[t.name] = t
t.servers[object.id] = object
end
end
return v(...)
end
end
end
return object
end

Expand Down Expand Up @@ -187,14 +208,12 @@ function Server:initialize()
self.env.LUATEST_LUACOV_ROOT = os.getenv('LUATEST_LUACOV_ROOT')
end

if self.current_test == nil then
self.current_test = rawget(_G, 'current_test')
if self.current_test then
local prefix = fio.pathjoin(Server.vardir, 'artifacts', self.rs_id or '')
self.artifacts = fio.pathjoin(prefix, self.id)
self.current_test:add_server_artifacts(self.alias, self.artifacts)
end
if not self.tests then
self.tests = {}
end

local prefix = fio.pathjoin(Server.vardir, 'artifacts', self.rs_id or '')
self.artifacts = fio.pathjoin(prefix, self.id)
end

-- Create a table with env variables based on the constructor params.
Expand Down Expand Up @@ -329,19 +348,31 @@ function Server:restart(params, opts)
log.debug('Restarted server PID: ' .. self.process.pid)
end

-- Save server artifacts by copying the working directory.
-- Throws an error when the copying is not successful.
function Server:save_artifacts()
local ok, err = fio.copytree(self.workdir, self.artifacts)
if not ok then
error(('Failed to copy artifacts for server (alias: %s, workdir: %s, pid: %d): %s')
:format(self.alias, fio.basename(self.workdir), self.process.pid, err))
end
end

-- Wait until the given condition is `true` (anything except `false` and `nil`).
-- Throws an error when the server process is terminated or timeout exceeds.
local function wait_for_condition(cond_desc, server, func, ...)
local deadline = clock.time() + WAIT_TIMEOUT
while true do
if not server.process:is_alive() then
server:save_artifacts()
error(('Process is terminated when waiting for "%s" condition for server (alias: %s, workdir: %s, pid: %d)')
:format(cond_desc, server.alias, fio.basename(server.workdir), server.process.pid))
end
if func(...) then
return
end
if clock.time() > deadline then
server:save_artifacts()
error(('Timed out to wait for "%s" condition for server (alias: %s, workdir: %s, pid: %d) within %ds')
:format(cond_desc, server.alias, fio.basename(server.workdir), server.process.pid, WAIT_TIMEOUT))
end
Expand Down Expand Up @@ -387,13 +418,7 @@ end
--- Stop the server and clean its working directory.
function Server:drop()
self:stop()

if self.current_test and not self.current_test:is('success') then
local ok, err = fio.copytree(self.workdir, self.artifacts)
if not ok then
error(('Failed to copy server artifacts: %s'):format(err))
end
end
self:save_artifacts()

fio.rmtree(self.workdir)

Expand Down
10 changes: 1 addition & 9 deletions luatest/test_instance.lua
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ end
-- default constructor, test are PASS by default
function TestInstance.mt:initialize()
self.status = 'success'
self.artifacts = nil
self.servers = {}
end

function TestInstance.mt:update_status(status, message, trace)
Expand All @@ -25,14 +25,6 @@ function TestInstance.mt:update_status(status, message, trace)
self.trace = trace
end

function TestInstance.mt:add_server_artifacts(alias, workdir)
local server_workdir = string.format('\n\t%s -> %s', alias, workdir)
if not self.artifacts then
self.artifacts = 'artifacts:'
end
self.artifacts = self.artifacts .. server_workdir
end

function TestInstance.mt:is(status)
return self.status == status
end
Expand Down
63 changes: 63 additions & 0 deletions test/artifacts/common_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
local t = require('luatest')
local utils = require('luatest.utils')

local g = t.group()
local Server = t.Server

g.public = Server:new({ alias = 'public'})
g.public:start()

g.test_servers_not_added_if_they_are_not_used = function()
end

g.after_test('test_servers_not_added_if_they_are_not_used', function()
t.fail_if(
utils.table_len(rawget(_G, 'current_test').value.servers) ~= 0,
'Test instance should not contain a servers')
end)

g.test_only_public_server_has_been_added = function()
g.public:get_vclock()
end

g.after_test('test_only_public_server_has_been_added', function()
t.fail_if(
rawget(_G, 'current_test').value.servers[g.public.id] == nil,
'Test should contain only public server')
end)


g.test_only_private_server_has_been_added = function()
g.private = Server:new({alias = 'private'})
g.private:start()
end

g.after_test('test_only_private_server_has_been_added', function()
t.fail_if(
rawget(_G, 'current_test').value.servers[g.private.id] == nil,
'Test should contain only private server')

end)

g.before_test('test_add_server_from_test_hooks', function()
g.before = Server:new({ alias = 'before' })
g.before:start()
end)

g.test_add_server_from_test_hooks = function()
end

g.after_test('test_add_server_from_test_hooks', function()
g.after = Server:new({ alias = 'after' })
g.after:start()

local test_servers = rawget(_G, 'current_test').value.servers

t.fail_if(
utils.table_len(test_servers) ~= 2,
'Test should contain two servers (from before/after hooks)')
t.fail_if(
test_servers[g.before.id] == nil or
test_servers[g.after.id] == nil,
'Test should contain only `before` and `after` servers')
end)
27 changes: 27 additions & 0 deletions test/artifacts/end_group_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
local t = require('luatest')
local utils = require('luatest.utils')

local Server = t.Server

local g = t.group()

g.test_foo = function()
g.foo_test = rawget(_G, 'current_test').value
end

g.test_bar = function()
g.bar_test = rawget(_G, 'current_test').value
end

g.after_all(function()
g.s = Server:new()
g.s:start()

t.fail_if(
utils.table_len(g.foo_test.servers) ~= 0,
'Test instance `foo` should not contain a servers')

t.fail_if(
utils.table_len(g.bar_test.servers) ~= 0,
'Test instance `bar` should not contain a servers')
end)
71 changes: 71 additions & 0 deletions test/artifacts/hooks_test.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
local t = require('luatest')
local utils = require('luatest.utils')
local fio = require('fio')

local g = t.group()
local Server = t.Server

local function is_server_in_test(server, test)
for _, s in pairs(test.servers) do
if server.id == s.id then
return true
end
end
return false
end

g.public = Server:new({alias = 'public'})
g.public:start()

g.before_all(function()
g.all = Server:new({alias = 'all9'})
g.all:start()
end)

g.before_each(function()
g.each = Server:new({alias = 'each'})
g.each:start()
end)

g.before_test('test_association_between_test_and_servers', function()
g.test = Server:new({alias = 'test'})
g.test:start()
end)


g.test_association_between_test_and_servers = function()
g.internal = Server:new({alias = 'internal'})
g.internal:start()

local test = rawget(_G, 'current_test').value

-- test static association
t.assert(is_server_in_test(g.internal, test))
t.assert(is_server_in_test(g.each, test))
t.assert(is_server_in_test(g.test, test))
t.assert_not(is_server_in_test(g.public, test))

g.public:exec(function() return 1 + 1 end)
g.all:exec(function() return 1 + 1 end)

-- test dynamic association
t.assert(is_server_in_test(g.public, test))
t.assert(is_server_in_test(g.all, test))

t.assert(utils.table_len(test.servers) == 5)
end

g.after_test('test_association_between_test_and_servers', function()
g.test:drop()
t.assert(fio.path.exists(g.test.artifacts))
end)

g.after_each(function()
g.each:drop()
t.assert(fio.path.exists(g.each.artifacts))
end)

g.after_all(function()
g.all:drop()
t.assert(fio.path.exists(g.all.artifacts))
end)
Loading

0 comments on commit 251b35f

Please sign in to comment.