From 9c7710ed09006d11a90aea1f08e734b0f87355b3 Mon Sep 17 00:00:00 2001
From: Oleg Chaplashkin <ochaplashkin@tarantool.org>
Date: Wed, 27 Sep 2023 00:30:22 +0400
Subject: [PATCH] Save artifacts only once

Since commit [1] the `Server:save_artifacts()` function was called more
than once. Due to this fact we could overwrite already saved artifacts.
Added a flag that will ensure that the saving will be executed only
once.

There was also a problem when copying artifacts was not performed but
the path to the artifacts was formed as a directory with artifacts:

    artifacts:
        server -> /tmp/t/artifacts/server-XXX

And if we tried to look at these artifacts, we could see this:

    $ ls -la /tmp/t/artifacts/server-XXX
    ls: cannot access '/tmp/t/artifacts/server-XXX': No such file or
    directory

To show explicitly that the saving failed with an error, the following
string will now be written to the artifacts:

    artifacts:
        server -> Failed to copy artifacts for server (alias: server-XXX
        workdir: /tmp/t/artifacts/server-XXX)

[1] tarantool/luatest@251b35f

Part of #304
---
 luatest/runner.lua | 12 +++++-------
 luatest/server.lua | 13 ++++++++++---
 2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/luatest/runner.lua b/luatest/runner.lua
index b855645..e4ce6b1 100644
--- a/luatest/runner.lua
+++ b/luatest/runner.lua
@@ -338,6 +338,11 @@ function Runner.mt:update_status(node, err)
     elseif err.status == 'fail' or err.status == 'error' or err.status == 'skip'
         or err.status == 'xfail' or err.status == 'xsuccess' then
         node:update_status(err.status, err.message, err.trace)
+        if utils.table_len(node.servers) > 0 then
+            for _, server in pairs(node.servers) do
+                server:save_artifacts()
+            end
+        end
     else
         error('No such status: ' .. pp.tostring(err.status))
     end
@@ -458,13 +463,6 @@ 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)
diff --git a/luatest/server.lua b/luatest/server.lua
index b82c556..03e36b3 100644
--- a/luatest/server.lua
+++ b/luatest/server.lua
@@ -372,13 +372,20 @@ function Server:restart(params, opts)
 end
 
 -- Save server artifacts by copying the working directory.
--- Throws an error when the copying is not successful.
+-- The save logic will only work once to avoid overwriting the artifacts directory.
+-- If an error occurred, then the server artifacts path will be replaced by the
+-- following string: `Failed to copy artifacts for server (alias: <alias>, workdir: <workdir>)`.
 function Server:save_artifacts()
+    if self.artifacts_saved then
+        return
+    end
     local ok, err = fio.copytree(self.workdir, self.artifacts)
     if not ok then
-        log.error(('Failed to copy artifacts for server (alias: %s, workdir: %s): %s')
-            :format(self.alias, fio.basename(self.workdir), err))
+        self.artifacts = ('Failed to copy artifacts for server (alias: %s, workdir: %s)')
+            :format(self.alias, fio.basename(self.workdir))
+        log.error(('%s: %s'):format(self.artifacts, err))
     end
+    self.artifacts_saved = true
 end
 
 -- Wait until the given condition is `true` (anything except `false` and `nil`).