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@xxxxxx>
>
> 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
