fix(test): better management of tmpfiles

Problem:

When tmpdir is local. The returned values from tmpname may already
exist. This can cause problems for tests which pass `create=false` as
they may require the file to not exist yet.

Solution:

When creating tmp names, always remove it to ensure it doesn't exist,
and optionally open it if `create~=false`

Additionally refactor the tmpname code and flattrn some functions into
constants.

Also while debugging this issue. It was discovered that `exec_lua()`
doesn't report error messages properly. This has been fixed.

2 files changed, 54 insertions, 62 deletions

diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua
index 8a2281e2a1..60b2f872fc 100644
--- a/test/functional/testnvim.lua
+++ b/test/functional/testnvim.lua
@@ -920,36 +920,38 @@ function M.exec_lua(code, ...)
     return M.api.nvim_exec_lua(code, { ... })
   end
 
-  assert(session)
+  assert(session, 'no Nvim session')
 
   if not session.exec_lua_setup then
-    M.api.nvim_exec_lua(
-      [[
-      _G.__test_exec_lua = {
-        get_upvalues = loadstring((select(1,...))),
-        set_upvalues = loadstring((select(2,...))),
-        handler = loadstring((select(3,...)))
-      }
-      setmetatable(_G.__test_exec_lua, { __index = _G.__test_exec_lua })
-    ]],
-      { string.dump(get_upvalues), string.dump(set_upvalues), string.dump(exec_lua_handler) }
+    assert(
+      session:request(
+        'nvim_exec_lua',
+        [[
+          _G.__test_exec_lua = {
+            get_upvalues = loadstring((select(1,...))),
+            set_upvalues = loadstring((select(2,...))),
+            handler = loadstring((select(3,...)))
+          }
+          setmetatable(_G.__test_exec_lua, { __index = _G.__test_exec_lua })
+        ]],
+        { string.dump(get_upvalues), string.dump(set_upvalues), string.dump(exec_lua_handler) }
+      )
     )
     session.exec_lua_setup = true
   end
 
+  local stat, rv = session:request(
+    'nvim_exec_lua',
+    'return { _G.__test_exec_lua:handler(...) }',
+    { string.dump(code), get_upvalues(code), ... }
+  )
+
+  if not stat then
+    error(rv[2])
+  end
+
   --- @type any[], table<string,any>
-  local ret, upvalues = unpack(M.api.nvim_exec_lua(
-    [[
-      return {
-        _G.__test_exec_lua:handler(...)
-      }
-    ]],
-    {
-      string.dump(code),
-      get_upvalues(code),
-      ...,
-    }
-  ))
+  local ret, upvalues = unpack(rv)
 
   -- Update upvalues
   if next(upvalues) then
diff --git a/test/testutil.lua b/test/testutil.lua
index a920f658a1..00b30d74d5 100644
--- a/test/testutil.lua
+++ b/test/testutil.lua
@@ -392,9 +392,7 @@ function M.check_logs()
   )
 end
 
-local function sysname()
-  return uv.os_uname().sysname:lower()
-end
+local sysname = uv.os_uname().sysname:lower()
 
 --- @param s 'win'|'mac'|'freebsd'|'openbsd'|'bsd'
 --- @return boolean
@@ -403,48 +401,27 @@ function M.is_os(s)
     error('unknown platform: ' .. tostring(s))
   end
   return not not (
-    (s == 'win' and (sysname():find('windows') or sysname():find('mingw')))
-    or (s == 'mac' and sysname() == 'darwin')
-    or (s == 'freebsd' and sysname() == 'freebsd')
-    or (s == 'openbsd' and sysname() == 'openbsd')
-    or (s == 'bsd' and sysname():find('bsd'))
+    (s == 'win' and (sysname:find('windows') or sysname:find('mingw')))
+    or (s == 'mac' and sysname == 'darwin')
+    or (s == 'freebsd' and sysname == 'freebsd')
+    or (s == 'openbsd' and sysname == 'openbsd')
+    or (s == 'bsd' and sysname:find('bsd'))
   )
 end
 
-local function tmpdir_get()
-  return os.getenv('TMPDIR') and os.getenv('TMPDIR') or os.getenv('TEMP')
-end
-
---- Is temp directory `dir` defined local to the project workspace?
---- @param dir string?
---- @return boolean
-local function tmpdir_is_local(dir)
-  return not not (dir and dir:find('Xtest'))
-end
-
 local tmpname_id = 0
-local tmpdir = tmpdir_get()
+local tmpdir = os.getenv('TMPDIR') or os.getenv('TEMP')
+local tmpdir_is_local = not not (tmpdir and tmpdir:find('Xtest'))
 
---- Generates a unique filepath for use by tests, in a test-specific "…/Xtest_tmpdir/T42.7"
---- directory (which is cleaned up by the test runner), and writes the file unless `create=false`.
----
----@param create? boolean (default true) Write the file.
-function M.tmpname(create)
-  if tmpdir_is_local(tmpdir) then
+local function get_tmpname()
+  if tmpdir_is_local then
     -- Cannot control os.tmpname() dir, so hack our own tmpname() impl.
     tmpname_id = tmpname_id + 1
     -- "…/Xtest_tmpdir/T42.7"
-    local fname = ('%s/%s.%d'):format(tmpdir, (_G._nvim_test_id or 'nvim-test'), tmpname_id)
-    if create ~= false then
-      io.open(fname, 'w'):close()
-    end
-    return fname
+    return ('%s/%s.%d'):format(tmpdir, (_G._nvim_test_id or 'nvim-test'), tmpname_id)
   end
 
   local fname = os.tmpname()
-  if create == false then
-    os.remove(fname)
-  end
 
   if M.is_os('win') and fname:sub(1, 2) == '\\s' then
     -- In Windows tmpname() returns a filename starting with
@@ -454,7 +431,20 @@ function M.tmpname(create)
     -- In OS X /tmp links to /private/tmp
     return '/private' .. fname
   end
+  return fname
+end
 
+--- Generates a unique filepath for use by tests, in a test-specific "…/Xtest_tmpdir/T42.7"
+--- directory (which is cleaned up by the test runner).
+---
+--- @param create? boolean (default true) Create the file.
+--- @return string
+function M.tmpname(create)
+  local fname = get_tmpname()
+  os.remove(fname)
+  if create ~= false then
+    assert(io.open(fname, 'w')):close()
+  end
   return fname
 end
 
@@ -479,11 +469,11 @@ function M.check_cores(app, force) -- luacheck: ignore
   local random_skip = false
   -- Workspace-local $TMPDIR, scrubbed and pattern-escaped.
   -- "./Xtest-tmpdir/" => "Xtest%-tmpdir"
-  local local_tmpdir = (
-    tmpdir_is_local(tmpdir_get())
-      and relpath(tmpdir_get()):gsub('^[ ./]+', ''):gsub('%/+$', ''):gsub('([^%w])', '%%%1')
-    or nil
-  )
+  local local_tmpdir = nil
+  if tmpdir_is_local and tmpdir then
+    local_tmpdir = vim.pesc(relpath(tmpdir):gsub('^[ ./]+', ''):gsub('%/+$', ''))
+  end
+
   local db_cmd --- @type string
   local test_glob_dir = os.getenv('NVIM_TEST_CORE_GLOB_DIRECTORY')
   if test_glob_dir and test_glob_dir ~= '' then