Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 1/2] test: introduce routine to build error message
Date: Sun, 25 Feb 2024 14:37:59 +0300	[thread overview]
Message-ID: <qg7ptyc2o7dwnyckg6zhtk6sj2arwxq4yacgus7hto7amivonk@phiyhiogmaef> (raw)
In-Reply-To: <24af2ffde3f76e954582ae298a6ed5b5262d74d5.1708856482.git.imun@tarantool.org>

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/
> 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.
> produced by LuaJIT module.
Typo: s/LuaJIT/the LuaJIT/
>
> 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/
> +-- error messages.
> +local function errbuild(message)
> +    local eprefix = _TARANTOOL and "" or "[^:]+: "
IMO, `errprefix` or `err_prefix` would be better.
Feel free to ignore, though.
> +    return table.concat({"^", eprefix, message})
> +end
> +
>  local lua = _retrieve_progname()
>
>  if not pcall(io.popen, lua .. [[ -e "a=1"]]) then
> @@ -158,13 +167,13 @@ f = io.popen(cmd)
>  if compiled_with_jit then
>      equals(f:read'*l', 'Hello World', "-jon")
>  else
> -    matches(f:read'*l', "^[^:]+: JIT compiler permanently disabled by build option", "no jit")
> +    matches(f:read'*l', errbuild("JIT compiler permanently disabled by build option"), "no jit")
>  end
>  f:close()
>
>  cmd = lua .. " -j bad hello-411.lua 2>&1"
>  f = io.popen(cmd)
> -matches(f:read'*l', "^[^:]+: unknown luaJIT command or jit%.%* modules not installed", "-j bad")
> +matches(f:read'*l', errbuild("unknown luaJIT command or jit%.%* modules not installed"), "-j bad")
>  f:close()
>
>  if compiled_with_jit then
> @@ -190,12 +199,12 @@ if compiled_with_jit then
>
>      cmd = lua .. " -O+bad hello-411.lua 2>&1"
>      f = io.popen(cmd)
> -    matches(f:read'*l', "^[^:]+: unknown or malformed optimization flag '%+bad'", "-O+bad")
> +    matches(f:read'*l', errbuild("unknown or malformed optimization flag '%+bad'"), "-O+bad")
>      f:close()
>  else
>      cmd = lua .. " -O0 hello-411.lua 2>&1"
>      f = io.popen(cmd)
> -    matches(f:read'*l', "^[^:]+: attempt to index a nil value")
> +    matches(f:read'*l', errbuild("attempt to index a nil value"))
>      f:close()
>  end
>
> --
> 2.39.2
>

  reply	other threads:[~2024-02-25 11:38 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-25 10:41 [Tarantool-patches] [PATCH luajit 0/2] Enable CLI-related tests in lua-Harness Igor Munkin via Tarantool-patches
2024-02-25 10:41 ` [Tarantool-patches] [PATCH luajit 1/2] test: introduce routine to build error message Igor Munkin via Tarantool-patches
2024-02-25 11:37   ` Maxim Kokryashkin via Tarantool-patches [this message]
2024-02-25 19:27     ` Igor Munkin via Tarantool-patches
2024-02-25 10:41 ` [Tarantool-patches] [PATCH luajit 2/2] test: enable CLI-related lua-Harness tests back Igor Munkin via Tarantool-patches
2024-02-25 11:41   ` Maxim Kokryashkin via Tarantool-patches
2024-02-25 19:31     ` Igor Munkin via Tarantool-patches
2024-02-25 19:41       ` Igor Munkin via Tarantool-patches
2024-02-27  8:36 ` [Tarantool-patches] [PATCH luajit 0/2] Enable CLI-related tests in lua-Harness Sergey Kaplun via Tarantool-patches
2024-02-28 18:25 ` Sergey Kaplun via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=qg7ptyc2o7dwnyckg6zhtk6sj2arwxq4yacgus7hto7amivonk@phiyhiogmaef \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 1/2] test: introduce routine to build error message' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox