test: use spawn_wait() instead of system() #31852

Problem:
Tests that need to check `nvim` CLI behavior (no RPC session) create
their own ad-hoc `system()` wrappers.

Solution:
- Use `n.spawn_wait` instead of `system()`.
- Bonus: this also improves the tests by explicitly checking for
  `stdout` or `stderr`. And if a signal is raised, `ProcStream.status`
  will reflect it.

8 files changed, 78 insertions, 146 deletions

diff --git a/test/client/uv_stream.lua b/test/client/uv_stream.lua
index a9ef2db115..6e1a6995be 100644
--- a/test/client/uv_stream.lua
+++ b/test/client/uv_stream.lua
@@ -126,13 +126,16 @@ end
 --- @field private _child_stdin uv.uv_pipe_t
 --- @field private _child_stdout uv.uv_pipe_t
 --- @field private _child_stderr uv.uv_pipe_t
+--- Collects stdout (if `collect_text=true`). Treats data as text (CRLF converted to LF).
 --- @field stdout string
---- stderr is always collected in this field, regardless of `collect_output`.
+--- Collects stderr as raw data.
 --- @field stderr string
+--- Gets stderr+stdout as text (CRLF converted to LF).
+--- @field output fun(): string
 --- @field stdout_eof boolean
 --- @field stderr_eof boolean
---- Collects stdout in the `stdout` field, and stdout+stderr in `output` field.
---- @field private collect_output boolean
+--- Collects text into the `stdout` field.
+--- @field collect_text boolean
 --- Exit code
 --- @field status integer
 --- @field signal integer
@@ -147,8 +150,13 @@ ProcStream.__index = ProcStream
 --- @return test.ProcStream
 function ProcStream.spawn(argv, env, io_extra)
   local self = setmetatable({
-    collect_output = false,
-    output = '',
+    collect_text = false,
+    output = function(self)
+      if not self.collect_text then
+        error('set collect_text=true')
+      end
+      return (self.stderr .. self.stdout):gsub('\r\n', '\n')
+    end,
     stdout = '',
     stderr = '',
     stdout_eof = false,
@@ -193,13 +201,10 @@ function ProcStream:on_read(stream, cb, err, chunk)
   elseif chunk then
     -- Always collect stderr, in case it gives useful info on failure.
     if stream == 'stderr' then
-      self.stderr = self.stderr .. chunk ---@type string
-    end
-    -- Collect stdout if `collect_output` is enabled.
-    if self.collect_output then
-      self.stdout = self.stdout .. chunk ---@type string
-      --- Collect both stdout + stderr in the `output` field.
-      self.output = self[stream] .. chunk ---@type string
+      self.stderr = self.stderr .. chunk --[[@as string]]
+    elseif stream == 'stdout' and self.collect_text then
+      -- Set `stdout` and convert CRLF => LF.
+      self.stdout = (self.stdout .. chunk):gsub('\r\n', '\n')
     end
   else
     -- stderr_eof/stdout_eof
@@ -214,7 +219,6 @@ end
 
 --- Collects output until the process exits.
 function ProcStream:wait()
-  self.collect_output = true
   while not (self.stdout_eof and self.stderr_eof and (self.status or self.signal)) do
     uv.run('once')
   end
diff --git a/test/functional/core/exit_spec.lua b/test/functional/core/exit_spec.lua
index 34c3eedbd2..65f6bc28a6 100644
--- a/test/functional/core/exit_spec.lua
+++ b/test/functional/core/exit_spec.lua
@@ -8,8 +8,6 @@ local feed = n.feed
 local eval = n.eval
 local eq = t.eq
 local run = n.run
-local fn = n.fn
-local nvim_prog = n.nvim_prog
 local pcall_err = t.pcall_err
 local exec_capture = n.exec_capture
 local poke_eventloop = n.poke_eventloop
@@ -69,8 +67,8 @@ describe(':cquit', function()
       poke_eventloop()
       assert_alive()
     else
-      fn.system({ nvim_prog, '-u', 'NONE', '-i', 'NONE', '--headless', '--cmd', cmdline })
-      eq(exit_code, eval('v:shell_error'))
+      local p = n.spawn_wait('--cmd', cmdline)
+      eq(exit_code, p.status)
     end
   end
 
diff --git a/test/functional/core/main_spec.lua b/test/functional/core/main_spec.lua
index ce4ba1905f..6add49ceae 100644
--- a/test/functional/core/main_spec.lua
+++ b/test/functional/core/main_spec.lua
@@ -9,7 +9,6 @@ local feed = n.feed
 local eval = n.eval
 local clear = n.clear
 local fn = n.fn
-local nvim_prog_abs = n.nvim_prog_abs
 local write_file = t.write_file
 local is_os = t.is_os
 local skip = t.skip
@@ -35,7 +34,7 @@ describe('command-line option', function()
     it('treats - as stdin', function()
       eq(nil, uv.fs_stat(fname))
       fn.system({
-        nvim_prog_abs(),
+        n.nvim_prog,
         '-u',
         'NONE',
         '-i',
@@ -56,41 +55,29 @@ describe('command-line option', function()
       eq(nil, uv.fs_stat(fname))
       eq(true, not not dollar_fname:find('%$%w+'))
       write_file(dollar_fname, ':call setline(1, "100500")\n:wqall!\n')
-      fn.system({
-        nvim_prog_abs(),
-        '-u',
-        'NONE',
-        '-i',
-        'NONE',
-        '--headless',
+      local p = n.spawn_wait(
         '--cmd',
         'set noswapfile shortmess+=IFW fileformats=unix',
         '-s',
         dollar_fname,
-        fname,
-      })
-      eq(0, eval('v:shell_error'))
+        fname
+      )
+      eq(0, p.status)
       local attrs = uv.fs_stat(fname)
       eq(#'100500\n', attrs.size)
     end)
 
-    it('does not crash when run completion in ex mode', function()
-      fn.system({
-        nvim_prog_abs(),
-        '--clean',
-        '-e',
-        '-s',
-        '--cmd',
-        'exe "norm! i\\<C-X>\\<C-V>"',
-      })
-      eq(0, eval('v:shell_error'))
+    it('does not crash when run completion in Ex mode', function()
+      local p =
+        n.spawn_wait('--clean', '-e', '-s', '--cmd', 'exe "norm! i\\<C-X>\\<C-V>"', '--cmd', 'qa!')
+      eq(0, p.status)
     end)
 
     it('does not crash after reading from stdin in non-headless mode', function()
       skip(is_os('win'))
       local screen = Screen.new(40, 8)
       local args = {
-        nvim_prog_abs(),
+        n.nvim_prog,
         '-u',
         'NONE',
         '-i',
@@ -138,51 +125,40 @@ describe('command-line option', function()
       ]=]
     end)
 
-    it('errors out when trying to use nonexistent file with -s', function()
+    it('fails when trying to use nonexistent file with -s', function()
+      local p = n.spawn_wait(
+        '--cmd',
+        'set noswapfile shortmess+=IFW fileformats=unix',
+        '--cmd',
+        'language C',
+        '-s',
+        nonexistent_fname
+      )
       eq(
         'Cannot open for reading: "' .. nonexistent_fname .. '": no such file or directory\n',
-        fn.system({
-          nvim_prog_abs(),
-          '-u',
-          'NONE',
-          '-i',
-          'NONE',
-          '--headless',
-          '--cmd',
-          'set noswapfile shortmess+=IFW fileformats=unix',
-          '--cmd',
-          'language C',
-          '-s',
-          nonexistent_fname,
-        })
+        --- TODO(justinmk): using `p.output` because Nvim emits CRLF even on non-Win. Emit LF instead?
+        p:output()
       )
-      eq(2, eval('v:shell_error'))
+      eq(2, p.status)
     end)
 
     it('errors out when trying to use -s twice', function()
       write_file(fname, ':call setline(1, "1")\n:wqall!\n')
       write_file(dollar_fname, ':call setline(1, "2")\n:wqall!\n')
-      eq(
-        'Attempt to open script file again: "-s ' .. dollar_fname .. '"\n',
-        fn.system({
-          nvim_prog_abs(),
-          '-u',
-          'NONE',
-          '-i',
-          'NONE',
-          '--headless',
-          '--cmd',
-          'set noswapfile shortmess+=IFW fileformats=unix',
-          '--cmd',
-          'language C',
-          '-s',
-          fname,
-          '-s',
-          dollar_fname,
-          fname_2,
-        })
+      local p = n.spawn_wait(
+        '--cmd',
+        'set noswapfile shortmess+=IFW fileformats=unix',
+        '--cmd',
+        'language C',
+        '-s',
+        fname,
+        '-s',
+        dollar_fname,
+        fname_2
       )
-      eq(2, eval('v:shell_error'))
+      --- TODO(justinmk): using `p.output` because Nvim emits CRLF even on non-Win. Emit LF instead?
+      eq('Attempt to open script file again: "-s ' .. dollar_fname .. '"\n', p:output())
+      eq(2, p.status)
       eq(nil, uv.fs_stat(fname_2))
     end)
   end)
@@ -190,8 +166,8 @@ describe('command-line option', function()
   it('nvim -v, :version', function()
     matches('Run ":verbose version"', fn.execute(':version'))
     matches('fall%-back for %$VIM: .*Run :checkhealth', fn.execute(':verbose version'))
-    matches('Run "nvim %-V1 %-v"', fn.system({ nvim_prog_abs(), '-v' }))
-    matches('fall%-back for %$VIM: .*Run :checkhealth', fn.system({ nvim_prog_abs(), '-V1', '-v' }))
+    matches('Run "nvim %-V1 %-v"', n.spawn_wait('-v').stdout)
+    matches('fall%-back for %$VIM: .*Run :checkhealth', n.spawn_wait('-V1', '-v').stdout)
   end)
 
   if is_os('win') then
@@ -205,7 +181,7 @@ describe('command-line option', function()
         eq(
           'some text',
           fn.system({
-            nvim_prog_abs(),
+            n.nvim_prog,
             '-es',
             '+%print',
             '+q',
diff --git a/test/functional/core/startup_spec.lua b/test/functional/core/startup_spec.lua
index c9ea280646..642e2b0287 100644
--- a/test/functional/core/startup_spec.lua
+++ b/test/functional/core/startup_spec.lua
@@ -77,22 +77,9 @@ describe('startup', function()
   end)
 
   it('--startuptime does not crash on error #31125', function()
-    eq(
-      "E484: Can't open file .",
-      fn.system({
-        nvim_prog,
-        '-u',
-        'NONE',
-        '-i',
-        'NONE',
-        '--headless',
-        '--startuptime',
-        '.',
-        '-c',
-        '42cquit',
-      })
-    )
-    eq(42, api.nvim_get_vvar('shell_error'))
+    local p = n.spawn_wait('--startuptime', '.', '-c', '42cquit')
+    eq("E484: Can't open file .", p.stderr)
+    eq(42, p.status)
   end)
 
   it('-D does not hang #12647', function()
@@ -184,7 +171,7 @@ describe('startup', function()
 
     it('Lua-error sets Nvim exitcode', function()
       local proc = n.spawn_wait('-l', 'test/functional/fixtures/startup-fail.lua')
-      matches('E5113: .* my pearls!!', proc.output)
+      matches('E5113: .* my pearls!!', proc:output())
       eq(1, proc.status)
 
       eq(0, eval('v:shell_error'))
@@ -606,15 +593,15 @@ describe('startup', function()
   it('fails on --embed with -es/-Es/-l', function()
     matches(
       'nvim[.exe]*: %-%-embed conflicts with %-es/%-Es/%-l',
-      fn.system({ nvim_prog, '--embed', '-es' })
+      n.spawn_wait('--embed', '-es').stderr
     )
     matches(
       'nvim[.exe]*: %-%-embed conflicts with %-es/%-Es/%-l',
-      fn.system({ nvim_prog, '--embed', '-Es' })
+      n.spawn_wait('--embed', '-Es').stderr
     )
     matches(
       'nvim[.exe]*: %-%-embed conflicts with %-es/%-Es/%-l',
-      fn.system({ nvim_prog, '--embed', '-l', 'foo.lua' })
+      n.spawn_wait('--embed', '-l', 'foo.lua').stderr
     )
   end)
 
@@ -698,20 +685,8 @@ describe('startup', function()
   end)
 
   it('get command line arguments from v:argv', function()
-    local out = fn.system({
-      nvim_prog,
-      '-u',
-      'NONE',
-      '-i',
-      'NONE',
-      '--headless',
-      '--cmd',
-      nvim_set,
-      '-c',
-      [[echo v:argv[-1:] len(v:argv) > 1]],
-      '+q',
-    })
-    eq("['+q'] 1", out)
+    local p = n.spawn_wait('--cmd', nvim_set, '-c', [[echo v:argv[-1:] len(v:argv) > 1]], '+q')
+    eq("['+q'] 1", p.stderr)
   end)
 end)
 
diff --git a/test/functional/lua/ui_event_spec.lua b/test/functional/lua/ui_event_spec.lua
index 27640d6066..af6b2ceac3 100644
--- a/test/functional/lua/ui_event_spec.lua
+++ b/test/functional/lua/ui_event_spec.lua
@@ -106,20 +106,15 @@ describe('vim.ui_attach', function()
   end)
 
   it('does not crash on exit', function()
-    fn.system({
-      n.nvim_prog,
-      '-u',
-      'NONE',
-      '-i',
-      'NONE',
+    local p = n.spawn_wait(
       '--cmd',
       [[ lua ns = vim.api.nvim_create_namespace 'testspace' ]],
       '--cmd',
       [[ lua vim.ui_attach(ns, {ext_popupmenu=true}, function() end) ]],
       '--cmd',
-      'quitall!',
-    })
-    eq(0, n.eval('v:shell_error'))
+      'quitall!'
+    )
+    eq(0, p.status)
   end)
 
   it('can receive accurate message kinds even if they are history', function()
diff --git a/test/functional/options/defaults_spec.lua b/test/functional/options/defaults_spec.lua
index e7f47ef4e9..a82279e775 100644
--- a/test/functional/options/defaults_spec.lua
+++ b/test/functional/options/defaults_spec.lua
@@ -14,7 +14,6 @@ local api = n.api
 local command = n.command
 local clear = n.clear
 local exc_exec = n.exc_exec
-local exec_lua = n.exec_lua
 local eval = n.eval
 local eq = t.eq
 local ok = t.ok
@@ -929,17 +928,12 @@ describe('stdpath()', function()
     assert_alive() -- Check for crash. #8393
 
     -- Check that Nvim rejects invalid APPNAMEs
-    -- Call jobstart() and jobwait() in the same RPC request to reduce flakiness.
     local function test_appname(testAppname, expected_exitcode)
-      local lua_code = string.format(
-        [[
-        local child = vim.fn.jobstart({ vim.v.progpath, '--clean', '--headless', '--listen', 'x', '+qall!' }, { env = { NVIM_APPNAME = %q } })
-        return vim.fn.jobwait({ child }, %d)[1]
-      ]],
-        testAppname,
-        3000
-      )
-      eq(expected_exitcode, exec_lua(lua_code))
+      local p = n.spawn_wait({
+        args = { '--listen', 'x', '+qall!' },
+        env = { NVIM_APPNAME = testAppname },
+      })
+      eq(expected_exitcode, p.status)
     end
     -- Invalid appnames:
     test_appname('a/../b', 1)
diff --git a/test/functional/terminal/highlight_spec.lua b/test/functional/terminal/highlight_spec.lua
index c43d139f70..0afbd010f7 100644
--- a/test/functional/terminal/highlight_spec.lua
+++ b/test/functional/terminal/highlight_spec.lua
@@ -6,7 +6,6 @@ local tt = require('test.functional.testterm')
 local feed, clear = n.feed, n.clear
 local api = n.api
 local testprg, command = n.testprg, n.command
-local nvim_prog_abs = n.nvim_prog_abs
 local fn = n.fn
 local nvim_set = n.nvim_set
 local is_os = t.is_os
@@ -151,7 +150,7 @@ it(':terminal highlight has lower precedence than editor #9964', function()
   })
   -- Child nvim process in :terminal (with cterm colors).
   fn.jobstart({
-    nvim_prog_abs(),
+    n.nvim_prog,
     '-n',
     '-u',
     'NORC',
diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua
index c8f1c35555..aa66e07ad0 100644
--- a/test/functional/testnvim.lua
+++ b/test/functional/testnvim.lua
@@ -318,16 +318,6 @@ function M.stop()
   assert(session):stop()
 end
 
-function M.nvim_prog_abs()
-  -- system(['build/bin/nvim']) does not work for whatever reason. It must
-  -- be executable searched in $PATH or something starting with / or ./.
-  if M.nvim_prog:match('[/\\]') then
-    return M.request('nvim_call_function', 'fnamemodify', { M.nvim_prog, ':p' })
-  else
-    return M.nvim_prog
-  end
-end
-
 -- Use for commands which expect nvim to quit.
 -- The first argument can also be a timeout.
 function M.expect_exit(fn_or_timeout, ...)
@@ -526,7 +516,7 @@ function M.spawn_argv(keep, ...)
   return M.spawn(argv, nil, env, keep, io_extra)
 end
 
---- Starts a (`--headless`, non-RPC) Nvim process, waits for exit, and returns output + info.
+--- Starts a (non-RPC, `--headless --listen "Tx"`) Nvim process, waits for exit, and returns result.
 ---
 --- @param ... string Nvim CLI args
 --- @return test.ProcStream
@@ -537,6 +527,7 @@ function M.spawn_wait(...)
   table.insert(opts.args_rm, '--embed')
   local argv, env, io_extra = M.new_argv(opts)
   local proc = ProcStream.spawn(argv, env, io_extra)
+  proc.collect_text = true
   proc:read_start()
   proc:wait()
   proc:close()