From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 3A88746970F
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 26 Nov 2019 19:54:11 +0300 (MSK)
Date: Tue, 26 Nov 2019 19:54:08 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20191126165407.ncmd7hepnpifgv6v@tkn_work_nb>
References: <a9d19273e8103ffd8ac07ced93a8b97dff770417.1574782721.git.imun@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <a9d19273e8103ffd8ac07ced93a8b97dff770417.1574782721.git.imun@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH] test: adjust the test name related
	to PAIRSMM flag
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.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.