test: spawn_wait() starts a non-RPC Nvim process #30384
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
build
justinmk
force-pushed
the
luaclient
branch
3 times, most recently
from
5245013 to
ed205ae
Compare
This comment was marked as resolved.
This comment was marked as resolved.
justinmk
commented
Jan 2, 2025
| @@ -383,7 +383,8 @@ void grid_line_start(ScreenGrid *grid, int row) | |||
|
|
|||
| assert((size_t)grid_line_maxcol <= linebuf_size); | |||
|
|
|||
| if (rdb_flags & kOptRdbFlagInvalid) { | |||
| if (full_screen && (rdb_flags & kOptRdbFlagInvalid)) { | |||
| assert(linebuf_char); | |||
There was a problem hiding this comment.
Check for full_screen to fix a NPE when running without a screen. The tests use redrawdebug=invalid by default, but default_grid_alloc skips calling grid_alloc when not full_screen.
local proc = n.spawn_wait('-l', 'test/functional/fixtures/startup-fail.lua')
RUN T1565 startup -l Lua Lua-error sets Nvim exitcode: 241.00 ms OK
==================== File …/build/log/asan.13763 ====================
= …/src/nvim/grid.c:389:12: runtime error: null pointer passed as argument 1, which is declared to never be null
= /usr/include/string.h:61:62: note: nonnull attribute specified here
= 0 0x55cc2d869762 in grid_line_start …/src/nvim/grid.c:389:5
= 1 0x55cc2d8717ca in grid_clear …/src/nvim/grid.c:618:5
...
= 8 0x55cc2d9c5945 in nlua_error …/src/nvim/lua/executor.c:158:5
= 9 0x55cc2d9c89fd in nlua_exec_file …/src/nvim/lua/executor.c:1862:5
Problem:
This test causes a null pointer dereference:
local proc = n.spawn_wait('-l', 'test/functional/fixtures/startup-fail.lua')
RUN T1565 startup -l Lua Lua-error sets Nvim exitcode: 241.00 ms OK
==================== File …/build/log/asan.13763 ====================
= …/src/nvim/grid.c:389:12: runtime error: null pointer passed as argument 1, which is declared to never be null
= /usr/include/string.h:61:62: note: nonnull attribute specified here
= 0 0x55cc2d869762 in grid_line_start …/src/nvim/grid.c:389:5
= 1 0x55cc2d8717ca in grid_clear …/src/nvim/grid.c:618:5
= 2 0x55cc2dbe0f6f in msg_clr_eos_force …/src/nvim/message.c:3085:3
= 3 0x55cc2dbbbdec in msg_clr_eos …/src/nvim/message.c:3061:5
= 4 0x55cc2dbbae2c in msg_multiline …/src/nvim/message.c:281:9
= 5 0x55cc2dbba2b4 in msg_keep …/src/nvim/message.c:364:5
= 6 0x55cc2dbc4992 in emsg_multiline …/src/nvim/message.c:773:10
= 7 0x55cc2dbc5d43 in semsg_multiline …/src/nvim/message.c:824:9
= 8 0x55cc2d9c5945 in nlua_error …/src/nvim/lua/executor.c:158:5
= 9 0x55cc2d9c89fd in nlua_exec_file …/src/nvim/lua/executor.c:1862:5
= 10 0x55cc2d9f4d69 in main …/src/nvim/main.c:637:19
= 11 0x7f319b62a1c9 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
= 12 0x7f319b62a28a in __libc_start_main csu/../csu/libc-start.c:360:3
= 13 0x55cc2ced0f64 in _start (…/build/bin/nvim+0xc48f64) (BuildId: 309c83f8d74297c89719dae9c271dd8ec23e64c3)
Cause:
The tests use `redrawdebug=invalid` by default, but `default_grid_alloc`
skips calling `grid_alloc` when not `full_screen`.
Solution:
Check for `full_screen`.
justinmk
commented
Jan 3, 2025
| -- See if `proc.stderr` has anything useful. | ||
| stderr = '' ~= ((stderr or ''):match('^%s*(.*%S)') or '') and ' stderr:\n' .. stderr or '' | ||
|
|
||
| self.eof_err = { 1, 'EOF was received from Nvim. Likely the Nvim process crashed.' .. stderr } |
There was a problem hiding this comment.
Include stderr (if any) in the EOF message. Not sure if this will ever be useful, will see...
justinmk
force-pushed
the
luaclient
branch
3 times, most recently
from
0e669ee to
d676e47
Compare
Problem: Can't use `n.clear()` to test non-RPC `nvim` invocations. So tests end up creating ad-hoc wrappers around `system()` or `jobstart()`. Solution: - Introduce `n.spawn_wait()` - TODO (followup PR): Rename `n.spawn()` and `n.spawn_wait()`. It's misleading that `n.spawn()` returns a RPC session...
This was referenced Jan 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Can't use
n.clear()to test non-RPC Nvim. So tests end up creating ad-hoc wrappers aroundsystem()orjobstart().Solution
n.spawn_wait()n.spawn()andn.spawn_wait().It's misleading that
n.spawn()returns a RPC session...old summary
Merge garyburd's https://github.com/justinmk/lua-client2 into
test/client2and use that instead of the old Lua client.Advantages of client2:
add_idle_calllooks neatNvim.handlers(for simplify remote plugins, massively #27949 )Disadvantages:
nvim_xx) RPC methods?coxpcall