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

["Alexander V. Tikhonov" <avtikhon@tarantool.org>]

Hi Vlad, thanks for the review, I've made all the changes you suggested,
please check the updated patch with it.

On Mon, Oct 19, 2020 at 10:12:41PM +0200, Vladislav Shpilevoy wrote:
> 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.