Re: [Tarantool-patches] [PATCH v2] test: fix hanging of gh.test.lua

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

Hi! Thanks for the patch!

See 4 comments below.

On 19.10.2020 10:08, Alexander V. Tikhonov wrote:
> Found that the previously fixed vinyl/gh.test.lua test in commit
> 
>   94dc5bddc1fb0f9e6ebebfc5aa6be586e5b6759e ('test: gh test hangs after gh-4957-too-many-upserts')
> 
> with adding fiber.sleep(1) workaround to avoid of raise from the
> previously run vinyl/gh-4957-too-many-upserts.test.lua test can be
> changed in the other way. The new change from one side will leave
> the found issue untouched to be able to resolve it within opened
> issue in github. And from the other side it will let the test-run
> tool to be able to avoid of this issue using fragile list feature
> to save the stability of testing due to found issue is flaky and
> can be passed on reruns.
> 
> The current fix changes the forever waiting loop to especially
> created for such situations test_run:wait_cond() routine which has
> timeout in it to avoid of hanging the test till global timeout will
> occure. It will let the testing to be continued even after the fail.
> 
> Also added box_snapshot.test.lua() function to vinyl/gh.test.lua and
> vinyl/iterator.test.lua tests to be able to avoid of printing changing
> data in results file to be able to use its checksum in fragile list
> of test-run to be able to rerun it as flaky issues.
> Added checksum to fragile list:
> 
>   vinyl/gh.test.lua			gh-5141
>   vinyl/iterator.test.lua		gh-5141
> 
> Follows up #5141
> ---
> 
> Github: https://github.com/tarantool/tarantool/tree/avtikhon/gh-5141-gh_test
> Issue: https://github.com/tarantool/tarantool/issues/5141
> 
>  test/vinyl/gh.result         | 28 ++++++++++++++++++----------
>  test/vinyl/gh.test.lua       | 19 +++++++++++++------
>  test/vinyl/iterator.result   | 26 ++++++++++++++++++--------
>  test/vinyl/iterator.test.lua | 17 +++++++++++++----

1. In the commit title you said you change gh.test.lua. Why do you also
change iterator.test.lua?

>  test/vinyl/suite.ini         |  6 +++---
>  5 files changed, 65 insertions(+), 31 deletions(-)
> 
> diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result
> index 0fd74af83..9806f2079 100644
> --- a/test/vinyl/gh.result
> +++ b/test/vinyl/gh.result
> @@ -7,7 +7,14 @@ env = require('test_run')
>  test_run = env.new()
>  ---
>  ...
> -fiber.sleep(1)
> +function box_snapshot()                                  \
> +    local ok, err = pcall(box.snapshot)                  \
> +    if (ok == false) then                                \

2. In Lua you don't need parentheses in the 'if' condition.

> +        require('log').error(                            \
> +            "box.snapshot() failed with error: " .. err) \
> +    end                                                  \
> +    return ok                                            \
> +end;

3. ';' is not necessary either.

>  ---
>  ...
>  -- gh-283: hang after three creates and drops
> diff --git a/test/vinyl/gh.test.lua b/test/vinyl/gh.test.lua
> index c72444f5f..d2d024877 100644
> --- a/test/vinyl/gh.test.lua
> +++ b/test/vinyl/gh.test.lua
> @@ -311,7 +318,7 @@ end;
>  test_run:cmd("setopt delimiter ''");
>  
>  cont = false
> -while finished ~= 2 do fiber.sleep(0.01) end
> +test_run:wait_cond(function() fiber.sleep(0.01) return finished ~= 2 end)

4. wait_cond() does fiber.sleep for you, no need to call it manually.