From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp53.i.mail.ru (smtp53.i.mail.ru [94.100.177.113]) (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 DAEB6469719 for ; Tue, 20 Oct 2020 11:46:47 +0300 (MSK) Date: Tue, 20 Oct 2020 11:46:44 +0300 From: "Alexander V. Tikhonov" Message-ID: <20201020084644.GA1584639@hpalx> References: <522704d93d2574694936e85c939b6e10135951d7.1603094877.git.avtikhon@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v2] test: fix hanging of gh.test.lua List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.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.