refactor(tests): merge n.spawn/n.spawn_argv into n.new_session #31859

Problem:
- `n.spawn()` is misleading because it also connects RPC, it's not just
  "spawning" a process.
- It's confusing that `n.spawn()` and `n.spawn_argv()` are separate.

Solution:
- Replace `n.spawn()`/`n.spawn_argv()` with a single function `n.new_session()`.
  This name aligns with the existing functions `n.set_session`/`n.get_session`.
    - Note: removes direct handling of `prepend_argv`, but I doubt that was
      important or intentional. If callers want to control use of `prepend_argv`
      then we should add a new flag to `test.session.Opts`.
- Move `keep` to first parameter of `n.new_session()`.
- Add a `merge` flag to `test.session.Opts`
- Mark `_new_argv()` as private. Test should use clear/new_session/spawn_wait
  instead.

test/functional/api/server_requests_spec.lua

@@ -9,7 +9,6 @@ local nvim_prog, command, fn = n.nvim_prog, n.command, n.fn
 local source, next_msg = n.source, n.next_msg
 local ok = t.ok
 local api = n.api
-local spawn, merge_args = n.spawn, n.merge_args
 local set_session = n.set_session
 local pcall_err = t.pcall_err
 local assert_alive = n.assert_alive
@@ -281,10 +280,9 @@ describe('server -> client', function()
   end)
 
   describe('connecting to another (peer) nvim', function()
-    local nvim_argv = merge_args(n.nvim_argv, { '--headless' })
     local function connect_test(server, mode, address)
       local serverpid = fn.getpid()
-      local client = spawn(nvim_argv, false, nil, true)
+      local client = n.new_session(true)
       set_session(client)
 
       local clientpid = fn.getpid()
@@ -312,7 +310,7 @@ describe('server -> client', function()
     end
 
     it('via named pipe', function()
-      local server = spawn(nvim_argv)
+      local server = n.new_session(false)
       set_session(server)
       local address = fn.serverlist()[1]
       local first = string.sub(address, 1, 1)
@@ -321,7 +319,7 @@ describe('server -> client', function()
     end)
 
     it('via ipv4 address', function()
-      local server = spawn(nvim_argv)
+      local server = n.new_session(false)
       set_session(server)
       local status, address = pcall(fn.serverstart, '127.0.0.1:')
       if not status then
@@ -332,7 +330,7 @@ describe('server -> client', function()
     end)
 
     it('via ipv6 address', function()
-      local server = spawn(nvim_argv)
+      local server = n.new_session(false)
       set_session(server)
       local status, address = pcall(fn.serverstart, '::1:')
       if not status then
@@ -343,7 +341,7 @@ describe('server -> client', function()
     end)
 
     it('via hostname', function()
-      local server = spawn(nvim_argv)
+      local server = n.new_session(false)
       set_session(server)
       local address = fn.serverstart('localhost:')
       eq('localhost:', string.sub(address, 1, 10))
@@ -351,10 +349,10 @@ describe('server -> client', function()
     end)
 
     it('does not crash on receiving UI events', function()
-      local server = spawn(nvim_argv)
+      local server = n.new_session(false)
       set_session(server)
       local address = fn.serverlist()[1]
-      local client = spawn(nvim_argv, false, nil, true)
+      local client = n.new_session(true)
       set_session(client)
 
       local id = fn.sockconnect('pipe', address, { rpc = true })

test/functional/core/channels_spec.lua

@@ -5,7 +5,6 @@ local clear, eq, eval, next_msg, ok, source = n.clear, t.eq, n.eval, n.next_msg,
 local command, fn, api = n.command, n.fn, n.api
 local matches = t.matches
 local sleep = vim.uv.sleep
-local spawn, nvim_argv = n.spawn, n.nvim_argv
 local get_session, set_session = n.get_session, n.set_session
 local nvim_prog = n.nvim_prog
 local is_os = t.is_os
@@ -33,10 +32,10 @@ describe('channels', function()
   end)
 
   pending('can connect to socket', function()
-    local server = spawn(nvim_argv, nil, nil, true)
+    local server = n.new_session(true)
     set_session(server)
     local address = fn.serverlist()[1]
-    local client = spawn(nvim_argv, nil, nil, true)
+    local client = n.new_session(true)
     set_session(client)
     source(init)
 
@@ -63,7 +62,7 @@ describe('channels', function()
 
   it('dont crash due to garbage in rpc #23781', function()
     local client = get_session()
-    local server = spawn(nvim_argv, nil, nil, true)
+    local server = n.new_session(true)
     set_session(server)
     local address = fn.serverlist()[1]
     set_session(client)

test/functional/core/fileio_spec.lua

@@ -31,7 +31,6 @@ local feed_command = n.feed_command
 local skip = t.skip
 local is_os = t.is_os
 local is_ci = t.is_ci
-local spawn = n.spawn
 local set_session = n.set_session
 
 describe('fileio', function()
@@ -51,12 +50,11 @@ describe('fileio', function()
     rmdir('Xtest_backupdir with spaces')
   end)
 
-  local args = { nvim_prog, '--clean', '--cmd', 'set nofsync directory=Xtest_startup_swapdir' }
+  local args = { '--clean', '--cmd', 'set nofsync directory=Xtest_startup_swapdir' }
   --- Starts a new nvim session and returns an attached screen.
-  local function startup(extra_args)
-    extra_args = extra_args or {}
-    local argv = vim.iter({ args, '--embed', extra_args }):flatten():totable()
-    local screen_nvim = spawn(argv)
+  local function startup()
+    local argv = vim.iter({ args, '--embed' }):flatten():totable()
+    local screen_nvim = n.new_session(false, { args = argv, merge = false })
     set_session(screen_nvim)
     local screen = Screen.new(70, 10)
     screen:set_default_attr_ids({
@@ -100,7 +98,8 @@ describe('fileio', function()
     eq('foozubbaz', trim(read_file('Xtest_startup_file1')))
 
     -- 4. Exit caused by deadly signal (+ 'swapfile').
-    local j = fn.jobstart(vim.iter({ args, '--embed' }):flatten():totable(), { rpc = true })
+    local j =
+      fn.jobstart(vim.iter({ nvim_prog, args, '--embed' }):flatten():totable(), { rpc = true })
     fn.rpcrequest(
       j,
       'nvim_exec2',

test/functional/core/remote_spec.lua

@@ -10,10 +10,8 @@ local expect = n.expect
 local fn = n.fn
 local insert = n.insert
 local nvim_prog = n.nvim_prog
-local new_argv = n.new_argv
 local neq = t.neq
 local set_session = n.set_session
-local spawn = n.spawn
 local tmpname = t.tmpname
 local write_file = t.write_file
 
@@ -32,8 +30,7 @@ describe('Remote', function()
   describe('connect to server and', function()
     local server
     before_each(function()
-      server = spawn(new_argv(), true)
-      set_session(server)
+      server = n.clear()
     end)
 
     after_each(function()
@@ -49,7 +46,7 @@ describe('Remote', function()
       -- to wait for the remote instance to exit and calling jobwait blocks
       -- the event loop. If the server event loop is blocked, it can't process
       -- our incoming --remote calls.
-      local client_starter = spawn(new_argv(), false, nil, true)
+      local client_starter = n.new_session(true)
       set_session(client_starter)
       -- Call jobstart() and jobwait() in the same RPC request to reduce flakiness.
       eq(
@@ -144,15 +141,8 @@ describe('Remote', function()
 
   describe('exits with error on', function()
     local function run_and_check_exit_code(...)
-      local bogus_argv = new_argv(...)
-
-      -- Create an nvim instance just to run the remote-invoking nvim. We want
-      -- to wait for the remote instance to exit and calling jobwait blocks
-      -- the event loop. If the server event loop is blocked, it can't process
-      -- our incoming --remote calls.
-      clear()
-      -- Call jobstart() and jobwait() in the same RPC request to reduce flakiness.
-      eq({ 2 }, exec_lua([[return vim.fn.jobwait({ vim.fn.jobstart(...) })]], bogus_argv))
+      local p = n.spawn_wait { args = { ... } }
+      eq(2, p.status)
     end
     it('bogus subcommand', function()
       run_and_check_exit_code('--remote-bogus')

test/functional/ex_cmds/swapfile_preserve_recover_spec.lua

@@ -12,12 +12,10 @@ local fn = n.fn
 local nvim_prog = n.nvim_prog
 local ok = t.ok
 local rmdir = n.rmdir
-local new_argv = n.new_argv
 local new_pipename = n.new_pipename
 local pesc = vim.pesc
 local os_kill = n.os_kill
 local set_session = n.set_session
-local spawn = n.spawn
 local async_meths = n.async_meths
 local expect_msg_seq = n.expect_msg_seq
 local pcall_err = t.pcall_err
@@ -56,7 +54,7 @@ describe("preserve and (R)ecover with custom 'directory'", function()
 
   local nvim0
   before_each(function()
-    nvim0 = spawn(new_argv())
+    nvim0 = n.new_session(false)
     set_session(nvim0)
     rmdir(swapdir)
     mkdir(swapdir)
@@ -76,7 +74,8 @@ describe("preserve and (R)ecover with custom 'directory'", function()
 
   local function test_recover(swappath1)
     -- Start another Nvim instance.
-    local nvim2 = spawn({ nvim_prog, '-u', 'NONE', '-i', 'NONE', '--embed' }, true)
+    local nvim2 =
+      n.new_session(false, { args = { '-u', 'NONE', '-i', 'NONE', '--embed' }, merge = false })
     set_session(nvim2)
 
     exec(init)
@@ -141,7 +140,7 @@ describe('swapfile detection', function()
     set swapfile fileformat=unix nomodified undolevels=-1 nohidden
   ]]
   before_each(function()
-    nvim0 = spawn(new_argv())
+    nvim0 = n.new_session(false)
     set_session(nvim0)
     rmdir(swapdir)
     mkdir(swapdir)
@@ -168,7 +167,8 @@ describe('swapfile detection', function()
     command('preserve')
 
     -- Start another Nvim instance.
-    local nvim2 = spawn({ nvim_prog, '-u', 'NONE', '-i', 'NONE', '--embed' }, true, nil, true)
+    local nvim2 =
+      n.new_session(true, { args = { '-u', 'NONE', '-i', 'NONE', '--embed' }, merge = false })
     set_session(nvim2)
     local screen2 = Screen.new(256, 40)
     screen2._default_attr_ids = nil
@@ -251,7 +251,7 @@ describe('swapfile detection', function()
     command('preserve') -- Make sure the swap file exists.
     local nvimpid = fn.getpid()
 
-    local nvim1 = spawn(new_argv(), true, nil, true)
+    local nvim1 = n.new_session(true)
     set_session(nvim1)
     local screen = Screen.new(75, 18)
     exec(init)
@@ -273,7 +273,7 @@ describe('swapfile detection', function()
       [1] = { bold = true, foreground = Screen.colors.SeaGreen }, -- MoreMsg
     })
 
-    local nvim1 = spawn(new_argv(), true, nil, true)
+    local nvim1 = n.new_session(true)
     set_session(nvim1)
     screen:attach()
     exec(init)
@@ -292,7 +292,7 @@ describe('swapfile detection', function()
     ]])
     nvim1:close()
 
-    local nvim2 = spawn(new_argv(), true, nil, true)
+    local nvim2 = n.new_session(true)
     set_session(nvim2)
     screen:attach()
     exec(init)

test/functional/ex_cmds/wundo_spec.lua

@@ -5,8 +5,6 @@ local n = require('test.functional.testnvim')()
 local command = n.command
 local clear = n.clear
 local eval = n.eval
-local spawn = n.spawn
-local nvim_prog = n.nvim_prog
 local set_session = n.set_session
 
 describe(':wundo', function()
@@ -24,15 +22,11 @@ end)
 
 describe('u_* functions', function()
   it('safely fail on new, non-empty buffer', function()
-    local session = spawn({
-      nvim_prog,
-      '-u',
-      'NONE',
-      '-i',
-      'NONE',
-      '--embed',
-      '-c',
-      'set undodir=. undofile',
+    local session = n.new_session(false, {
+      args = {
+        '-c',
+        'set undodir=. undofile',
+      },
     })
     set_session(session)
     command('echo "True"') -- Should not error out due to crashed Neovim

test/functional/shada/marks_spec.lua

@@ -218,7 +218,7 @@ describe('ShaDa support code', function()
   -- -c temporary sets lnum to zero to make `+/pat` work, so calling setpcmark()
   -- during -c used to add item with zero lnum to jump list.
   it('does not create incorrect file for non-existent buffers when writing from -c', function()
-    local argv = n.new_argv {
+    local p = n.spawn_wait {
       args_rm = {
         '-i',
         '--embed', -- no --embed
@@ -232,12 +232,13 @@ describe('ShaDa support code', function()
         'qall',
       },
     }
-    eq('', fn.system(argv))
+    eq('', p:output())
+    eq(0, p.status)
     eq(0, exc_exec('rshada'))
   end)
 
   it('does not create incorrect file for non-existent buffers opened from -c', function()
-    local argv = n.new_argv {
+    local p = n.spawn_wait {
       args_rm = {
         '-i',
         '--embed', -- no --embed
@@ -251,7 +252,8 @@ describe('ShaDa support code', function()
         'autocmd VimEnter * qall',
       },
     }
-    eq('', fn.system(argv))
+    eq('', p:output())
+    eq(0, p.status)
     eq(0, exc_exec('rshada'))
   end)
 

test/functional/shada/shada_spec.lua

@@ -6,8 +6,7 @@ local uv = vim.uv
 local paths = t.paths
 
 local api, nvim_command, fn, eq = n.api, n.command, n.fn, t.eq
-local write_file, spawn, set_session, nvim_prog, exc_exec =
-  t.write_file, n.spawn, n.set_session, n.nvim_prog, n.exc_exec
+local write_file, set_session, exc_exec = t.write_file, n.set_session, n.exc_exec
 local is_os = t.is_os
 local skip = t.skip
 
@@ -254,7 +253,7 @@ describe('ShaDa support code', function()
   it('does not crash when ShaDa file directory is not writable', function()
     skip(is_os('win'))
 
-    fn.mkdir(dirname, '', 0)
+    fn.mkdir(dirname, '', '0')
     eq(0, fn.filewritable(dirname))
     reset { shadafile = dirshada, args = { '--cmd', 'set shada=' } }
     api.nvim_set_option_value('shada', "'10", {})
@@ -270,10 +269,10 @@ end)
 
 describe('ShaDa support code', function()
   it('does not write NONE file', function()
-    local session = spawn(
-      { nvim_prog, '-u', 'NONE', '-i', 'NONE', '--embed', '--headless', '--cmd', 'qall' },
-      true
-    )
+    local session = n.new_session(false, {
+      merge = false,
+      args = { '-u', 'NONE', '-i', 'NONE', '--embed', '--headless', '--cmd', 'qall' },
+    })
     session:close()
     eq(nil, uv.fs_stat('NONE'))
     eq(nil, uv.fs_stat('NONE.tmp.a'))
@@ -281,7 +280,10 @@ describe('ShaDa support code', function()
 
   it('does not read NONE file', function()
     write_file('NONE', '\005\001\015\131\161na\162rX\194\162rc\145\196\001-')
-    local session = spawn({ nvim_prog, '-u', 'NONE', '-i', 'NONE', '--embed', '--headless' }, true)
+    local session = n.new_session(
+      false,
+      { merge = false, args = { '-u', 'NONE', '-i', 'NONE', '--embed', '--headless' } }
+    )
     set_session(session)
     eq('', fn.getreg('a'))
     session:close()

test/functional/terminal/tui_spec.lua

@@ -26,7 +26,6 @@ local api = n.api
 local is_ci = t.is_ci
 local is_os = t.is_os
 local new_pipename = n.new_pipename
-local spawn_argv = n.spawn_argv
 local set_session = n.set_session
 local write_file = t.write_file
 local eval = n.eval
@@ -3320,8 +3319,8 @@ describe('TUI as a client', function()
   end)
 
   it('connects to remote instance (with its own TUI)', function()
-    local server_super = spawn_argv(false) -- equivalent to clear()
-    local client_super = spawn_argv(true)
+    local server_super = n.new_session(false)
+    local client_super = n.new_session(true)
 
     set_session(server_super)
     local server_pipe = new_pipename()
@@ -3395,8 +3394,8 @@ describe('TUI as a client', function()
   end)
 
   it('connects to remote instance (--headless)', function()
-    local server = spawn_argv(false) -- equivalent to clear()
-    local client_super = spawn_argv(true, { env = { NVIM_LOG_FILE = testlog } })
+    local server = n.new_session(false)
+    local client_super = n.new_session(true, { env = { NVIM_LOG_FILE = testlog } })
 
     set_session(server)
     local server_pipe = api.nvim_get_vvar('servername')
@@ -3462,8 +3461,8 @@ describe('TUI as a client', function()
   end)
 
   local function test_remote_tui_quit(status)
-    local server_super = spawn_argv(false) -- equivalent to clear()
-    local client_super = spawn_argv(true)
+    local server_super = n.new_session(false)
+    local client_super = n.new_session(true)
 
     set_session(server_super)
     local server_pipe = new_pipename()

test/functional/testnvim.lua

@@ -455,23 +455,6 @@ function M.check_close()
   session = nil
 end
 
---- Starts `argv` process as a Nvim msgpack-RPC session.
----
---- @param argv string[]
---- @param merge boolean?
---- @param env string[]?
---- @param keep boolean? Don't close the current global session.
---- @param io_extra uv.uv_pipe_t? used for stdin_fd, see :help ui-option
---- @return test.Session
-function M.spawn(argv, merge, env, keep, io_extra)
-  if not keep then
-    M.check_close()
-  end
-
-  local proc = ProcStream.spawn(merge and M.merge_args(prepend_argv, argv) or argv, env, io_extra)
-  return Session.new(proc)
-end
-
 -- Creates a new Session connected by domain socket (named pipe) or TCP.
 function M.connect(file_or_address)
   local addr, port = string.match(file_or_address, '(.*):(%d+)')
@@ -480,9 +463,9 @@ function M.connect(file_or_address)
   return Session.new(stream)
 end
 
---- Starts (and returns) a new global Nvim session.
+--- Starts a new, global Nvim session and clears the current one.
 ---
---- Use `spawn_argv()` to get a new session without replacing the current global session.
+--- Note: Use `new_session()` to start a session without replacing the current one.
 ---
 --- Parameters are interpreted as startup args, OR a map with these keys:
 --- - args:       List: Args appended to the default `nvim_argv` set.
@@ -499,33 +482,41 @@ end
 ---
 --- @param ... string Nvim CLI args
 --- @return test.Session
---- @overload fun(opts: test.new_argv.Opts): test.Session
+--- @overload fun(opts: test.session.Opts): test.Session
 function M.clear(...)
-  M.set_session(M.spawn_argv(false, ...))
+  M.set_session(M.new_session(false, ...))
   return M.get_session()
 end
 
---- Same as clear(), but doesn't replace the current global session.
+--- Starts a new Nvim process with the given args and returns a msgpack-RPC session.
 ---
---- @param keep boolean Don't close the current global session.
---- @param ... string Nvim CLI args
+--- Does not replace the current global session, unlike `clear()`.
+---
+--- @param keep boolean (default: false) Don't close the current global session.
+--- @param ... string Nvim CLI args (or see overload)
 --- @return test.Session
---- @overload fun(opts: test.new_argv.Opts): test.Session
-function M.spawn_argv(keep, ...)
-  local argv, env, io_extra = M.new_argv(...)
-  return M.spawn(argv, nil, env, keep, io_extra)
+--- @overload fun(keep: boolean, opts: test.session.Opts): test.Session
+function M.new_session(keep, ...)
+  if not keep then
+    M.check_close()
+  end
+
+  local argv, env, io_extra = M._new_argv(...)
+
+  local proc = ProcStream.spawn(argv, env, io_extra)
+  return Session.new(proc)
 end
 
 --- Starts a (non-RPC, `--headless --listen "Tx"`) Nvim process, waits for exit, and returns result.
 ---
---- @param ... string Nvim CLI args
+--- @param ... string Nvim CLI args, or `test.session.Opts` table.
 --- @return test.ProcStream
---- @overload fun(opts: test.new_argv.Opts): test.ProcStream
+--- @overload fun(opts: test.session.Opts): test.ProcStream
 function M.spawn_wait(...)
   local opts = type(...) == 'string' and { args = { ... } } or ...
   opts.args_rm = opts.args_rm and opts.args_rm or {}
   table.insert(opts.args_rm, '--embed')
-  local argv, env, io_extra = M.new_argv(opts)
+  local argv, env, io_extra = M._new_argv(opts)
   local proc = ProcStream.spawn(argv, env, io_extra)
   proc.collect_text = true
   proc:read_start()
@@ -534,36 +525,50 @@ function M.spawn_wait(...)
   return proc
 end
 
---- @class test.new_argv.Opts
+--- @class test.session.Opts
+--- Nvim CLI args
 --- @field args? string[]
+--- Remove these args from the default `nvim_argv` args set. Ignored if `merge=false`.
 --- @field args_rm? string[]
+--- (default: true) Merge `args` with the default set. Else use only the provided `args`.
+--- @field merge? boolean
+--- Environment variables
 --- @field env? table<string,string>
+--- Used for stdin_fd, see `:help ui-option`
 --- @field io_extra? uv.uv_pipe_t
 
---- Builds an argument list for use in clear().
+--- @private
 ---
---- @param ... string See clear().
+--- Builds an argument list for use in `new_session()`, `clear()`, and `spawn_wait()`.
+---
+--- @param ... string Nvim CLI args, or `test.session.Opts` table.
 --- @return string[]
 --- @return string[]?
 --- @return uv.uv_pipe_t?
---- @overload fun(opts: test.new_argv.Opts): string[], string[]?, uv.uv_pipe_t?
-function M.new_argv(...)
-  local args = { unpack(M.nvim_argv) }
-  table.insert(args, '--headless')
-  if _G._nvim_test_id then
-    -- Set the server name to the test-id for logging. #8519
-    table.insert(args, '--listen')
-    table.insert(args, _G._nvim_test_id)
+--- @overload fun(opts: test.session.Opts): string[], string[]?, uv.uv_pipe_t?
+function M._new_argv(...)
+  --- @type test.session.Opts|string
+  local opts = select(1, ...)
+  local merge = type(opts) ~= 'table' and true or opts.merge ~= false
+
+  local args = merge and { unpack(M.nvim_argv) } or { M.nvim_prog }
+  if merge then
+    table.insert(args, '--headless')
+    if _G._nvim_test_id then
+      -- Set the server name to the test-id for logging. #8519
+      table.insert(args, '--listen')
+      table.insert(args, _G._nvim_test_id)
+    end
   end
+
   local new_args --- @type string[]
   local io_extra --- @type uv.uv_pipe_t?
-  local env --- @type string[]?
-  --- @type test.new_argv.Opts|string
-  local opts = select(1, ...)
+  local env --- @type string[]? List of "key=value" env vars.
+
   if type(opts) ~= 'table' then
     new_args = { ... }
   else
-    args = remove_args(args, opts.args_rm)
+    args = merge and remove_args(args, opts.args_rm) or args
     if opts.env then
       local env_opt = {} --- @type table<string,string>
       for k, v in pairs(opts.env) do

test/functional/ui/screen_basic_spec.lua

@@ -2,7 +2,7 @@ local t = require('test.testutil')
 local n = require('test.functional.testnvim')()
 local Screen = require('test.functional.ui.screen')
 
-local spawn, set_session, clear = n.spawn, n.set_session, n.clear
+local set_session, clear = n.set_session, n.clear
 local feed, command = n.feed, n.command
 local exec = n.exec
 local insert = n.insert
@@ -26,7 +26,7 @@ describe('screen', function()
   }
 
   before_each(function()
-    local screen_nvim = spawn(nvim_argv)
+    local screen_nvim = n.new_session(false, { args = nvim_argv, merge = false })
     set_session(screen_nvim)
     screen = Screen.new()
   end)
@@ -766,7 +766,7 @@ describe('Screen default colors', function()
       'colorscheme vim',
       '--embed',
     }
-    local screen_nvim = spawn(nvim_argv)
+    local screen_nvim = n.new_session(false, { args = nvim_argv, merge = false })
     set_session(screen_nvim)
     screen = Screen.new(53, 14, { rgb = true, ext_termcolors = termcolors or nil })
   end