Re: [Tarantool-patches] [PATCH luajit] test: adapt tests checking loading bytecode files

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

Hi, Maxim!

Thanks for the patch!

LGTM, except a few comments below.

On 24.09.21, Maxim Kokryashkin wrote:
> Loading bytecode with an extra header (BOM or "#") is disabled
> for security reasons since LuaJIT-2.0.0-beta10.
> For more information see comment for `lj_lex_setup()` in <src/lj_lex.c>.
> Also see commit 53a285c ('Disable

Nit: It is better to use the full commit hash here to avoid clashing.

> loading bytecode with an extra header (BOM or #!).').
> Tests are adapted to LuaJIT behavior.
> 
> Closes tarantool/tarantool#5691
> Part of tarantool/tarantool#5845
> Part of tarantool/tarantool#4473

Looks like it should be 5870 instead. 5873 is a unbrella for all tests
activities.

> ---

Also, please attach tarantool branch with CI to check it is OK.

> Issue: https://github.com/tarantool/tarantool/issues/5691
> Branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5691-adapt-bytecode-PUC-Rio
> 
>  test/PUC-Rio-Lua-5.1-tests/main.lua | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/test/PUC-Rio-Lua-5.1-tests/main.lua b/test/PUC-Rio-Lua-5.1-tests/main.lua
> index 07facc4c..c62950c8 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/main.lua
> +++ b/test/PUC-Rio-Lua-5.1-tests/main.lua
> @@ -172,23 +172,17 @@ prepfile(s)
>  prepfile[[#comment in 1st line without \n at the end]]
>  RUN("lua %s", prog)
>  
> --- FIXME: Loading bytecode with an extra header (BOM or "#")
> +-- Loading bytecode with an extra header (BOM or "#")
>  -- is disabled for security reasons since LuaJIT-2.0.0-beta10.
>  -- For more information see comment for `lj_lex_setup()`
>  -- in <src/lj_lex.c>.
>  -- Also see commit 53a285c0c3544ff5dea7c67b741c3c2d06d22b47
>  -- (Disable loading bytecode with an extra header (BOM or #!).).
>  -- See also https://github.com/tarantool/tarantool/issues/5691.
> --- The test is disabled for LuaJIT.
> -prepfile("#comment with a binary file\n"..string.dump(loadstring("print(1)")))
> --- RUN("lua %s > %s", prog, out)
> --- checkout("1\n")
> -
> -prepfile("#comment with a binary file\r\n"..string.dump(loadstring("print(1)")))
> --- FIXME: Behavior is different for LuaJIT. See the comment above.
> --- The test is disabled for LuaJIT.
> --- RUN("lua %s > %s", prog, out)
> --- checkout("1\n")
> +-- The test is adapted to LuaJIT behavior.
> +prepfile(string.dump(loadstring("print(1)")))
> +RUN("lua %s > %s", prog, out)
> +checkout("1\n")

May be it is better to test `NoRun()` for 1 binary file with a comment at the
first string and `Run()` for another binary file without the comment.

>  
>  -- close Lua with an open file
>  prepfile(string.format([[io.output(%q); io.write('alo')]], out))
> -- 
> 2.33.0
> 

-- 
Best regards,
Sergey Kaplun