From 9432e6c1e2e93ffffabeeeaeb6ccc0cbdf9609dd Mon Sep 17 00:00:00 2001 From: Lewis Russell Date: Fri, 8 May 2026 11:35:50 +0100 Subject: [PATCH] test: run Lua harness with nvim -l Problem: The Lua test harness still ran through standalone -ll mode, so tests depended on the low-level Lua path instead of the regular Nvim Lua environment. That also meant os.exit() coverage had to carry an ASAN workaround because Lua's raw process exit skipped Nvim teardown and let LeakSanitizer interfere with the observed exit code. Solution: Run the harness and related fixtures with nvim -l. Patch os.exit() in the main Lua state to exit through getout(), so scripts observe normal Nvim shutdown while standalone -ll remains available for generator-style scripts. As a consequence, the startup test can assert os.exit() without disabling leak detection. AI-assisted: Codex --- cmake/RunTests.cmake | 4 +- runtime/doc/dev_test.txt | 10 ++--- runtime/doc/dev_tools.txt | 2 +- src/nvim/lua/executor.c | 17 ++++++++ src/nvim/main.c | 4 +- test/client/uv_stream.lua | 41 +++++++++++++------ test/functional/api/rpc_fixture.lua | 1 + test/functional/api/server_requests_spec.lua | 2 +- test/functional/api/ui_spec.lua | 15 +++++++ test/functional/core/startup_spec.lua | 40 ++++++++++++++---- test/functional/harness/harness_spec.lua | 4 +- test/functional/terminal/tui_spec.lua | 12 +++++- test/functional/testnvim.lua | 43 ++++++++++++++++++-- test/functional/ui/screen.lua | 4 ++ test/run_tests.zig | 2 +- test/runner.lua | 10 ++++- test/unit/testutil.lua | 1 - 17 files changed, 166 insertions(+), 46 deletions(-) diff --git a/cmake/RunTests.cmake b/cmake/RunTests.cmake index 5cd397f52b..20063116dd 100644 --- a/cmake/RunTests.cmake +++ b/cmake/RunTests.cmake @@ -87,9 +87,7 @@ if(NOT WIN32) endif() execute_process( - # Note: because of "-ll" (low-level interpreter mode), some modules like - # _core/editor.lua are not loaded. - COMMAND ${NVIM_PRG} -ll ${ROOT_DIR}/test/runner.lua -v + COMMAND ${NVIM_PRG} -l ${ROOT_DIR}/test/runner.lua -v --summary-file=${TEST_SUMMARY_FILE} --helper=${TEST_DIR}/${TEST_TYPE}/preload.lua --lpath=${BUILD_DIR}/?.lua diff --git a/runtime/doc/dev_test.txt b/runtime/doc/dev_test.txt index 2b16a9f65f..e911a76d38 100644 --- a/runtime/doc/dev_test.txt +++ b/runtime/doc/dev_test.txt @@ -46,10 +46,10 @@ skipped. Harness isolation *dev-test-harness-isolation* -The Lua harness runs all selected spec files in one low-level `nvim -ll` -process, then restores a baseline before each file. For each suite iteration, -the file boundary baseline is captured after `--helper` is loaded, so -helper-provided modules and defaults persist across files. +The Lua harness runs all selected spec files in one `nvim -l` process, then +restores a baseline before each file. For each suite iteration, the file +boundary baseline is captured after `--helper` is loaded, so helper-provided +modules and defaults persist across files. Helper files are preload-only: they may require modules, set defaults, and register suite-end cleanup, but they do not define tests or hooks. @@ -221,7 +221,7 @@ With these installed you can use a configuration like this: > command = "nlua", }, args = { - "-ll", + "-l", "test/runner.lua", "--helper=test/functional/preload.lua", "--lpath=build/?.lua", diff --git a/runtime/doc/dev_tools.txt b/runtime/doc/dev_tools.txt index 79b9b0fcd9..3d2a8c6b1d 100644 --- a/runtime/doc/dev_tools.txt +++ b/runtime/doc/dev_tools.txt @@ -403,7 +403,7 @@ Then, in another terminal: USING LLDB TO STEP THROUGH UNIT TESTS > - lldb build/bin/nvim -- -ll test/runner.lua --lpath=./build/?.lua test/unit/ + lldb build/bin/nvim -- -l test/runner.lua --lpath=./build/?.lua test/unit/ < USING GDB diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c index 199813bf4a..c2b1887d60 100644 --- a/src/nvim/lua/executor.c +++ b/src/nvim/lua/executor.c @@ -601,6 +601,17 @@ static int nlua_check_interrupt(lua_State *lstate) return 1; } +static int nlua_os_exit(lua_State *lstate) +{ + int status = 0; + if (lua_gettop(lstate) >= 1 && !lua_isnil(lstate, 1)) { + status = lua_isboolean(lstate, 1) ? (lua_toboolean(lstate, 1) ? 0 : 1) + : (int)luaL_checkinteger(lstate, 1); + } + getout(status); + return 0; // Unreachable, but MSVC does not infer getout() is noreturn. +} + static nlua_ref_state_t *nlua_new_ref_state(lua_State *lstate, bool is_thread) FUNC_ATTR_NONNULL_ALL { @@ -841,6 +852,12 @@ static bool nlua_state_init(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL lua_pop(lstate, 1); #endif + // os.exit() + lua_getglobal(lstate, "os"); + lua_pushcfunction(lstate, &nlua_os_exit); + lua_setfield(lstate, -2, "exit"); + lua_pop(lstate, 1); + // vim lua_newtable(lstate); diff --git a/src/nvim/main.c b/src/nvim/main.c index d2c00cd2f4..ca2ebd9919 100644 --- a/src/nvim/main.c +++ b/src/nvim/main.c @@ -189,9 +189,7 @@ static bool event_teardown(void) } /// Performs early initialization. -/// -/// Needed for unit tests. -void early_init(mparm_T *paramp) +static void early_init(mparm_T *paramp) { os_hint_priority(); estack_init(); diff --git a/test/client/uv_stream.lua b/test/client/uv_stream.lua index a8f83b4eeb..5e9b405031 100644 --- a/test/client/uv_stream.lua +++ b/test/client/uv_stream.lua @@ -5,6 +5,18 @@ local uv = vim.uv +local function close_handle(handle) + if handle and not handle:is_closing() then + handle:close() + end +end + +local function read_stop(handle) + if handle and not handle:is_closing() then + handle:read_stop() + end +end + --- @class test.Stream --- @field write fun(self, data: string|string[]) --- @field read_start fun(self, cb: fun(chunk: string)) @@ -44,12 +56,12 @@ function StdioStream:read_start(cb) end function StdioStream:read_stop() - self._in:read_stop() + read_stop(self._in) end function StdioStream:close() - self._in:close() - self._out:close() + close_handle(self._in) + close_handle(self._out) end --- Stream over a named pipe or TCP socket. @@ -74,6 +86,7 @@ function SocketStream.open(file) -- so wait for the connect callback to be called. uv.run() if self._stream_error then + close_handle(socket) error(self._stream_error) end return self @@ -120,13 +133,11 @@ function SocketStream:read_stop() if self._stream_error then error(self._stream_error) end - uv.read_stop(self._socket) + read_stop(self._socket) end function SocketStream:close() - if not self._socket:is_closing() then - uv.close(self._socket) - end + close_handle(self._socket) end --- Stream over child process stdio. @@ -199,6 +210,8 @@ function ProcStream.spawn(argv, env, io_extra, on_exit, forward_stderr) self.signal = signal -- "Abort" exit may not set status; force to nonzero in that case. self.status = (0 ~= (status or 0) or 0 == (signal or 0)) and status or (128 + (signal or 0)) + close_handle(self._child_stdin) + close_handle(self._proc) if self._on_exit then self._on_exit(self._closed) end @@ -230,6 +243,8 @@ function ProcStream:on_read(stream, cb, err, chunk) else -- stderr_eof/stdout_eof self[stream .. '_eof'] = true ---@type boolean + -- EOF is the stream's lifecycle end even if the caller never closes it. + close_handle(stream == 'stdout' and self._child_stdout or self._child_stderr) end -- Handler provided by the caller. @@ -255,8 +270,8 @@ function ProcStream:read_start(on_stdout, on_stderr) end function ProcStream:read_stop() - self._child_stdout:read_stop() - self._child_stderr:read_stop() + read_stop(self._child_stdout) + read_stop(self._child_stderr) end function ProcStream:close(signal, noblock) @@ -265,10 +280,10 @@ function ProcStream:close(signal, noblock) end self._closed = uv.now() self:read_stop() - self._child_stdin:close() - self._child_stdout:close() - self._child_stderr:close() - if type(signal) == 'string' then + close_handle(self._child_stdin) + close_handle(self._child_stdout) + close_handle(self._child_stderr) + if type(signal) == 'string' and self._proc and not self._proc:is_closing() then self._proc:kill('sig' .. signal) end if not noblock then diff --git a/test/functional/api/rpc_fixture.lua b/test/functional/api/rpc_fixture.lua index 050d439a1b..7799d0d4e7 100644 --- a/test/functional/api/rpc_fixture.lua +++ b/test/functional/api/rpc_fixture.lua @@ -29,3 +29,4 @@ local function on_notification(event, args) end session:run(on_request, on_notification) +session:close() diff --git a/test/functional/api/server_requests_spec.lua b/test/functional/api/server_requests_spec.lua index fbe854df1a..e848df4b63 100644 --- a/test/functional/api/server_requests_spec.lua +++ b/test/functional/api/server_requests_spec.lua @@ -246,7 +246,7 @@ describe('server -> client', function() ]]) api.nvim_set_var('args', { nvim_prog, - '-ll', + '-l', 'test/functional/api/rpc_fixture.lua', package.path, package.cpath, diff --git a/test/functional/api/ui_spec.lua b/test/functional/api/ui_spec.lua index 3fcad28b3a..b9722e56d7 100644 --- a/test/functional/api/ui_spec.lua +++ b/test/functional/api/ui_spec.lua @@ -94,6 +94,13 @@ describe('nvim_ui_send', function() clear() end) + local function close_pipe(pipe) + if not pipe:is_closing() then + pipe:read_stop() + pipe:close() + end + end + it('works with stdout_tty', function() local fds = assert(uv.pipe()) @@ -110,6 +117,10 @@ describe('nvim_ui_send', function() local screen = Screen.new(50, 10, { stdout_tty = true }) screen:set_stdout(fds.write) + finally(function() + screen:detach() + close_pipe(read_pipe) + end) api.nvim_ui_send('Hello world') @@ -140,6 +151,10 @@ describe('nvim_ui_send', function() local screen = Screen.new(50, 10) screen:set_stdout(fds.write) + finally(function() + screen:detach() + close_pipe(read_pipe) + end) api.nvim_ui_send('Hello world') diff --git a/test/functional/core/startup_spec.lua b/test/functional/core/startup_spec.lua index f7e167f10b..d1aa782b58 100644 --- a/test/functional/core/startup_spec.lua +++ b/test/functional/core/startup_spec.lua @@ -157,14 +157,6 @@ describe('startup', function() end) it('os.exit() sets Nvim exitcode', function() - -- tricky: LeakSanitizer triggers on os.exit() and disrupts the return value, disable it - exec_lua [[ - local asan_options = os.getenv('ASAN_OPTIONS') or '' - if asan_options ~= '' then - asan_options = asan_options .. ':' - end - vim.uv.os_setenv('ASAN_OPTIONS', asan_options .. ':detect_leaks=0') - ]] -- nvim -l foo.lua -arg1 -- a b c assert_l_out( [[ @@ -179,6 +171,38 @@ describe('startup', function() eq(73, eval('v:shell_error')) end) + it('os.exit() runs Nvim teardown', function() + local exit_file = t.tmpname(false) + finally(function() + os.remove(exit_file) + end) + + fn.system( + { + nvim_prog, + '-u', + 'NONE', + '-i', + 'NONE', + '--cmd', + 'set shada=', + '-l', + '-', + }, + ([[ + vim.api.nvim_create_autocmd('VimLeave', { + callback = function() + vim.fn.writefile({ tostring(vim.v.exiting) }, %s) + end, + }) + os.exit(73) + ]]):format(vim.inspect(exit_file)) + ) + + eq(73, eval('v:shell_error')) + eq('73\n', read_file(exit_file)) + end) + 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())) diff --git a/test/functional/harness/harness_spec.lua b/test/functional/harness/harness_spec.lua index 26c6b7d82a..a344a0db7b 100644 --- a/test/functional/harness/harness_spec.lua +++ b/test/functional/harness/harness_spec.lua @@ -1,5 +1,5 @@ -- Black-box tests for the local Lua harness itself. These spawn a separate --- low-level Nvim process instead of driving an embedded instance via testnvim. +-- Nvim process instead of driving an embedded instance via testnvim. local t = require('test.testutil') local uv = vim.uv @@ -66,7 +66,7 @@ local function run_harness(suite_dir, extra_args) local exit_code --- @type integer? local args = { - '-ll', + '-l', runner, '-v', '--lpath=' .. build_dir .. '/?.lua', diff --git a/test/functional/terminal/tui_spec.lua b/test/functional/terminal/tui_spec.lua index ce91534304..8366f02fb5 100644 --- a/test/functional/terminal/tui_spec.lua +++ b/test/functional/terminal/tui_spec.lua @@ -219,9 +219,17 @@ describe('TUI :restart', function() -- Retry: old server may still be alive (connect succeeds but yields stale starttime), -- or on Windows the --listen address is restored async. retry(nil, 5000, function() - sess = n.connect(addr) - local _, t = sess:request('nvim_eval', 'v:starttime') + local candidate = n.connect(addr) + local status, t = candidate:request('nvim_eval', 'v:starttime') + if not status then + candidate:close() + error(type(t) == 'table' and t[2] or t) + end + if t <= starttime then + candidate:close() + end ok(t > starttime, ('v:starttime (%d) > old starttime (%d)'):format(t, starttime), t) + sess = candidate new_starttime = t end) return new_starttime, sess diff --git a/test/functional/testnvim.lua b/test/functional/testnvim.lua index 90c18cc5b7..3ea94c25ee 100644 --- a/test/functional/testnvim.lua +++ b/test/functional/testnvim.lua @@ -107,12 +107,14 @@ if prepend_argv then end local session --- @type test.Session? +local sessions = {} --- @type table +local sigpipe_handler --- @type uv.uv_signal_t? local loop_running --- @type boolean? local last_error --- @type string? local method_error --- @type string? if not is_os('win') then - local sigpipe_handler = assert(uv.new_signal()) + sigpipe_handler = assert(uv.new_signal()) uv.signal_start(sigpipe_handler, 'sigpipe', function() print('warning: got SIGPIPE signal. Likely related to a crash in nvim') end) @@ -346,6 +348,7 @@ function M.expect_exit(fn_or_timeout, ...) end, fn_or_timeout, ...) ) end + M.check_close() end --- Executes a Vimscript function via Lua. @@ -447,16 +450,32 @@ function M.check_close(noblock) return end - session:close(nil, noblock) + local s = session + s:close(nil, noblock) + sessions[s] = nil session = nil end +local function close_extra_sessions(test_id, noblock) + for s in pairs(sessions) do + if s ~= session and (test_id == nil or (s.data and s.data.test_id == test_id)) then + if not s.closed then + s:close(nil, noblock) + end + sessions[s] = nil + end + end +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+)') local stream = (addr and port) and SocketStream.connect(addr, port) or SocketStream.open(file_or_address) - return Session.new(stream) + local s = Session.new(stream) + s.data = { test_id = _G._nvim_test_id } + sessions[s] = true + return s end --- Starts a new, global Nvim session and clears the current one. @@ -499,8 +518,10 @@ local n_processes = 0 function M.new_session(keep, ...) local test_id = _G._nvim_test_id if not keep and session ~= nil then + local s = session -- Don't block for the previous session's exit if it's from a different test. - session:close(nil, session.data and session.data.test_id ~= test_id) + s:close(nil, s.data and s.data.test_id ~= test_id) + sessions[s] = nil session = nil end @@ -524,6 +545,7 @@ function M.new_session(keep, ...) n_processes = n_processes + 1 local new_session = Session.new(proc) + sessions[new_session] = true -- Make it possible to check whether two sessions are from the same test. new_session.data = { test_id = test_id } return new_session @@ -531,6 +553,10 @@ end harness.on_suite_end(function() M.check_close(true) + -- Some tests create extra sessions without replacing the global one. + -- Close any that individual tests did not clean up before the runner exits. + close_extra_sessions(nil, true) + local timed_out = false local timer = assert(vim.uv.new_timer()) timer:start(10000, 0, function() @@ -540,6 +566,14 @@ harness.on_suite_end(function() uv.run('once') end timer:close() + if sigpipe_handler and not sigpipe_handler:is_closing() then + sigpipe_handler:close() + end + -- Drain close callbacks queued by explicit Session/ProcStream cleanup before + -- Nvim checks the runner loop for active handles. + for _ = 1, 10 do + uv.run('nowait') + end if timed_out then print(('warning: %d dangling Nvim processes'):format(n_processes)) io.stdout:flush() @@ -1049,6 +1083,7 @@ return function() if after_each then after_each(function() + close_extra_sessions(_G._nvim_test_id, true) if not vim.endswith(_G._nvim_test_id, 'x') then -- Use a different test ID for skipped tests as well as Nvim instances spawned -- between this after_each() and the next before_each() (e.g. in setup()). diff --git a/test/functional/ui/screen.lua b/test/functional/ui/screen.lua index a64565d7cb..3c6e1034d7 100644 --- a/test/functional/ui/screen.lua +++ b/test/functional/ui/screen.lua @@ -315,6 +315,10 @@ function Screen:attach(session) end function Screen:detach() + if self._stdout and not self._stdout:is_closing() then + self._stdout:close() + end + self._stdout = nil self.uimeths.detach() self._session = nil end diff --git a/test/run_tests.zig b/test/run_tests.zig index ebecdb5909..39238b4473 100644 --- a/test/run_tests.zig +++ b/test/run_tests.zig @@ -3,7 +3,7 @@ const LazyPath = std.Build.LazyPath; pub fn testStep(b: *std.Build, kind: []const u8, nvim_bin: *std.Build.Step.Compile, config_dir: LazyPath, include_path: ?[]const LazyPath) !*std.Build.Step.Run { const test_step = b.addRunArtifact(nvim_bin); - test_step.addArg("-ll"); + test_step.addArg("-l"); test_step.addFileArg(b.path("./test/runner.lua")); if (include_path) |paths| { for (paths) |path| { diff --git a/test/runner.lua b/test/runner.lua index a73719893d..b434351384 100644 --- a/test/runner.lua +++ b/test/runner.lua @@ -27,9 +27,15 @@ end local root = repo_root() prepend_package_roots({ root, root .. '/test', '.', './test' }) +-- The harness is not an Nvim instance under test. If its startup server stays +-- visible, serverlist({ peer = true }) can connect back to the runner and wait +-- forever for an RPC response. +if vim.v.servername ~= '' then + assert(vim.fn.serverstop(vim.v.servername) == 1) +end + local exit_code = require('test.harness').main(_G.arg) io.stdout:flush() io.stderr:flush() --- Close the standalone Lua state before exit so sanitizers see Lua-owned cleanup. -os.exit(exit_code, true) +os.exit(exit_code) diff --git a/test/unit/testutil.lua b/test/unit/testutil.lua index 8604cb27bb..5bd1313302 100644 --- a/test/unit/testutil.lua +++ b/test/unit/testutil.lua @@ -102,7 +102,6 @@ local init = only_separate(function() c.func(unpack(c.args)) end libnvim.event_init() - libnvim.early_init(nil) if child_calls_mod then for _, c in ipairs(child_calls_mod) do c.func(unpack(c.args))