From: Sergey Bronnikov 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/5] test: introduce `samevalues()` TAP checker
Date: Thu, 24 Aug 2023 10:44:58 +0300 [thread overview]
Message-ID: <8fb09570-4795-5412-86bd-487749e93101@tarantool.org> (raw)
In-Reply-To: <ZONAyRcX/eylqNm/@tarantool.org>
H, Igor, Sergey
On 8/21/23 13:47, Igor Munkin wrote:
> Sergey,
>
> <snipped>
>
>>>>> 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?
> Frankly speaking, it was me, who originally suggested this name (AFAIR,
> but Sergey K. might correct me if I'm wrong), so I'm totally fine with
> the naming and here why:
> 1. There are no functions named in the style you're referring to above.
> This may relate to luatest, but definitely not to our version of
> tap.lua module.
> 2. All the names *except* <is_deeply> for some historical reasons (and
> due to many Python lovers, that worked in Tarantool, I guess) are
> named in so-called "Lua-way" (you can see many examples in Lua
> Reference Manual or in popular Lua modules): short name with in lower
> case with no separators like underscore or other. This applies to
> <samevalues> too.
> 3. As for me <assert_items_equals> should validate all the items in
> table against the one *expected*, however <samevalues> just checks
> that the table consists of the same values, but nobody has to know
> this particular value.
>
> All in all, I'm OK with the current name, since it fits to the current
> naming policy. However, I'm open to other options regarding the
> assertion module to be used in our testing suite (of course, out of the
> scope of this series).
>
Igor, thanks for detailed explanation. Arguments looks reasonable for me.
I just wanted to make sure that the choice is not accidental and made
spontaneously.
Sergey, LGTM now.
next prev parent reply other threads:[~2023-08-24 7:45 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
2023-08-21 10:47 ` Igor Munkin via Tarantool-patches
2023-08-24 7:44 ` Sergey Bronnikov via Tarantool-patches [this message]
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=8fb09570-4795-5412-86bd-487749e93101@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=imun@tarantool.org \
--cc=sergeyb@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