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

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Igor!
Thanks for the fixes!
LGTM, just a few minor nits below.

On 27.02.23, Igor Munkin wrote:
> Unfortunately, <utils.selfrun> is too complex to be maintained, so the
> corresponding tests are split into two files: the test itself and the
> script to be run by the test. As a result of the patch <utils.makecmd>
> helper is introduced: it inherits some approaches from <utils.selfrun>,
> but it's considered for more general use.
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
>  .../gh-4427-ffi-sandwich.test.lua             | 88 +++++++++++--------
>  .../gh-4427-ffi-sandwich/script.lua           | 25 ++++++
>  .../lj-351-print-tostring-number.test.lua     | 34 +++----
>  .../lj-351-print-tostring-number/script.lua   |  9 ++
>  .../lj-586-debug-non-string-error.test.lua    |  2 +-
>  .../lj-flush-on-trace.test.lua                | 87 ++++++++++--------
>  .../lj-flush-on-trace/script.lua              | 23 +++++
>  test/tarantool-tests/utils.lua                | 80 +++++++----------
>  8 files changed, 200 insertions(+), 148 deletions(-)
>  create mode 100644 test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
>  create mode 100644 test/tarantool-tests/lj-351-print-tostring-number/script.lua
>  create mode 100644 test/tarantool-tests/lj-flush-on-trace/script.lua
> 
> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index dd02130c..06985dcd 100644
> --- a/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua

<snipped>

> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua
> new file mode 100644
> index 00000000..9ecd964e
> --- /dev/null
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich/script.lua

<snipped>

> +jit.opt.start("3", string.format("hotloop=%d", hotloop))

Do we need set level 3 direct here?
Also, I suggest to use single quotes according to our coding style in
tests.

> +

<snipped>

> diff --git a/test/tarantool-tests/lj-351-print-tostring-number.test.lua b/test/tarantool-tests/lj-351-print-tostring-number.test.lua
> index da5b31be..72a9ec2b 100644
> --- a/test/tarantool-tests/lj-351-print-tostring-number.test.lua
> +++ b/test/tarantool-tests/lj-351-print-tostring-number.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-351-print-tostring-number/script.lua b/test/tarantool-tests/lj-351-print-tostring-number/script.lua
> new file mode 100644
> index 00000000..c3066f49
> --- /dev/null
> +++ b/test/tarantool-tests/lj-351-print-tostring-number/script.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
> index f02353fe..dcb730a2 100644
> --- a/test/tarantool-tests/lj-586-debug-non-string-error.test.lua
> +++ b/test/tarantool-tests/lj-586-debug-non-string-error.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> index c46b93f0..3351cc5a 100644
> --- a/test/tarantool-tests/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua

<snipped>

> diff --git a/test/tarantool-tests/lj-flush-on-trace/script.lua b/test/tarantool-tests/lj-flush-on-trace/script.lua
> new file mode 100644
> index 00000000..d2c35534
> --- /dev/null
> +++ b/test/tarantool-tests/lj-flush-on-trace/script.lua

<snipped>

> +jit.opt.start("3", string.format("hotloop=%d", hotloop))

Do we need set level 3 direct here?
Also, I suggest to use single quotes according to our coding style in
tests.

> +

<snipped>

> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> index eb11d40d..8355149b 100644
> --- a/test/tarantool-tests/utils.lua
> +++ b/test/tarantool-tests/utils.lua

<snipped>

> +    SCRIPT = opts and opts.script or arg[0]:gsub('%.test%.lua$', '/script.lua'),
> +    ENV = opts and makeenv(opts.env) or '',
> +    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.

Extra dot at the end of the sentence (or missing dots for previous
bullets).

> +      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,
Sergey Kaplun