Re: [GSoC][PATCH] t/: migrate helper/test-example-decorate to the unit testing framework

From: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Date: Jun 3, 2024, 17:51 UTC
Message-ID: <uplnglu2texnwzwf4fnu6kkbpg46nfxwuhum6pzwlgqwsqksg4@xgxy4mayyrcp>
In-Reply-To: <xmqqjzjbqoqc.fsf@gitster.g>

On Thu, 30 May 2024, Junio C Hamano <gitster@pobox.com> wrote:

Show 22 quoted lines

> Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:
> 
> > The latter provides much more context (we almost don't have to open
> > t-example-decorate.c file itself in some cases to know what failed)
> > than the former. Now, of course we can add more test_msg()s to the
> > former to improve, but I feel that this approach of splitting them
> > provides and improves the information provided on stdout _without_
> > adding any of my own test_msg()s. And I think that this is a good
> > middleground between cluttering the stdout vs providing very little
> > context while also remaining a faithful copy of the original.
> 
> If so, why stop at having four, each of which has more than one step
> that could further be split?  What's the downside?
> 
>     Note: Here in this review, I am not necessarily suggesting the
>     tests in this patch to be further split into greater number of
>     smaller helper functions.  I am primarily interested in finding
>     out what the unit test framework can further do to help unit
>     tests written using it (i.e., like this patch).  If using
>     finer-grained tests gives you better diagnosis, but if it is too
>     cumbersome to separate the tests out further, is it because the
>     framework is inadequate in some way?  How can we improve it?

It's not that the framework is inadequate in its current state
(for this test). As Christian said, in the original
test-example-decorate.c, the tests were divided into four sections 
by a space and comments like:
	/*
	 * Add 2 objects, one with a non-NULL decoration and one with a NULL
	 * decoration.
	 */

So, I also made those four sections in the form of those functions and the comments became the test description. I definitely don't see any downside in further dividing where it makes sense. For example, the first test can be split into two, one which adds an object with non-NULL decoration and one with NULL (I think you mentioned this). And the third test can split to test lookup for a known object vs an unknown object. Besides these I don't see where we can split.

Thanks.