server: fix artifact handling reset on restart

Fix a bug when server initialization didn't reset artifact handling,
which caused stale artifacts to persist after server restarts.

Closes #409

Author: mandesero

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+- Fixed a bug when server initialization didn't reset artifact handling,
+  causing stale artifacts to persist after server restarts (gh-409).
 - Group and suite hooks must now be registered using the call-style
   API. Use:
 

luatest/server.lua

@@ -526,20 +526,29 @@ 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 args = {...}
+    local opts = {}
+    if type(args[#args]) == 'table' and args[#args].save_artifacts ~= nil then
+        opts = table.remove(args)
+    end
     log.info('Wait for %q condition for server %q (pid: %d) within %d sec',
         cond_desc, server.alias, server.process.pid, WAIT_TIMEOUT)
     local deadline = clock.time() + WAIT_TIMEOUT
     while true do
         if not server.process:is_alive() then
-            server:save_artifacts()
+            if opts.save_artifacts ~= false then
+                server:save_artifacts()
+            end
             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
+        if func(unpack(args)) then
             return
         end
         if clock.time() > deadline then
-            server:save_artifacts()
+            if opts.save_artifacts ~= false then
+                server:save_artifacts()
+            end
             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
@@ -563,7 +572,7 @@ function Server:stop()
         self.process:kill()
         local ok, err = pcall(wait_for_condition, 'process is terminated', self, function()
             return not self.process:is_alive()
-        end)
+        end, {save_artifacts = false})
         if not ok and not err:find('Process is terminated when waiting for') then
             error(err)
         end
@@ -593,7 +602,22 @@ end
 -- analysis if the test fails.
 function Server:drop()
     self:stop()
-    self:save_artifacts()
+
+    local current_test = rawget(_G, 'current_test')
+    local should_save = current_test == nil
+
+    if current_test then
+        local status = current_test.status
+        if status == 'success' or status == 'skip' or status == 'xfail' then
+            should_save = false
+        else
+            should_save = true
+        end
+    end
+
+    if should_save then
+        self:save_artifacts()
+    end
 
     self.instance_id = nil
     self.instance_uuid = nil

test/server_test.lua

@@ -645,3 +645,45 @@ g.test_assertion_failure = function()
     server:connect_net_box()
     helper.assert_failure(server.exec, server, function() t.assert(false) end)
 end
+
+g.test_save_artifacts_after_restart_when_test_failed = function()
+    local s = Server:new()
+
+    s:start()
+    s:exec(function()
+        require('log').info('before_restart_artifacts_marker')
+    end)
+
+    s:restart()
+    s:exec(function()
+        require('log').info('after_restart_artifacts_marker')
+    end)
+
+    local test = rawget(_G, 'current_test')
+    test.status = 'fail'
+    s:drop()
+    test.status = 'success'
+
+    local log_path = fio.pathjoin(s.artifacts, s.alias .. '.log')
+    local log_file = fio.open(log_path)
+    local log_stat = log_file:stat()
+    local log_content = log_file:read(log_stat.size)
+    log_file:close()
+
+    t.assert_str_contains(log_content, 'before_restart_artifacts_marker')
+    t.assert_str_contains(log_content, 'after_restart_artifacts_marker')
+end
+
+g.test_do_not_save_server_artifacts_when_test_succeeded = function()
+    local s = Server:new()
+    fio.rmtree(s.artifacts)
+
+    s:start()
+
+    local test = rawget(_G, 'current_test')
+    test.status = 'success'
+
+    s:drop()
+
+    t.assert_equals(fio.path.exists(s.artifacts), false)
+end