[tarantool-patches] Re: [PATCH] lua: show locals when a tap test fails

[Alexander Turenko <alexander.turenko@tarantool.org>]

On Mon, Sep 24, 2018 at 01:47:45PM +0300, Vladislav Shpilevoy wrote:
> 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.
> 

To be honest, now I don't so sure in the thought that we should enable
it by default or discard completely. Sometimes it is useful to have
clean picture which cases are passed and which are not w/o extra
information (w/o locals and w/o traces). But sometimes it is useful to
know exact reason of a certain fail and have as much info as possible
(so locals are helpful).

When work on a patch locally both situations occur. Don't sure about CI.

Now I thinking about the following implementation: save locals for a
first failing case, print it after the whole test in the uppermost
test:check() and then print cases list (with ok/not ok, name and so on)
again to bring it to the focus. It gives both benefits: a developer does
not have to scroll for pass/fail list and have detailed info available
after some scrolling.

We could simplify that to 'print traces and locals during the test, then
give a short list in the uppermost test:check()', but there is a
pitfall. When something (say, a test case preparation) lead to premature
exit (say, because of a raised error) we still want to see the whole
picture (what is passed and what is failed) at the first glance and
then, maybe, detailed information about the fail. With the proposed
implementation we'll get the cases list, but not details: I think it is
better then vice versa.

So, to sum it up, I propose to keep a during-test output short, then, if
the test reach the uppermost test:check(), give details and repeat the
short cases list.

Now I think how better to handle the raised error case. Maybe it worth
to do xpcall around the whole test and then traverse over a stack to
find a frame with the uppermost test and print locals for that frame.
I'm thinking whether it is implementable w/o extra handle from a test
writer, say, as a global option (_G.print_test_locals_before_exit) of
tarantool that can be set by test-run.

Ugh, tricky thing...

WBR, Alexander Turenko.