From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>, Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker
Date: Fri, 18 Aug 2023 14:12:17 +0300 [thread overview]
Message-ID: <1b754f01-34f8-d1a5-504a-fa825f555065@tarantool.org> (raw)
In-Reply-To: <ZN9O6iTYOGrI-orY@root>
Hi,
On 8/18/23 13:58, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Please, consider my comments below.
>
> On 18.08.23, Sergey Bronnikov wrote:
>> 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_
> As you can see we use naming without _ for exported function in the
> <tap.lua> module, so additional one with strange naming will be
> inconsistent.
>
> Also, discussed this naming with Igor and Max offline and this name is
> OK for them, feel free also to CC Igor to discuss:).
>
>> 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
>>
>>
Igor, what do you think regarding naming of the introduced function?
next prev parent reply other threads:[~2023-08-18 11:12 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-15 9:36 [Tarantool-patches] [PATCH luajit 0/5] Fix pow inconsistencies and improve asserts Sergey Kaplun via Tarantool-patches
2023-08-15 9:36 ` [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker Sergey Kaplun via Tarantool-patches
2023-08-17 14:03 ` Maxim Kokryashkin via Tarantool-patches
2023-08-17 15:03 ` Sergey Kaplun via Tarantool-patches
2023-08-18 10:43 ` Sergey Bronnikov via Tarantool-patches
2023-08-18 10:58 ` Sergey Kaplun via Tarantool-patches
2023-08-18 11:12 ` Sergey Bronnikov via Tarantool-patches [this message]
2023-08-21 10:47 ` Igor Munkin via Tarantool-patches
2023-08-24 7:44 ` Sergey Bronnikov via Tarantool-patches
2023-08-15 9:36 ` [Tarantool-patches] [PATCH luajit 2/5] Remove pow() splitting and cleanup backends Sergey Kaplun via Tarantool-patches
2023-08-17 14:52 ` Maxim Kokryashkin via Tarantool-patches
2023-08-17 15:33 ` Sergey Kaplun via Tarantool-patches
2023-08-20 9:48 ` Maxim Kokryashkin via Tarantool-patches
2023-08-18 11:08 ` Sergey Bronnikov via Tarantool-patches
2023-08-15 9:36 ` [Tarantool-patches] [PATCH luajit 3/5] Improve assertions Sergey Kaplun via Tarantool-patches
2023-08-17 14:58 ` Maxim Kokryashkin via Tarantool-patches
2023-08-18 7:56 ` Sergey Kaplun via Tarantool-patches
2023-08-18 11:20 ` Sergey Bronnikov via Tarantool-patches
2023-08-15 9:36 ` [Tarantool-patches] [PATCH luajit 4/5] Fix pow() optimization inconsistencies Sergey Kaplun via Tarantool-patches
2023-08-18 12:45 ` Sergey Bronnikov via Tarantool-patches
2023-08-21 8:07 ` Sergey Kaplun via Tarantool-patches
2023-08-20 9:26 ` Maxim Kokryashkin via Tarantool-patches
2023-08-21 8:06 ` Sergey Kaplun via Tarantool-patches
2023-08-21 9:00 ` Maxim Kokryashkin via Tarantool-patches
2023-08-21 9:31 ` Sergey Kaplun via Tarantool-patches
2023-08-15 9:36 ` [Tarantool-patches] [PATCH luajit 5/5] Revert to trival pow() optimizations to prevent inaccuracies Sergey Kaplun via Tarantool-patches
2023-08-18 12:49 ` Sergey Bronnikov via Tarantool-patches
2023-08-21 8:16 ` Sergey Kaplun via Tarantool-patches
2023-08-20 9:37 ` Maxim Kokryashkin via Tarantool-patches
2023-08-21 8:15 ` Sergey Kaplun via Tarantool-patches
2023-08-21 9:06 ` Maxim Kokryashkin via Tarantool-patches
2023-08-21 9:36 ` Sergey Kaplun via Tarantool-patches
2023-08-24 7:47 ` [Tarantool-patches] [PATCH luajit 0/5] Fix pow inconsistencies and improve asserts Sergey Bronnikov via Tarantool-patches
2023-08-31 15:18 ` Igor Munkin 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=1b754f01-34f8-d1a5-504a-fa825f555065@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=imun@tarantool.org \
--cc=sergeyb@tarantool.org \
--cc=skaplun@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker' \
/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