From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <tarantool-patches-bounce@freelists.org>
Received: from localhost (localhost [127.0.0.1])
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id DEBF929E70
	for <tarantool-patches@freelists.org>; Mon, 24 Sep 2018 06:47:48 -0400 (EDT)
Received: from turing.freelists.org ([127.0.0.1])
	by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024)
	with ESMTP id rI0s2m2tcmpp for <tarantool-patches@freelists.org>;
	Mon, 24 Sep 2018 06:47:48 -0400 (EDT)
Received: from smtp61.i.mail.ru (smtp61.i.mail.ru [217.69.128.41])
	(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
	(No client certificate requested)
	by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9CF3629E05
	for <tarantool-patches@freelists.org>; Mon, 24 Sep 2018 06:47:48 -0400 (EDT)
Subject: [tarantool-patches] Re: [PATCH] lua: show locals when a tap test
 fails
References: <84509c8ab6e29159082ff27a5d72c9228adc1c21.1534595542.git.alexander.turenko@tarantool.org>
 <4ed59be1-1b73-423f-f016-d7ecb14ed907@tarantool.org>
 <20180923175917.fehrr36hxfthqgvw@tkn_work_nb>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <2034b653-1054-470d-1296-d2d74f990a7b@tarantool.org>
Date: Mon, 24 Sep 2018 13:47:45 +0300
MIME-Version: 1.0
In-Reply-To: <20180923175917.fehrr36hxfthqgvw@tkn_work_nb>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: tarantool-patches-bounce@freelists.org
Errors-to: tarantool-patches-bounce@freelists.org
Reply-To: tarantool-patches@freelists.org
List-help: <mailto:ecartis@freelists.org?Subject=help>
List-unsubscribe: <tarantool-patches-request@freelists.org?Subject=unsubscribe>
List-software: Ecartis version 1.0.0
List-Id: tarantool-patches <tarantool-patches.freelists.org>
List-subscribe: <tarantool-patches-request@freelists.org?Subject=subscribe>
List-owner: <mailto:>
List-post: <mailto:tarantool-patches@freelists.org>
List-archive: <http://www.freelists.org/archives/tarantool-patches>
To: Alexander Turenko <alexander.turenko@tarantool.org>
Cc: Eugine Blikh <bigbes@gmail.com>, tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org>

Hi! Thanks for the fixes!

>>> @@ -245,7 +280,7 @@ local function check(test)
>>>        if test.planned ~= test.total then
>>>            if test.parent ~= nil then
>>>                ok(test.parent, false, "bad plan", { planned = test.planned;
>>> -                run = test.total})
>>> +                run = test.total}, {locals = false})
>>
>> 6. Are we sure, that locals should be printed by default? Maybe
>> it would be better to print them on demand only via a global
>> option and remove per-test-case option? As I imagine a typical
>> test fail, when a test did not pass, you turn locals printing
>> on and run the test again.
>>
> 
> The primary reason of the option is to omit the second run. It is more
> useful when the fail is flaky or when it occurs in CI. If you going to
> do second run you can print anything you want manually.
> 
> So I think it worth to enable it by default or discard the patch
> entirely. Or maybe control it from an environment variable.
> 
> I personally use the patch around month locally. It sometimes gives very
> large output (graphql schemas), but saves time to write something like
> `print(require('yaml').encode(res))` each time a test fails.
> 
> If you dislike such extra output for failing tests, we should skip the
> patch entirely. It is useful or not useful, not something in the middle.

Understandable. Then LGTM, looks useful.