From 272e04178060841e1bfda535de6f31825985ffef Mon Sep 17 00:00:00 2001 From: bfredl Date: Tue, 20 May 2025 11:21:52 +0200 Subject: [PATCH 1/3] fix(tests): don't surpress stderr output This is a diabolical anti-pattern and is hiding errors which exist currently in unittests. Also we want to see _where_ in the process stderr was emitted, stashing it away at the end erases important context. --- cmake/RunTests.cmake | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cmake/RunTests.cmake b/cmake/RunTests.cmake index fc4c2c587a..6d123060b6 100644 --- a/cmake/RunTests.cmake +++ b/cmake/RunTests.cmake @@ -78,7 +78,6 @@ execute_process( ${TEST_PATH} TIMEOUT $ENV{TEST_TIMEOUT} WORKING_DIRECTORY ${WORKING_DIR} - ERROR_VARIABLE err RESULT_VARIABLE res ${EXTRA_ARGS}) @@ -87,11 +86,6 @@ file(REMOVE_RECURSE ${RM_FILES}) if(res) message(STATUS "Tests exited non-zero: ${res}") - if("${err}" STREQUAL "") - message(STATUS "No output to stderr.") - else() - message(STATUS "Output to stderr:\n${err}") - endif() # Dump the logfile on CI (if not displayed and moved already). if(CI_BUILD) From c81af9428c08d21d6e7ddc7ce7ac1761e9a20b9e Mon Sep 17 00:00:00 2001 From: bfredl Date: Tue, 20 May 2025 12:10:37 +0200 Subject: [PATCH 2/3] fix(tests): use uv.spawn instead of io.popen for unittest helpers The old implementation of repeated_read_cmd would attempt to run the command multiple times to handle racyness of async output. Code like this should not be written. Instead, use the libuv event loop to read until the process has exited and the pipe has been closed. This causes some previous discarded errors to be propagated. Fix these as well. --- test/testutil.lua | 60 ++++++++++++++------------ test/unit/api/private_helpers_spec.lua | 2 +- test/unit/api/testutil.lua | 7 ++- test/unit/vterm_spec.lua | 2 +- 4 files changed, 40 insertions(+), 31 deletions(-) diff --git a/test/testutil.lua b/test/testutil.lua index 62cf1b18ca..fb018a3e9e 100644 --- a/test/testutil.lua +++ b/test/testutil.lua @@ -5,17 +5,6 @@ local Paths = require('test.cmakeconfig.paths') luaassert:set_parameter('TableFormatLevel', 100) -local quote_me = '[^.%w%+%-%@%_%/]' -- complement (needn't quote) - ---- @param str string ---- @return string -local function shell_quote(str) - if string.find(str, quote_me) or str == '' then - return '"' .. str:gsub('[$%%"\\]', '\\%0') .. '"' - end - return str -end - --- Functions executing in the context of the test runner (not the current nvim test session). --- @class test.testutil local M = { @@ -66,19 +55,15 @@ function M.argss_to_cmd(...) for i = 1, select('#', ...) do local arg = select(i, ...) if type(arg) == 'string' then - cmd[#cmd + 1] = shell_quote(arg) + cmd[#cmd + 1] = arg else --- @cast arg string[] for _, subarg in ipairs(arg) do - cmd[#cmd + 1] = shell_quote(subarg) + cmd[#cmd + 1] = subarg end end end - return table.concat(cmd, ' ') -end - -function M.popen_r(...) - return io.popen(M.argss_to_cmd(...), 'r') + return cmd end --- Calls fn() until it succeeds, up to `max` times or until `max_ms` @@ -356,7 +341,7 @@ function M.check_logs() local status, f local out = io.stdout if os.getenv('SYMBOLIZER') then - status, f = pcall(M.popen_r, os.getenv('SYMBOLIZER'), '-l', file) + status, f = pcall(M.repeated_read_cmd, os.getenv('SYMBOLIZER'), '-l', file) end out:write(start_msg .. '\n') if status then @@ -539,16 +524,37 @@ end --- @return string? function M.repeated_read_cmd(...) - for _ = 1, 10 do - local stream = M.popen_r(...) - local ret = stream:read('*a') - stream:close() - if ret then - return ret + local cmd = M.argss_to_cmd(...) + local data = {} + local got_code = nil + local stdout = assert(vim.uv.new_pipe(false)) + local handle = assert( + vim.uv.spawn( + cmd[1], + { args = vim.list_slice(cmd, 2), stdio = { nil, stdout, 2 }, hide = true }, + function(code, _signal) + got_code = code + end + ) + ) + stdout:read_start(function(err, chunk) + if err or chunk == nil then + stdout:read_stop() + stdout:close() + else + table.insert(data, chunk) end + end) + + while not stdout:is_closing() or got_code == nil do + vim.uv.run('once') end - print('ERROR: Failed to execute ' .. M.argss_to_cmd(...) .. ': nil return after 10 attempts') - return nil + + if got_code ~= 0 then + error('command ' .. vim.inspect(cmd) .. 'unexpectedly exited with status ' .. got_code) + end + handle:close() + return table.concat(data) end --- @generic T diff --git a/test/unit/api/private_helpers_spec.lua b/test/unit/api/private_helpers_spec.lua index bdfc83a031..4d9c2e5046 100644 --- a/test/unit/api/private_helpers_spec.lua +++ b/test/unit/api/private_helpers_spec.lua @@ -18,7 +18,7 @@ local type_key = api_t.type_key local obj2lua = api_t.obj2lua local func_type = api_t.func_type -local api = cimport('./src/nvim/api/private/t.h', './src/nvim/api/private/converter.h') +local api = cimport('./src/nvim/api/private/helpers.h', './src/nvim/api/private/converter.h') describe('vim_to_object', function() local vim_to_object = function(l) diff --git a/test/unit/api/testutil.lua b/test/unit/api/testutil.lua index bb387ae0e1..77c9d608cf 100644 --- a/test/unit/api/testutil.lua +++ b/test/unit/api/testutil.lua @@ -13,8 +13,11 @@ local int_type = t_eval.int_type local flt_type = t_eval.flt_type local type_key = t_eval.type_key -local api = - cimport('./src/nvim/api/private/defs.h', './src/nvim/api/private/t.h', './src/nvim/memory.h') +local api = cimport( + './src/nvim/api/private/defs.h', + './src/nvim/api/private/helpers.h', + './src/nvim/memory.h' +) local obj2lua diff --git a/test/unit/vterm_spec.lua b/test/unit/vterm_spec.lua index c5293a21cb..aab895b7dc 100644 --- a/test/unit/vterm_spec.lua +++ b/test/unit/vterm_spec.lua @@ -94,7 +94,7 @@ local vterm = t.cimport( './src/nvim/vterm/screen.h', './src/nvim/vterm/state.h', './src/nvim/vterm/vterm.h', - './src/nvim/vterm/vterm_internal.h', + './src/nvim/vterm/vterm_internal_defs.h', './test/unit/fixtures/vterm_test.h' ) From 2d4b028d02c1255c1f21a8f1c50851626e867761 Mon Sep 17 00:00:00 2001 From: bfredl Date: Thu, 22 May 2025 11:22:29 +0200 Subject: [PATCH 3/3] fix(tests): use correct include path for unittest Copy whatever was made to work for generated headers: (1) we need to consider all cmake targets not just main_lib (2) we need to add the sysroot for macOS --- build.zig | 1 + src/nvim/CMakeLists.txt | 2 ++ test/CMakeLists.txt | 2 +- test/cmakeconfig/paths.lua.in | 1 + test/unit/preprocess.lua | 14 +++++++++++++- test/unit/testutil.lua | 5 +++++ 6 files changed, 23 insertions(+), 2 deletions(-) diff --git a/build.zig b/build.zig index 424c00c091..b999631951 100644 --- a/build.zig +++ b/build.zig @@ -408,6 +408,7 @@ pub fn test_config(b: *std.Build, gen_dir: LazyPath) ![]u8 { \\local M = {{}} \\ \\M.include_paths = {{}} + \\M.apple_sysroot = "" \\M.translations_enabled = "$ENABLE_TRANSLATIONS" == "ON" \\M.is_asan = "$ENABLE_ASAN_UBSAN" == "ON" \\M.is_zig_build = true diff --git a/src/nvim/CMakeLists.txt b/src/nvim/CMakeLists.txt index f6112a3c2a..e59be46314 100644 --- a/src/nvim/CMakeLists.txt +++ b/src/nvim/CMakeLists.txt @@ -481,11 +481,13 @@ foreach(target ${targets}) message(STATUS "${target} props '${prop}'") foreach(gen_include ${prop}) list(APPEND gen_cflags "-I${gen_include}") + list(APPEND TEST_INCLUDE_DIRS "${gen_include}") endforeach() endif() endforeach() list(REMOVE_DUPLICATES gen_cflags) +list(REMOVE_DUPLICATES TEST_INCLUDE_DIRS) if(APPLE AND CMAKE_OSX_SYSROOT) list(APPEND gen_cflags "-isysroot" "${CMAKE_OSX_SYSROOT}") diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 8db05ac839..1141e6aded 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -1,6 +1,6 @@ add_subdirectory(functional/fixtures) # compile test programs -get_target_property(TEST_INCLUDE_DIRS main_lib INTERFACE_INCLUDE_DIRECTORIES) +get_directory_property(TEST_INCLUDE_DIRS DIRECTORY ${PROJECT_SOURCE_DIR}/src/nvim DEFINITION TEST_INCLUDE_DIRS) set(TEST_OPTIONS -D BUILD_DIR=${CMAKE_BINARY_DIR} diff --git a/test/cmakeconfig/paths.lua.in b/test/cmakeconfig/paths.lua.in index 56d3257bcf..2984200952 100644 --- a/test/cmakeconfig/paths.lua.in +++ b/test/cmakeconfig/paths.lua.in @@ -4,6 +4,7 @@ M.include_paths = {} for p in ("${TEST_INCLUDE_DIRS}" .. ";"):gmatch("[^;]+") do table.insert(M.include_paths, p) end +M.apple_sysroot = "${CMAKE_OSX_SYSROOT}" M.translations_enabled = "${ENABLE_TRANSLATIONS}" == "ON" M.is_asan = "${ENABLE_ASAN_UBSAN}" == "ON" diff --git a/test/unit/preprocess.lua b/test/unit/preprocess.lua index 8d481df0d0..6114f43ad7 100644 --- a/test/unit/preprocess.lua +++ b/test/unit/preprocess.lua @@ -146,13 +146,20 @@ end --- @param ... string function Gcc:add_to_include_path(...) + local ef = self.preprocessor_extra_flags for i = 1, select('#', ...) do local path = select(i, ...) - local ef = self.preprocessor_extra_flags ef[#ef + 1] = '-I' .. path end end +function Gcc:add_apple_sysroot(sysroot) + local ef = self.preprocessor_extra_flags + + table.insert(ef, '-isysroot') + table.insert(ef, sysroot) +end + -- returns a list of the headers files upon which this file relies --- @param hdr string --- @return string[]? @@ -278,4 +285,9 @@ function M.add_to_include_path(...) return cc:add_to_include_path(...) end +--- @param ... string +function M.add_apple_sysroot(...) + return cc:add_apple_sysroot(...) +end + return M diff --git a/test/unit/testutil.lua b/test/unit/testutil.lua index a38e602ac8..3c40f7062e 100644 --- a/test/unit/testutil.lua +++ b/test/unit/testutil.lua @@ -19,6 +19,11 @@ for _, p in ipairs(paths.include_paths) do Preprocess.add_to_include_path(p) end +-- add some nonstandard header locations +if paths.apple_sysroot ~= '' then + Preprocess.add_apple_sysroot(paths.apple_sysroot) +end + local child_pid = nil --- @type integer? --- @generic F: function --- @param func F