Re: [Tarantool-patches] [PATCH 4/4] test: enable luajit-tap:lj-flush-on-trace tests

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hi!

>>>  if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
>>> diff --git a/test/app-tap/lj-flush-on-trace.test.lua b/test/app-tap/lj-flush-on-trace.test.lua
>>> new file mode 100755
>>> index 000000000..70b7bd9a2
>>> --- /dev/null
>>> +++ b/test/app-tap/lj-flush-on-trace.test.lua
>>> @@ -0,0 +1,30 @@
>>> +#!/usr/bin/env tarantool
>>> +
>>> +local tap = require('tap')
>>> +
>>> +local test = tap.test('lj-flush-on-trace')
>>> +
>>> +local cmd = string.gsub(
>>> +  'LUA_CPATH=$/?.so LD_LIBRARY_PATH=$ tarantool 2>&1 $/test.lua %d %d',
>>> +  '%$', os.getenv('BUILDDIR') .. '/test/luajit-tap/lj-flush-on-trace')
>>> +
>>> +local checks = {
>>> +  { hotloop = 1, trigger = 1, success = true  },
>>> +  { hotloop = 1, trigger = 2, success = false },
>>> +}
>>> +
>>> +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
>>> +  assert(res, 'proc:lines failed')
>>
>> This file is exactly the same as the other file for running a
>> luajit test in the previous commit. I propose you to move this to
>> a separate file, which would provide API to run arbitrary test via
>> io.popen. Unless this won't be dropped if you find a way to make
>> the new luajit tests runable via test-run as is.
> 
> Sounds rational. They are not the same but quite similar (the difference
> is only in the result values).

At least first 24 lines of them match almost completely (except file
name to start with io.popen), that is more than half of each. I think
that part either should be simplified so as there is nothing to extract
already (my proposal about io.popen() of self may help, or may not), or
just extracted into a common file like

    test/app-tap/utils/utils.lua

You can extract even more if you pass expected output with 'checks'
array elements.

If you want to do that in scope of luajit tests rework, then add it to
the ticket description, please, so as not to forget.

> I guess I can do it in scope of #4862[1]
> if you're OK with it.
> 
>>
>>> +  if ch.success then
>>> +    test:is(res, 'OK')
>>> +  else
>>> +    test:is(res, 'JIT mode change is detected while executing the trace')
>>> +  end
>>> +end
>>> +
>>> +os.exit(test:check() and 0 or 1)
>>>
> 
> [1]: https://github.com/tarantool/tarantool/issues/4862
>