Re: [Tarantool-patches] [PATCH luajit 1/2] test: introduce routine to build error message

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

Max,

Thanks for your review! I've fixed all the nits you've left, the branch
is force-pushed.

On 25.02.24, Maxim Kokryashkin wrote:
> Hi, Igor!
> Thanks for the patch!
> LGTM, except for a few comments below.
> 
> On Sun, Feb 25, 2024 at 10:41:06AM +0000, Igor Munkin wrote:
> > LuaJIT modules yields the plain errors, but LuaJIT binary adds 'luajit:'
> Typo: s/yields/yield/

Fixed, thanks!

> > prefix to it, so 411-luajit.t test in lua-Harness suite expects the
> > error message with the aforementioned prefix in the corresponding
> > assertions. At the same time, Tarantool prepends nothing to the error
> Typo: s/prepends/adds/
> Or, alternatively, you can say:
> | Tarantool doesn't prepend errors produced by the LuaJIT module with
> | anything.

Thanks, chose yours wording.

> > produced by LuaJIT module.
> Typo: s/LuaJIT/the LuaJIT/

Fixed by your wording.

> >
> > To tweak the pattern to be used within the 411-luajit.t chunk, the
> > auxiliary error building function is introduced in this patch.
> >
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  test/lua-Harness-tests/411-luajit.t | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
> > index 6cfd6837..1b2da729 100755
> > --- a/test/lua-Harness-tests/411-luajit.t
> > +++ b/test/lua-Harness-tests/411-luajit.t
> > @@ -31,6 +31,15 @@ if not jit or ujit or _TARANTOOL then
> >      skip_all("only with LuaJIT")
> >  end
> >
> > +-- XXX: Unfortunately, Lua patterns do not support optional
> > +-- capture groups, so the helper below implements poor man's
> > +-- optional capture groups for the patters matching LuaJIT CLI
> Typo: s/patters/patterns/

Fixed, thanks!

> > +-- error messages.
> > +local function errbuild(message)
> > +    local eprefix = _TARANTOOL and "" or "[^:]+: "
> IMO, `errprefix` or `err_prefix` would be better.
> Feel free to ignore, though.

No problem, fixed. The diff is below:

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

diff --git a/test/lua-Harness-tests/411-luajit.t b/test/lua-Harness-tests/411-luajit.t
index 414e6a57..40241db3 100755
--- a/test/lua-Harness-tests/411-luajit.t
+++ b/test/lua-Harness-tests/411-luajit.t
@@ -36,8 +36,8 @@ end
 -- optional capture groups for the patters matching LuaJIT CLI
 -- error messages.
 local function errbuild(message)
-    local eprefix = _TARANTOOL and "" or "[^:]+: "
-    return table.concat({"^", eprefix, message})
+    local errprefix = _TARANTOOL and "" or "[^:]+: "
+    return table.concat({"^", errprefix, message})
 end
 
 local lua = _retrieve_progname()

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

> > +    return table.concat({"^", eprefix, message})
> > +end
> > +
> >  local lua = _retrieve_progname()
> >
> >  if not pcall(io.popen, lua .. [[ -e "a=1"]]) then

<snipped>

> > --
> > 2.39.2
> >

-- 
Best regards,
IM