From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 996F04696C3 for ; Fri, 10 Apr 2020 01:05:37 +0300 (MSK) References: <0f113c2f2f746a26f85575d7904235e8920dc5a3.1585312984.git.imun@tarantool.org> <0376106b-1398-f888-16a8-1498212a2915@tarantool.org> <20200407232855.GC5713@tarantool.org> From: Vladislav Shpilevoy Message-ID: <2a709b78-4ff1-6627-4751-adcc7b38825d@tarantool.org> Date: Fri, 10 Apr 2020 00:05:35 +0200 MIME-Version: 1.0 In-Reply-To: <20200407232855.GC5713@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH 3/4] test: enable luajit-tap:gh-4427-ffi-sandwich tests List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Hi! >>> if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR}) >>> diff --git a/test/app-tap/gh-4427-ffi-sandwich.test.lua b/test/app-tap/gh-4427-ffi-sandwich.test.lua >>> new file mode 100755 >>> index 000000000..602af88d4 >>> --- /dev/null >>> +++ b/test/app-tap/gh-4427-ffi-sandwich.test.lua >>> @@ -0,0 +1,30 @@ >>> +#!/usr/bin/env tarantool >>> + >>> +local tap = require('tap') >>> + >>> +local test = tap.test('gh-4427-ffi-sandwich') >>> + >>> +local cmd = string.gsub( >>> + 'LUA_CPATH=$/?.so LD_LIBRARY_PATH=$ tarantool 2>&1 $/test.lua %d %d', >>> + '%$', os.getenv('BUILDDIR') .. '/test/luajit-tap/gh-4427-ffi-sandwich') >>> + >>> +local checks = { >>> + { hotloop = 1, trigger = 1, success = true }, >>> + { hotloop = 1, trigger = 2, success = false }, >> >> Why hotloop is needed, if it is always 1? > > I decided to localize instance configuration values and make an explicit > hotloop parameter. Since the result of the tests depends on hotloop and > trigger values ratio, it seems more convenient for further maintainence > to store these params nearby each other. After the private talk I know that hotloop is some kind of thing affecting when to start recording a trace. It is worth leaving a comment on that. And on what is trigger (about this I still don't know). >>> +} >>> + >>> +test:plan(#checks) >>> + >>> +for _, ch in pairs(checks) do >>> + local res >>> + local proc = io.popen(cmd:format(ch.hotloop, ch.trigger)) >>> + for s in proc:lines('*l') do res = s end >> >> What is '*l'? > > Considering the doc[1] this option makes iterator read the > stream line by line (the options passes to its iterator function > are exactly the same as read accepts). In the [1] I see that lines() does not take any arguments. file:read() does. file:lines() does not. Even for file:read() '*l' is default and can be omitted. >>> + assert(res, 'proc:lines failed') >>> + if ch.success then >>> + test:is(tonumber(res), ch.hotloop + ch.trigger + 1) >>> + else >>> + test:is(res, 'Lua VM re-entrancy is detected while executing the trace') >>> + end >>> +end >>> + >>> +os.exit(test:check() and 0 or 1) >> >> Oh God. So luajit already contains cyclic dependencies. It stores suite.ini >> file in its test folder, which of course is useless for the luajit alone. >> >> Since suite.ini file is here, why do you need to run tests from there in such >> a complex way? Test-run should already be able to peek tests from there (and >> it does). It probably does not work for your test only because you put it into >> a folder. I propose you to flatten it, like it is done for function1.test.lua. >> You still can keep issue name in form of 'gh-####-' in file names to keep them >> related. They would be >> >> gh-4427-ffi-sandwich.test.lua >> gh-4427-ffi-sandwich.c >> >> And after build: >> >> + gh-4427-ffi.sandwich.so/.dylib >> >> Then you don't a need separate file in app-tap. > > A separate file in app-tap is a runner for the chunk in luajit-tap > tests. Since I need to test a platform failure, I don't know other way > than run one Lua script via io.popen from another one. > > So if my math is OK we still have two Lua chunks and a C one: > * the Lua runner with io.popen and output analyzer > * the Lua script to be run by Tarantool instance > * the Dynamic library to be required by the latter Lua script and to be > build from the corresponding C file > > Could you please clarify your proposal a bit if I'm wrong or missing > something? The problem is that you have one test located in 2 folders and 3 files now. I propose to make it just 3 files. We never do any test hierarchies. Also test-run does not support nested folders with tests. Strictly speaking, you can make it 2 files: C and Lua. In Lua file you do io.popen on self with an argument, which would mean the file was started not by test-run, and which would launch the 'platform failure test'. In that way you will have all test code located in one place (not counting C file). if arg[3] == 'do_test' then jit.opt, C, print, ... os.exit() end -- arg[1] - tarantool, arg[2] - this file io.popen(string.format('%s %s ...', arg[1], arg[2], 'do_test')) test:is(...) ... > [1]: https://www.lua.org/manual/5.1/manual.html#pdf-file:read