[vim/vim] Added test for libcall() and libcallnr() (#2982)

[Dominique Pellé]

This PR adds tests for the libcall() and libcallnr() functions
which were not covered with tests according to codecov:

https://codecov.io/gh/vim/vim/src/6b810d92a9cd9378ab46ea0db07079cb789f9faa/src/os_unix.c#L7264
https://codecov.io/gh/vim/vim/src/6b810d92a9cd9378ab46ea0db07079cb789f9faa/src/evalfunc.c#L7229

I only enabled the tests on Linux as I don't know how these
functions can be tested on Windows. I'd appreciate if someone
who has windows can make the libcall/libcallnr test work on Windows.

---

You can view, comment on, or merge this pull request online at:

  https://github.com/vim/vim/pull/2982

Commit Summary

• Added test for libcall() and libcallnr()

File Changes

• M src/testdir/test_functions.vim (26)

Patch Links:

• https://github.com/vim/vim/pull/2982.patch
• https://github.com/vim/vim/pull/2982.diff

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

[Dominique Pellé]

@dpelle pushed 1 commit.

• dc7bc96 Fixed Test_libcall_callnr() failing in CI

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

[Codecov]

Codecov Report

Merging #2982 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2982      +/-   ##
==========================================
- Coverage   75.81%    75.8%   -0.01%     
==========================================
  Files          92       92              
  Lines      135201   135202       +1     
==========================================
- Hits       102501   102494       -7     
- Misses      32700    32708       +8

Impacted Files	Coverage Δ
src/gui_beval.c	55.79% <0%> (-7.3%)	⬇️
src/window.c	81.68% <0%> (-0.59%)	⬇️
src/message.c	74.59% <0%> (-0.25%)	⬇️
src/term.c	59.79% <0%> (-0.21%)	⬇️
src/gui.c	49.25% <0%> (-0.16%)	⬇️
src/gui_gtk_x11.c	47.83% <0%> (-0.05%)	⬇️
src/screen.c	76.63% <0%> (-0.05%)	⬇️
src/syntax.c	78.35% <0%> (-0.03%)	⬇️
src/if_py_both.h	76.6% <0%> (ø)	⬆️
src/ex_docmd.c	76.66% <0%> (+0.01%)	⬆️
... and 5 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acb9eff...dc7bc96. Read the comment docs.

[K.Takata]

@dpelle How about this for Windows?

    let msvcrt = 'msvcrt.dll'

    let userprofile = libcall(msvcrt, 'getenv', 'USERPROFILE')
    call assert_equal($USERPROFILE, userprofile)

    let nr = libcallnr(msvcrt, 'atoi', '1234')
    call assert_equal(1234, nr)

[Dominique Pellé]

@dpelle pushed 1 commit.

• 9f23a92 Test libcall() for Windows (kudos to Ken Takata for hints)

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

[Dominique Pellé]

@dpelle pushed 1 commit.

• 8827d8f Tentatively fixed failing Test_libcall_libcallnr in CI

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

[K.Takata]

@k-takata commented on this pull request.

---

In src/testdir/test_functions.vim:

>  
-    let pid = libcallnr(libc, 'getpid', '')
-    call assert_equal(getpid(), pid)
+  let $X_TEST_ENV_VAR_ = 'foo'
+  call assert_equal('foo', libcall(libc, 'getenv', 'X_TEST_ENV_VAR_'))

This might not work on Windows.
If Vim is compiled with a recent version of MSVC, it will be linked with a newer version of msvcrXXX.dll (or other CRT dll) instead of msvcrt.dll.
Therefore let $FOO='bar' doesn't affect the return value of getenv() from msvcrt.dll.

[Dominique Pellé]

@dpelle commented on this pull request.

---

In src/testdir/test_functions.vim:

>  
-    let pid = libcallnr(libc, 'getpid', '')
-    call assert_equal(getpid(), pid)
+  let $X_TEST_ENV_VAR_ = 'foo'
+  call assert_equal('foo', libcall(libc, 'getenv', 'X_TEST_ENV_VAR_'))

Thanks. I'll do it how you suggested earlier than, i.e. by using the USERPROFILE
environment variable. The reason I tried to do it differently, is that I was hoping that
my solution would be identical on all operating systems. Clearly it's not.

I don't have my development machine right now so I'll fix the Windows test failure later today.

[Dominique Pellé]

@dpelle pushed 1 commit.

• 1a43945 Test_libcall_libcall_nr() still did not pass on Windows

—
You are receiving this because you are subscribed to this thread.

View it on GitHub or mute the thread.

[ichizok]

On macOS, can use libSystem.B.dylib.

if has('win32')
  ...
elseif has('mac')
  let libc = 'libSystem.B.dylib'
else
  ...

However, I think there are some problems on other unix-like system.

1. Location of libc

This libcall test tries to find libc from /lib or /lib64, but on Ubuntu libc is in architecture-specific directory under /lib (e.g. /lib/x86_64-linux-gnu/).
In fact, Travis-CI linux worker (Ubuntu) doesn't execute the main part of the test.

2. Consistency about 32/64 bit arch

Some systems have both 32 and 64 bit binaries/libraries on an environment, so the test must use libc of which arch is same as of vim.

Thus I think finding proper libc path is hard.
How about making shared library for libcall on testing?

[Dominique Pellé]

@dpelle pushed 1 commit.

• 1ab6979 Make Test_libcall_libcallnr() more portable

—
You are receiving this because you are subscribed to this thread.

View it on GitHub

[Dominique Pellé]

@ichizok wrote:

On macOS, can use libSystem.B.dylib

Thanks. I just did that in 692cbd641b9eaa8d0d364a345512f7de3c422fe7

Some systems have both 32 and 64 bit binaries/libraries on an environment,
so the test must use libc of which arch is same as of vim

Yes, finding the location of libc on Linux or Unix is a bit hard.
I found that using an empty string for the library location allows
to call libc functions. In other words, this appears to work:

:echo libcall("", "getenv", "HOME") 

I'm not sure how portable it is (it's not documented) but since
it avoids having to try different location of libc for Unix, I ended
up choosing this solution for the test to keep it simple.

[Bram Moolenaar]

Closed #2982 via 1ceebb4.