[PATCH] test: fix app-tap/tarantoolctl sporadic failure

[PATCH] test: fix app-tap/tarantoolctl sporadic failure

[Serge Petrenko]

In rare cases this test failed on `tarantoolctl status` after
`tarantoolctl stop` due to `tarantoolctl stop` taking too much time to
unlink pid file. Fix this by waiting for pid file deletion in test.

Closes #3557
---
https://github.com/tarantool/tarantool/issues/3557
https://github.com/tarantool/tarantool/tree/sp/gh-3557-tarantoolctl-test-fix


 test/app-tap/tarantoolctl.test.lua | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index 6946c8312..83a8bfc37 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -34,6 +34,13 @@ local function recursive_rmdir(path)
     end
 end
 
+local function wait_delete(path)
+    path = fio.abspath(path)
+    while fio.path.exists(path) do
+	fiber.sleep(0.0001)
+    end
+end
+
 ffi.cdef[[
 typedef int32_t pid_t;
 int kill(pid_t pid, int sig);
@@ -157,6 +164,7 @@ test:plan(6)
 do
     local dir = fio.tempdir()
     local code = [[ box.cfg{memtx_memory = 104857600} ]]
+    local pid_path = fio.pathjoin(dir, 'script.pid')
     create_script(dir, 'script.lua', code)
 
     local status, err = pcall(function()
@@ -168,6 +176,7 @@ do
             check_ok(test_i, dir, 'start',  'script', 1, nil, "is already running")
             check_ok(test_i, dir, 'status', 'script', 0, nil, "is running")
             check_ok(test_i, dir, 'stop',   'script', 0, nil, "Stopping")
+            wait_delete(pid_path)
             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped")
             check_ok(test_i, dir, 'stop',   'script', 0, nil, "is not running")
             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped" )
-- 
2.15.2 (Apple Git-101.1)

Re: [PATCH] test: fix app-tap/tarantoolctl sporadic failure

[Vladimir Davydov]

On Thu, Aug 16, 2018 at 06:24:00PM +0300, Serge Petrenko wrote:
> In rare cases this test failed on `tarantoolctl status` after
> `tarantoolctl stop` due to `tarantoolctl stop` taking too much time to
> unlink pid file.

This is too vague for an explanation. AFAIU the problem here is that the
pid file is deleted by tarantool (from tarantool_free => pidfile_remove)
while the control socket file is deleted by tarantoolctl so if
`tarantoolctl status` is called immediately after `tarantoolctl stop`,
there's a chance tarantool hasn't exited yet and so the pid file still
exists.

Please update the comment.

> Fix this by waiting for pid file deletion in test.
> 
> Closes #3557
> ---
> https://github.com/tarantool/tarantool/issues/3557
> https://github.com/tarantool/tarantool/tree/sp/gh-3557-tarantoolctl-test-fix
> 
> 
>  test/app-tap/tarantoolctl.test.lua | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
> index 6946c8312..83a8bfc37 100755
> --- a/test/app-tap/tarantoolctl.test.lua
> +++ b/test/app-tap/tarantoolctl.test.lua
> @@ -34,6 +34,13 @@ local function recursive_rmdir(path)
>      end
>  end
>  
> +local function wait_delete(path)
> +    path = fio.abspath(path)

You don't need to call abspath here.

> +    while fio.path.exists(path) do
> +	fiber.sleep(0.0001)

A tab! Please fix your editor.

Also, please use fiber.sleep(0.01) for consistency with other timeouts
used in this script.

> +    end
> +end
> +

There's tctl_wait() helper that waits for tarantool to start.
Let's rename it to tctl_wait_start() and introduce tctl_wait_stop()
with the same signature that would wait for tarantool to stop.

>  ffi.cdef[[
>  typedef int32_t pid_t;
>  int kill(pid_t pid, int sig);
> @@ -157,6 +164,7 @@ test:plan(6)
>  do
>      local dir = fio.tempdir()
>      local code = [[ box.cfg{memtx_memory = 104857600} ]]
> +    local pid_path = fio.pathjoin(dir, 'script.pid')
>      create_script(dir, 'script.lua', code)
>  
>      local status, err = pcall(function()
> @@ -168,6 +176,7 @@ do
>              check_ok(test_i, dir, 'start',  'script', 1, nil, "is already running")
>              check_ok(test_i, dir, 'status', 'script', 0, nil, "is running")
>              check_ok(test_i, dir, 'stop',   'script', 0, nil, "Stopping")
> +            wait_delete(pid_path)
>              check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped")
>              check_ok(test_i, dir, 'stop',   'script', 0, nil, "is not running")
>              check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped" )

Re: [tarantool-patches] Re: [PATCH] test: fix app-tap/tarantoolctl sporadic failure

[Serge Petrenko]

Hi! All fixed, new diff is below.

> 16 авг. 2018 г., в 22:42, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Thu, Aug 16, 2018 at 06:24:00PM +0300, Serge Petrenko wrote:
>> In rare cases this test failed on `tarantoolctl status` after
>> `tarantoolctl stop` due to `tarantoolctl stop` taking too much time to
>> unlink pid file.
> 
> This is too vague for an explanation. AFAIU the problem here is that the
> pid file is deleted by tarantool (from tarantool_free => pidfile_remove)
> while the control socket file is deleted by tarantoolctl so if
> `tarantoolctl status` is called immediately after `tarantoolctl stop`,
> there's a chance tarantool hasn't exited yet and so the pid file still
> exists.
> 
> Please update the comment.
> 
>> Fix this by waiting for pid file deletion in test.
>> 
>> Closes #3557
>> ---
>> https://github.com/tarantool/tarantool/issues/3557
>> https://github.com/tarantool/tarantool/tree/sp/gh-3557-tarantoolctl-test-fix
>> 
>> 
>> test/app-tap/tarantoolctl.test.lua | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>> 
>> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
>> index 6946c8312..83a8bfc37 100755
>> --- a/test/app-tap/tarantoolctl.test.lua
>> +++ b/test/app-tap/tarantoolctl.test.lua
>> @@ -34,6 +34,13 @@ local function recursive_rmdir(path)
>>     end
>> end
>> 
>> +local function wait_delete(path)
>> +    path = fio.abspath(path)
> 
> You don't need to call abspath here.
> 
>> +    while fio.path.exists(path) do
>> +	fiber.sleep(0.0001)
> 
> A tab! Please fix your editor.
> 
> Also, please use fiber.sleep(0.01) for consistency with other timeouts
> used in this script.
> 
>> +    end
>> +end
>> +
> 
> There's tctl_wait() helper that waits for tarantool to start.
> Let's rename it to tctl_wait_start() and introduce tctl_wait_stop()
> with the same signature that would wait for tarantool to stop.
> 
>> ffi.cdef[[
>> typedef int32_t pid_t;
>> int kill(pid_t pid, int sig);
>> @@ -157,6 +164,7 @@ test:plan(6)
>> do
>>     local dir = fio.tempdir()
>>     local code = [[ box.cfg{memtx_memory = 104857600} ]]
>> +    local pid_path = fio.pathjoin(dir, 'script.pid')
>>     create_script(dir, 'script.lua', code)
>> 
>>     local status, err = pcall(function()
>> @@ -168,6 +176,7 @@ do
>>             check_ok(test_i, dir, 'start',  'script', 1, nil, "is already running")
>>             check_ok(test_i, dir, 'status', 'script', 0, nil, "is running")
>>             check_ok(test_i, dir, 'stop',   'script', 0, nil, "Stopping")
>> +            wait_delete(pid_path)
>>             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped")
>>             check_ok(test_i, dir, 'stop',   'script', 0, nil, "is not running")
>>             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped" )
> 

 test/app-tap/tarantoolctl.test.lua | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index 6946c8312..cc4ca17b6 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -86,7 +86,7 @@ local function run_command(dir, command)
     return res/256, fstdout_e, fstderr_e
 end
 
-local function tctl_wait(dir, name)
+local function tctl_wait_start(dir, name)
     if name then
         local path = fio.pathjoin(dir, name .. '.control')
         while not fio.stat(path) do
@@ -113,6 +113,13 @@ local function tctl_wait(dir, name)
     end
 end
 
+local function tctl_wait_stop(dir, name)
+    local path = fio.pathjoin(dir, name .. '.pid')
+    while fio.path.exists(path) do
+        fiber.sleep(0.01)
+    end
+end
+
 local function tctl_command(dir, cmd, args, name)
     local pid = nil
     if not fio.stat(fio.pathjoin(dir, '.tarantoolctl')) then
@@ -163,11 +170,12 @@ do
         test:test("basic test", function(test_i)
             test_i:plan(16)
             check_ok(test_i, dir, 'start',  'script', 0, nil, "Starting instance")
-            tctl_wait(dir, 'script')
+            tctl_wait_start(dir, 'script')
             check_ok(test_i, dir, 'status', 'script', 0, nil, "is running")
             check_ok(test_i, dir, 'start',  'script', 1, nil, "is already running")
             check_ok(test_i, dir, 'status', 'script', 0, nil, "is running")
             check_ok(test_i, dir, 'stop',   'script', 0, nil, "Stopping")
+            tctl_wait_stop(dir, 'script')
             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped")
             check_ok(test_i, dir, 'stop',   'script', 0, nil, "is not running")
             check_ok(test_i, dir, 'status', 'script', 1, nil, "is stopped" )
@@ -200,7 +208,7 @@ do
             check_ok(test_i, dir, 'start', 'bad_script', 1, nil,
                      'unexpected symbol near')
             check_ok(test_i, dir, 'start', 'good_script', 0)
-            tctl_wait(dir, 'good_script')
+            tctl_wait_start(dir, 'good_script')
             -- wait here
             check_ok(test_i, dir, 'eval',  'good_script bad_script.lua', 3,
                      nil, 'Error while reloading config:')
@@ -232,7 +240,7 @@ do
         test:test("check answers in case of call", function(test_i)
             test_i:plan(6)
             check_ok(test_i, dir, 'start', 'good_script', 0)
-            tctl_wait(dir, 'good_script')
+            tctl_wait_start(dir, 'good_script')
             check_ok(test_i, dir, 'eval',  'good_script bad_script.lua', 3, nil,
                      'Error while reloading config')
             check_ok(test_i, dir, 'eval',  'good_script ok_script.lua', 0,
-- 
2.15.2 (Apple Git-101.1)

Re: [tarantool-patches] Re: [PATCH] test: fix app-tap/tarantoolctl sporadic failure

[Vladimir Davydov]

Pushed to 1.10