From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 A1ED6469719 for ; Mon, 19 Oct 2020 23:12:44 +0300 (MSK) References: <522704d93d2574694936e85c939b6e10135951d7.1603094877.git.avtikhon@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Mon, 19 Oct 2020 22:12:41 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: "Alexander V. Tikhonov" , Kirill Yukhin Cc: tarantool-patches@dev.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.