Re: [vim/vim] Reduce number of calls to STRLEN() (Issue #14002)

[Christian Brabandt]

On Mi, 14 Feb 2024, John Marriott wrote:

> Hi Christian,
> 
> I have made my first PR (#14029).

Thank you!

> 
> When I did so, I got a 9 failing checks which mostly appear to be failing
> tests, like this:
> 
> command line..script
> /home/runner/work/vim/vim/src/testdir/runtest.vim[642]..function 
> RunTheTest[57]..Test_prop_below_split_line[25]..StopVimInTerminal[17]..WaitForAssert[2]..4_WaitForCommon[11]..38
> line 1: Expected 'finished' but got 'running'
> Flaky test failed too often, giving up
> 
> 
> I'm not sure how to proceed with this. This is my first PR.

No problem. If I check the CI on github, I see a few failures, including 
a buffer-overflow 
(https://github.com/vim/vim/actions/runs/7891958995/job/21537397938?pr=14029):
,
| READ of size 1 at 0x502abad3 thread T0
| #0 0x5610bb99642a in ___interceptor_strlen ??:?
| #1 0x5610bbaca046 in open_line 
/home/runner/work/vim/vim/src/change.c:1488:19
| #2 0x5610bbc064ea in ins_eol /home/runner/work/vim/vim/src/edit.c:5162:9
| #3 0x5610bbbf045d in edit /home/runner/work/vim/vim/src/edit.c:1230:10
| #4 0x5610bc01e110 in invoke_edit 
/home/runner/work/vim/vim/src/normal.c:7098:9
| #5 0x5610bbfd8fd9 in normal_cmd 
/home/runner/work/vim/vim/src/normal.c:949:5
| #6 0x5610bc8d1d18 in main_loop /home/runner/work/vim/vim/src/main.c
| #7 0x5610bc8cfc89 in vim_main2 /home/runner/work/vim/vim/src/main.c:895:5
| #8 0x5610bc8c825c in main /home/runner/work/vim/vim/src/main.c:441:12
| #9 0x7f4586229d8f in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
| #10 0x7f4586229e3f in __libc_start_main csu/../csu/libc-start.c:392:3
| #1 0x5610bb97ef94 in _start ??:?
`

This indicates that something serious got brocken. You may want to 
compile with ASAN enabled and run the test suite on your side to find 
out which test causes this. The fact that it is finished, could mean 
that the process segfaulted, in which case it would be finished indeed 
:)
I guess this may be the following test.
Test_prop_below_split_line(): Vim(call):E958: Job already finished @ 
command line..script 
/home/runner/work/vim/vim/src/testdir/runtest.vim[607]..function 
RunTheTest[57]..Test_prop_below_split_line[17]..VerifyScreenDump, line 35

Test_prop_below_split_line() sounds like it could invoke the open_line() 
function but I am just guessing here.

If I check the test, line 17:

,
| 1func Test_prop_below_split_line()
| 2  CheckRunVimInTerminal
| 3
| 4  let lines =<< trim END
| 5  vim9script
| 6  setline(1, ['one one one', 'two two two', 'three three three'])
| 7  prop_type_add('test', {highlight: 'Search'})
| 8  prop_add(2, 0, {
| 9  text:  '└─ Virtual text below the 2nd line',
| 10  type: 'test',
| 11  text_align: 'below',
| 12  text_padding_left: 3
| 13  })
| 14  END
| 15  call writefile(lines, 'XscriptPropBelowSpitLine', 'D')
| 16  let buf = RunVimInTerminal('-S XscriptPropBelowSpitLine', #{rows: 8})
| 17  call term_sendkeys(buf, "2GA\xx")
| 18  call VerifyScreenDump(buf, 'Test_prop_below_split_line_1', {})
| 19
| 20  call term_sendkeys(buf, "\:set number\")
| 21  call VerifyScreenDump(buf, 'Test_prop_below_split_line_2', {})
`

Line 17 is the line where we append a  to the buffer, so it could match.

Other than that, screendumps are unfortunately quite flaky, so it is 
okay, if only 1 or 2 tests from the suite fail, but 6 is too much :(

So try to run the failing tests in your local environment and see if you 
can debug what is going wrong. See the readme of the testsuite on how to 
run only a single test: 
https://github.com/vim/vim/blob/master/src/testdir/README.txt

Thanks,
Chris 
-- 
Youth is a blunder, manhood a struggle, old age a regret.
-- Benjamin Disraeli, "Coningsby"

-- 
-- 
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups 
"vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/ZcvYWXoFeIbiD33p%40256bit.org.

Re: [vim/vim] Reduce number of calls to STRLEN() (Issue #14002)

[John Marriott]

On 12-Feb-2024 02:32, Christian Brabandt (Vim Github Repository) wrote:


can you make a PR for this please?

—
Reply to this email directly, view it on GitHub 
.
You are receiving this because you are subscribed to this 
thread.Message ID: 


--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

---
You received this message because you are subscribed to the Google 
Groups "vim_dev" group.
To unsubscribe from this group and stop receiving emails from it, send 
an email to vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/vim/vim/issues/14002/1937786687%40github.com 
.

Hi Christian,

I have made my first PR (#14029).

When I did so, I got a 9 failing checks which mostly appear to be 
failing tests, like this:


command line..script 
/home/runner/work/vim/vim/src/testdir/runtest.vim[642]..function 
RunTheTest[57]..Test_prop_below_split_line[25]..StopVimInTerminal[17]..WaitForAssert[2]..4_WaitForCommon[11]..38 
line 1: Expected 'finished' but got 'running'

Flaky test failed too often, giving up


I'm not sure how to proceed with this. This is my first PR.

Any suggestions would be appreciated.

Cheers
John

--
--
You received this message from the "vim_dev" maillist.
Do not top-post! Type your reply below the text you are replying to.
For more information, visit http://www.vim.org/maillist.php

--- 
You received this message because you are subscribed to the Google Groups "vim_dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email 
to vim_dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/vim_dev/be04c922-1adb-4f1c-a9a2-2642728fddae%40internode.on.net.