Re: [Tarantool-patches] [PATCH luajit 3/7] test: stop using utils.selfrun in tests

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Max,

Thanks for your review! See my answers below.

On 16.02.23, Maxim Kokryashkin wrote:
> 
> Hi, Igor!
> LGTM, except for a few nits below.
>  

<snipped>

> >>diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>index dd02130c..f4795db0 100644
> >>--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> >>@@ -3,52 +3,43 @@ local utils = require('utils')
> ><snipped>
> >>+ -- TODO: Leave another toxic comment regarding SIP on macOS.
> >That comment is unnecessary. Here and below.

================================================================================

diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
index ff3eaf01..ad06c329 100644
--- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
+++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
@@ -6,8 +6,23 @@ local test = tap.test('gh-4427-ffi-sandwich'):skipcond({
 
 test:plan(2)
 
+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
 local script = require('utils').makecmd(arg, {
-  -- TODO: Leave another toxic comment regarding SIP on macOS.
+  -- XXX: Apple tries their best to "protect their users from
+  -- malware". As a result SIP (see the link[1] below) has been
+  -- designed and released. Now, Apple developers are so
+  -- protected, that they can load nothing being not installed in
+  -- the system, since the environment is sanitized before the
+  -- child process is launched. In particular, environment
+  -- variables starting with DYLD_ and LD_ are unset for child
+  -- process. For more info, see the docs[2] below.
+  --
+  -- The environment variable below is used by FFI machinery to
+  -- find the proper shared library.
+  --
+  -- [1]: https://support.apple.com/en-us/HT204899
+  -- [2]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
   env = { DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH') },
   redirect = '2>&1',
 })

================================================================================

> > 
> ><snipped>
> >>
> >>diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> >>index eb11d40d..41a7c22a 100644
> >>--- a/test/tarantool-tests/utils.lua
> >>+++ b/test/tarantool-tests/utils.lua

<snipped>

> >>+ }, {
> >>+ __call = function(self, ...)
> >>+ local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
> >>+ .. (' %s'):rep(select('#', ...)):format(...)
> >>+ return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
> >>+ end
> >>+ })
> >That part is barely readable, so I think you should either drop
> >a comment with explanation, or rewrite it in a more readable way.

Well, yeah... my Perl background is coming out ;)

Anyway, here is the comment:

================================================================================

diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
index ac04c01d..f5e08043 100644
--- a/test/tarantool-tests/tap.lua
+++ b/test/tarantool-tests/tap.lua
@@ -60,8 +65,17 @@ function M.makecmd(arg, opts)
     REDIRECT = opts and opts.redirect or '',
   }, {
     __call = function(self, ...)
+      -- This line just makes the command for <io.popen> by the
+      -- following steps:
+      -- 1. Replace the placeholders with the corresponding values
+      --    given to the command constructor (e.g. script, env)
+      -- 2. Join all CLI arguments given to the __call metamethod
+      -- 3. Concatenate the results of step 1 and step 2 to obtain
+      --    the resulting command.
       local cmd = ('<ENV> <LUABIN> <REDIRECT> <SCRIPT>'):gsub('%<(%w+)>', self)
                   .. (' %s'):rep(select('#', ...)):format(...)
+      -- Trim both leading and trailing whitespace from the output
+      -- produced by the child process.
       return io.popen(cmd):read('*all'):gsub('^%s+', ''):gsub('%s+$', '')
     end
   })

================================================================================

> >> end
> >> 
> >> function M.skipcond(condition, message)
> >>--
> >>2.30.2
> >--
> >Best regards,
> >Maxim Kokryashkin
> > 

-- 
Best regards,
IM