Re: [PATCH liburing 6/7] test: add make targets for each test

[Dylan Yudaken <dylany@fb.com>]

On Fri, 2022-04-22 at 20:40 +0700, Ammar Faizi wrote:
> On 4/22/22 6:48 PM, Dylan Yudaken wrote:
> > Add a make target runtests-parallel which can run tests in
> > parallel.
> > This is very useful to quickly run all the tests locally with
> >    $ make -j runtests-parallel
> > 
> > Signed-off-by: Dylan Yudaken <dylany@fb.com>
> 
> Two comments below...
> 
> >   test/Makefile          | 10 +++++++++-
> >   test/runtests-quiet.sh | 10 ++++++++++
> >   2 files changed, 19 insertions(+), 1 deletion(-)
> >   create mode 100755 test/runtests-quiet.sh
> 
> I suggest to add the following to the main Makefile:
> ```
> runtests-parallel: all
>         +$(MAKE) -C test runtests-parallel
> ```
> 
> So we can do this directly:
> ```
>     make -j runtests-parallel;
> ```
> instead of doing this:
> ```
>     cd test;
>     make -j runtests-parallel;
> ```
> 
> > -.PHONY: all install clean runtests runtests-loop
> > +%.run_test: %.t
> > +       @./runtests-quiet.sh $<
> > +
> > +runtests-parallel: $(run_test_targets)
> > +       @echo "All tests passed"
> 
> Note that this parallel thing is doing:
> 
>     @./runtests-quiet.sh $THE_TEST_FILE
> 
> ^ That thing is not a problem. But the ./runtests-quiet.sh exit code
> is.
> > diff --git a/test/runtests-quiet.sh b/test/runtests-quiet.sh
> > new file mode 100755
> > index 0000000..ba9fe2b
> > --- /dev/null
> > +++ b/test/runtests-quiet.sh
> > @@ -0,0 +1,10 @@
> > +#!/usr/bin/env bash
> > +
> > +TESTS=("$@")
> > +RESULT_FILE=$(mktemp)
> > +./runtests.sh "${TESTS[@]}" 2>&1 > $RESULT_FILE
> > +RET="$?"
> > +if [ "${RET}" -ne 0 ]; then
> > +    cat $RESULT_FILE
> > +fi
> > +rm $RESULT_FILE
> 
> This script's exit code doesn't necessarily represent the exit code
> of
> the `./runtests.sh "${TESTS[@]}"`, so you have to add `exit $RET` at
> the
> end of the script. Otherwise, the Makefile will always print "All
> tests
> passed" even if we have tests failed.
> 

Both of these are good ideas - thanks! The second one especially is a
great spot. Will respin a v2 with them

Dylan