Re: [PATCH v7 2/3] unit tests: add TAP unit test framework

[Josh Steadmon <steadmon@google.com>]

On 2023.09.24 14:57, phillip.wood123@gmail.com wrote:
> On 22/09/2023 21:05, Junio C Hamano wrote:
> > Any thought on the "polarity" of the return values from the
> > assertion?  I still find it confusing and hard to follow.
> 
> When I was writing this I was torn between whether to follow our usual
> convention of returning zero for success and minus one for failure or to
> return one for success and zero for failure. In the end I decided to go with
> the former but I tend to agree with you that the latter would be easier to
> understand.

Agreed. V8 will switch to 0 for failure and 1 for success for the TEST,
TEST_TODO, and check macros.


> > > > +test_expect_success 'TAP output from unit tests' '
> > > > [...]
> > > > +	ok 19 - test with no checks returns -1
> > > > +	1..19
> > > > +	EOF
> > > 
> > > Presumably t-basic will serve as a catalog of check_* functions and
> > > the test binary, together with this test piece, will keep growing as
> > > we gain features in the unit tests infrastructure.  I wonder how
> > > maintainable the above is, though.  When we acquire new test, we
> > > would need to renumber.  What if multiple developers add new
> > > features to the catalog at the same time?
> 
> I think we could just add new tests to the end so we'd only need to change
> the "1..19" line. That will become a source of merge conflicts if multiple
> developers add new features at the same time though. Having several unit
> test programs called from separate tests in t0080 might help with that.

My hope is that test-lib.c will not have to grow too extensively after
this series; that said, it's already been a pain to have to adjust the
t0080 expected text several times just during development of this
series. I'll look into splitting this into several "meta-tests", but I'm
not sure I'll get to it for V8 yet.


> > > > diff --git a/t/unit-tests/.gitignore b/t/unit-tests/.gitignore
> > > > new file mode 100644
> > > > index 0000000000..e292d58348
> > > > --- /dev/null
> > > > +++ b/t/unit-tests/.gitignore
> > > > @@ -0,0 +1,2 @@
> > > > +/t-basic
> > > > +/t-strbuf
> > > 
> > > Also, can we come up with some naming convention so that we do not
> > > have to keep adding to this file every time we add a new test
> > > script?
> 
> Perhaps we should put the unit test binaries in a separate directory so we
> can just add that directory to .gitignore.

Sounds good to me.