Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] test: adjust the test name related to PAIRSMM flag
Date: Tue, 26 Nov 2019 19:54:08 +0300	[thread overview]
Message-ID: <20191126165407.ncmd7hepnpifgv6v@tkn_work_nb> (raw)
In-Reply-To: <a9d19273e8103ffd8ac07ced93a8b97dff770417.1574782721.git.imun@tarantool.org>

On Tue, Nov 26, 2019 at 06:47:38PM +0300, Igor Munkin wrote:
> The prior name doesn't respect the naming convention used by test-run
> thus this chunk was ignored by our testing machinery.
> 
> Part-of tarantool/tarantool#4560
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> Branch: https://github.com/tarantool/luajit/tree/imun/test-name-fixup
> Issue: https://github.com/tarantool/tarantool/issues/4560
> 
> 
>  test/{mmpairs_test.lua => mmpairs-tarantool-4560.test.lua} | 0
>  1 file changed, 0 insertions(+), 0 deletions(-)
>  rename test/{mmpairs_test.lua => mmpairs-tarantool-4560.test.lua} (100%)
> 
> diff --git a/test/mmpairs_test.lua b/test/mmpairs-tarantool-4560.test.lua
> similarity index 100%
> rename from test/mmpairs_test.lua
> rename to test/mmpairs-tarantool-4560.test.lua
> -- 
> 2.24.0
> 

Eagle eye!

LGTM.

Please, ensure that the test name will be updated in tarantool (with the
submodule update) only after we'll enable LUAJIT_ENABLE_PAIRSMM macro in
tarantool for luajit's build.

Several comments around are below.

----

Re test itself:

* Use `local` for variables (`tap`, `test`).
* Use `os.exit(test:check() and 0 or 1)` at the end.

The test depends on tarantool indirectly: we (will) set
LUAJIT_ENABLE_PAIRSMM macro in tarantool when building LuaJIT, so the
test formally may not be applicable for luajit itself (despite that it
is placed in the luajit's repo).

I don't see real problems here, to be honest; just feel it a bit less
strict then it maybe should be.

We discussed possible ways to skip the test when it will not be
appropriate with Igor and they are the following.

* mmpairs-tarantool-4560.skipcond Python script that look into some
  generated header (don't know whether luajit has it) to determine
  options with which luajit was built.
* mmpairs-tarantool-4560.skipcond that calls, say, `nm` to deduce build
  flags from symbols in luajit executable.
* Look at _TARANTOOL global variable from the test and do a couple of
  test:skip().

The latter is simplest.

Anyway, it is not very important now and surely should not be part of
this patch. Let's skip all those thoughts or add the simplest check
separately if you wish.

WBR, Alexander Turenko.

  reply	other threads:[~2019-11-26 16:54 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-26 15:47 Igor Munkin
2019-11-26 16:54 ` Alexander Turenko [this message]
2019-11-26 20:58   ` Konstantin Osipov
2019-11-26 21:32     ` Igor Munkin
2019-11-27  8:32       ` Konstantin Osipov
2019-11-26 21:03   ` Vladislav Shpilevoy
2019-11-26 21:10     ` Igor Munkin
2019-11-27 15:50   ` Igor Munkin

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=20191126165407.ncmd7hepnpifgv6v@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=imun@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] test: adjust the test name related to PAIRSMM flag' \
    /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