Skip to content

Commit e70587d

Browse files
ychinchrisbra
authored andcommitted
patch 9.1.1110: Vim tests are slow and flaky
Problem: Vim tests are slow and flaky at the same time due to reliance on timeouts which are unreliable. Solution: improve Vim test performance and reduce flakiness (Yee Cheng Chin) A lot of Vim tests currently rely on waiting a specific amount of time before asserting a condition. This is bad because 1) it is slow, as the timeout is hardcoded, 2) it's unreliable as a resource-starved runner may overshoot the timeout. Also, there are a lot of builtin sleep commands in commonly used utilities like VerifyScreenDump and WaitFor() which leads to a lot of unnecessary idle time. Fix these issues by doing the following: 1. Make utilities like VerifyScreenDump and WaitFor use the lowest wait time possible (1 ms). This essentially turns it into a spin wait. On fast machines, these will finish very quickly. For existing tests that had an implicit reliance on the old timeouts (e.g. VerifyScreenDump had a 50ms wait before), fix the tests to wait that specific amount explicitly. 2. Fix tests that sleep or wait for long amounts of time to instead explicitly use a callback mechanism to be notified when a child terminal job has finished. This allows the test to only take as much time as possible instead of having to hard code an unreliable timeout. With these fixes, tests should 1) completely quickly on fast machines, and 2) on slow machines they will still run to completion albeit slowly. Note that previoulsy both were not true. The hardcoded timeouts meant that on fast machines the tests were mostly idling wasting time, whereas on slow machines, the timeouts often were not generous enough to allow them to run to completion. closes: #16615 Signed-off-by: Yee Cheng Chin <[email protected]> Signed-off-by: Christian Brabandt <[email protected]>
1 parent 977561a commit e70587d

17 files changed

Lines changed: 162 additions & 145 deletions

runtime/doc/terminal.txt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
*terminal.txt* For Vim version 9.1. Last change: 2025 Jan 11
1+
*terminal.txt* For Vim version 9.1. Last change: 2025 Feb 13
22

33

44
VIM REFERENCE MANUAL by Bram Moolenaar
@@ -999,7 +999,8 @@ term_wait({buf} [, {time}]) *term_wait()*
999999
Wait for pending updates of {buf} to be handled.
10001000
{buf} is used as with |term_getsize()|.
10011001
{time} is how long to wait for updates to arrive in msec. If
1002-
not set then 10 msec will be used.
1002+
not set then 10 msec will be used. Queued messages will also
1003+
be processed similar to |:sleep|.
10031004

10041005
Can also be used as a |method|: >
10051006
GetBufnr()->term_wait()

runtime/doc/various.txt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
*various.txt* For Vim version 9.1. Last change: 2024 Dec 16
1+
*various.txt* For Vim version 9.1. Last change: 2025 Feb 13
22

33

44
VIM REFERENCE MANUAL by Bram Moolenaar
@@ -765,9 +765,8 @@ K Run a program to lookup the keyword under the
765765
MS-Windows). "gs" stands for "goto sleep".
766766
While sleeping the cursor is positioned in the text,
767767
if at a visible position.
768-
Also process the received netbeans messages. {only
769-
available when compiled with the |+netbeans_intg|
770-
feature}
768+
Queued messages and timers (|+timers|) are processed
769+
during the sleep as well.
771770

772771
*:sl!* *:sleep!*
773772
:[N]sl[eep]! [N][m] Same as above, but hide the cursor.

src/testdir/crash/ex_redraw_crash

Lines changed: 0 additions & 1 deletion
This file was deleted.

src/testdir/runtest.vim

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@ func RunTheTest(test)
265265
" buffers.
266266
%bwipe!
267267

268+
" Clear all children notifications in case there are stale ones left
269+
let g:child_notification = 0
270+
268271
" The test may change the current directory. Save and restore the
269272
" directory after executing the test.
270273
let save_cwd = getcwd()

src/testdir/screendump.vim

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,14 +52,14 @@ func VerifyScreenDump(buf, filename, options, ...)
5252
let filter = 'dumps/' . a:filename . '.vim'
5353
let testfile = 'failed/' . a:filename . '.dump'
5454

55-
let max_loops = get(a:options, 'wait', 1000) / 10
55+
let max_loops = get(a:options, 'wait', 1000) / 1
5656

5757
" Starting a terminal to make a screendump is always considered flaky.
5858
let g:test_is_flaky = 1
5959
let g:giveup_same_error = 0
6060

6161
" wait for the pending updates to be handled.
62-
call TermWait(a:buf)
62+
call TermWait(a:buf, 0)
6363

6464
" Redraw to execute the code that updates the screen. Otherwise we get the
6565
" text and attributes only from the internal buffer.
@@ -80,8 +80,8 @@ func VerifyScreenDump(buf, filename, options, ...)
8080

8181
let i = 0
8282
while 1
83-
" leave some time for updating the original window
84-
sleep 50m
83+
" leave a bit of time for updating the original window while we spin wait.
84+
sleep 1m
8585
call delete(testfile)
8686
call term_dumpwrite(a:buf, testfile, a:options)
8787

src/testdir/shared.vim

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,34 @@ func s:kill_server(cmd)
136136
endif
137137
endfunc
138138

139+
" Callback function to be invoked by a child terminal job. The parent could
140+
" then wait for the notification using WaitForChildNotification()
141+
let g:child_notification = 0
142+
func Tapi_notify_parent(bufnum, arglist)
143+
let g:child_notification = 1
144+
endfunc
145+
146+
" Generates a command that we can pass to a terminal job that it uses to
147+
" notify us. Argument 'escape' will specify whether to escape the double
148+
" quote.
149+
func TermNotifyParentCmd(escape)
150+
call assert_false(has("win32"), 'Windows does not support terminal API right now. Use another method to synchronize timing.')
151+
let cmd = '\033]51;["call", "Tapi_notify_parent", []]\007'
152+
if a:escape
153+
return escape(cmd, '"')
154+
endif
155+
return cmd
156+
endfunc
157+
158+
" Wait for a child process to notify us. This allows us to sequence events in
159+
" conjunction with the child. Currently the only supported notification method
160+
" is for a terminal job to call Tapi_notify_parent() using terminal API.
161+
func WaitForChildNotification(...)
162+
let timeout = get(a:000, 0, 5000)
163+
call WaitFor({-> g:child_notification == 1}, timeout)
164+
let g:child_notification = 0
165+
endfunc
166+
139167
" Wait for up to five seconds for "expr" to become true. "expr" can be a
140168
" stringified expression to evaluate, or a funcref without arguments.
141169
" Using a lambda works best. Example:
@@ -162,7 +190,7 @@ endfunc
162190
" A second argument can be used to specify a different timeout in msec.
163191
"
164192
" Return zero for success, one for failure (like the assert function).
165-
func WaitForAssert(assert, ...)
193+
func g:WaitForAssert(assert, ...)
166194
let timeout = get(a:000, 0, 5000)
167195
if s:WaitForCommon(v:null, a:assert, timeout) < 0
168196
return 1
@@ -200,11 +228,11 @@ func s:WaitForCommon(expr, assert, timeout)
200228
call remove(v:errors, -1)
201229
endif
202230

203-
sleep 10m
231+
sleep 1m
204232
if exists('*reltimefloat')
205233
let slept = float2nr(reltimefloat(reltime(start)) * 1000)
206234
else
207-
let slept += 10
235+
let slept += 1
208236
endif
209237
endwhile
210238

src/testdir/term_util.vim

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,14 @@ func StopVimInTerminal(buf, kill = 1)
157157
call assert_equal("running", term_getstatus(a:buf))
158158

159159
" Wait for all the pending updates to terminal to complete
160-
call TermWait(a:buf)
160+
call TermWait(a:buf, 1)
161161

162162
" CTRL-O : works both in Normal mode and Insert mode to start a command line.
163163
" In Command-line it's inserted, the CTRL-U removes it again.
164164
call term_sendkeys(a:buf, "\<C-O>:\<C-U>qa!\<cr>")
165165

166166
" Wait for all the pending updates to terminal to complete
167-
call TermWait(a:buf)
167+
call TermWait(a:buf, 1)
168168

169169
" Wait for the terminal to end.
170170
call WaitForAssert({-> assert_equal("finished", term_getstatus(a:buf))})

src/testdir/test_balloon.vim

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ func Test_balloon_eval_term()
3636
call TermWait(buf, 50)
3737
call term_sendkeys(buf, 'll')
3838
call term_sendkeys(buf, ":call Trigger()\<CR>")
39+
sleep 150m " Wait for balloon to show up (100ms balloondelay time)
3940
call VerifyScreenDump(buf, 'Test_balloon_eval_term_01', {})
4041

4142
" Make sure the balloon still shows after 'updatetime' passed and CursorHold

0 commit comments

Comments
 (0)