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.
This commit is contained in:
Justin M. Keyes
2025-01-04 06:29:13 -08:00
committed by GitHub
parent 7ddadd0fee
commit 975c2124a6
8 changed files with 78 additions and 146 deletions

View File

@ -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

View File

@ -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

View File

@ -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,39 +125,27 @@ describe('command-line option', function()
]=]
end)
it('errors out when trying to use nonexistent file with -s', function()
eq(
'Cannot open for reading: "' .. nonexistent_fname .. '": no such file or directory\n',
fn.system({
nvim_prog_abs(),
'-u',
'NONE',
'-i',
'NONE',
'--headless',
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,
})
nonexistent_fname
)
eq(2, eval('v:shell_error'))
eq(
'Cannot open for reading: "' .. nonexistent_fname .. '": no such file or directory\n',
--- TODO(justinmk): using `p.output` because Nvim emits CRLF even on non-Win. Emit LF instead?
p:output()
)
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',
local p = n.spawn_wait(
'--cmd',
'set noswapfile shortmess+=IFW fileformats=unix',
'--cmd',
@ -179,10 +154,11 @@ describe('command-line option', function()
fname,
'-s',
dollar_fname,
fname_2,
})
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',

View File

@ -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)

View File

@ -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()

View File

@ -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)

View File

@ -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',

View File

@ -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()