Re: [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker

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

Hi, Sergey


Thanks for the patch! See my comments inline.


On 8/15/23 12:36, Sergey Kaplun wrote:
> The introduced `samevalues()` helper checks that values in range from
> 1, to `table.maxn()` of the given table are exactly the same. It may be
> usefull for test consistency of JIT and VM behaviour. Originally, the
> `arr_is_consistent()` function was introduced in the
> <tarantool-tests/gh-6163-min-max.test.lua>. `samevalues()` has the same
> functionallity (except usage of `table.maxn()` instead `#` operator to
> be sure, that the table we check isn't a sparse array).

I would rename samevalues to something like assert_equals or 
assert_items_equals just because

similar functions are named in unit testing frameworks and helpers with 
prefix assert_

more readable from my point of view. See names for assertions in luatest 
[1] and JUnit (popular unit testing framework).


1. https://github.com/tarantool/luatest#list-of-luatest-functions

2. 
https://junit.org/junit5/docs/5.0.1/api/org/junit/jupiter/api/Assertions.html

> ---
>   test/tarantool-tests/gh-6163-min-max.test.lua | 52 ++++++++-----------
>   test/tarantool-tests/tap.lua                  | 14 +++++
>   2 files changed, 37 insertions(+), 29 deletions(-)
>
> diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua
> index 63437955..4bc6155c 100644
> --- a/test/tarantool-tests/gh-6163-min-max.test.lua
> +++ b/test/tarantool-tests/gh-6163-min-max.test.lua
> @@ -2,25 +2,17 @@ local tap = require('tap')
>   local test = tap.test('gh-6163-jit-min-max'):skipcond({
>     ['Test requires JIT enabled'] = not jit.status(),
>   })
> +
Nit: Unneeded change.

<snipped>


> diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua
> index 8559ee52..af1d4b20 100644
> --- a/test/tarantool-tests/tap.lua
> +++ b/test/tarantool-tests/tap.lua
> @@ -254,6 +254,19 @@ local function iscdata(test, v, ctype, message, extra)
>     return ok(test, ffi.istype(ctype, v), message, extra)
>   end
>   
> +local function isnan(v)
> +  return v ~= v
> +end
> +

I would put isnan() to utils, because it is not related to TAP module 
actually and

can be useful for other tests too and exporting it in tap module will be 
strange.


> +local function samevalues(test, got, message, extra)
> +  for i = 1, table.maxn(got) - 1 do
> +    if got[i] ~= got[i + 1] and not (isnan(got[i]) and isnan(got[i + 1])) then
> +      return fail(test, message, extra)
> +    end
> +  end
> +  return ok(test, true, message, extra)
> +end
> +
>   local test_mt
>   
>   local function new(parent, name, fun, ...)
> @@ -372,6 +385,7 @@ test_mt = {
>       isudata    = isudata,
>       iscdata    = iscdata,
>       is_deeply  = is_deeply,
> +    samevalues = samevalues,
>       like       = like,
>       unlike     = unlike,
>     }