From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
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 <tarantool-patches@dev.tarantool.org>;
Mon, 19 Oct 2020 23:12:44 +0300 (MSK)
References: <522704d93d2574694936e85c939b6e10135951d7.1603094877.git.avtikhon@tarantool.org>
<aad6c1bfda24455f1fa720ad66aaf41aaf8c7653.1603094877.git.avtikhon@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <e9f1289b-1fb0-1eba-8e8f-40e0110b30b2@tarantool.org>
Date: Mon, 19 Oct 2020 22:12:41 +0200
MIME-Version: 1.0
In-Reply-To: <aad6c1bfda24455f1fa720ad66aaf41aaf8c7653.1603094877.git.avtikhon@tarantool.org>
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 <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: "Alexander V. Tikhonov" <avtikhon@tarantool.org>, Kirill Yukhin <kyukhin@tarantool.org>
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.