Re: [hackers] [sbase][PATCH] Test framework created. See tests/README for details. Individual test cases can be run from tests dir using command line. Entire test suite can be run using: make && make tests

From: Xan Phung <xan.phung_AT_gmail.com>
Date: Thu, 6 Nov 2025 21:47:56 +1100

Hi Roberto,

Thanks for coming up with the points about progressing the test framework.

I agree with many of your points - I've addressed individually them inside
the quotations underneath.

However, I do want to advocate upfront for the following:

1. File Naming Conventions for Expected & Output files:

I notice 0001-echo.sh generates an expected file & an output file using $$
(PID) - so essentially a random number with no way to know which tests
produced which output.

I recommend instead scripts generate files using the names "$0.expected"
and "$0.out" (for test output) - ie: prefix these files with test script's
name, as "$0.out" becomes "0001-echo.sh.out", etc. This allows a whole
batch of tests to be run, then the output of each test can later be
examined by the user at their leisure & diff'ed manually by the user,
without tracing which PID's belong to which tests scripts.

Also I'd advocate test scripts don't do cleanup of these files, as it is
useful to keep them for debugging or for system verification logs. There
will be an automatic upper bound to these files as any new test execution
will overwrite the old files with new files (if my naming convention is
used). Without needing to do this cleanup, this can also avoid the need
for the cleanup() function and trapping signals entirely.

I notice your Makefile already can do "make clean" within the tests
directory to force a cleanup when required (so not doing cleanup when
running tests is consistent with the $CC leaving behind the object .o files
- and also like my naming convention above, $CC uses the executable code
file name as the base name for the .o file, not a PID name)

2. Shell variables can be good (optional) programming practice (like how
#define MAX_STRING 100 might be used in self contained C source files):

a. The shell variables aren't "shared" between scripts & don't change the
script's function. They are defined at the top of the file, and then only
used within the file itself afterwards. No values are shared between
scripts, ie: TESTEXE is "../echo" in one script but will be "../xargs" in a
different script, and the shell vars are thus defined & used in a
self-contained way.

b. Shell variables improve cosmetic readability. Readers of the script
will be able to immediately see "TESTEXE" and know where the execution of
the test binary is done. Note that "echo" could be used elsewhere in the
script for other purposes (eg. printing to a log, or report absence of
needed data files etc) and it is visually confusing if multiple such echo's
are present in the echo test script.

c. Shell variables improve scalability and maintainability of tests. By
defining a TESTARG variable at the top of a test script, you can have very
large or very long input strings to a command, which would otherwise make
the execution line quite cluttered. (This is optional, developers can
avoid TESTARG and instead hard-code the arguments to the test command if
they see no benefit for their individual script)

d. I'd advocate that having TESTEXE variable improves self-containment of
the tests directory, (which you've stated is a major goal).

The 0001-echo.sh script hardcodes "../echo" and so there is a hard
dependency on the parent directory, that will be replicated many times over
across all test scripts. By defining "TESTEXE="${TESTEXE:-../echo}" at the
top of the 0001-echo test script, although there is a (self contained
default) to "../echo", the user can actually override this and supply a
different executable path for echo. This allows for example the user to
run tests on their echo in (say) "/usr/local/bin/echo" or even in
"~/.local/bin/echo" and it also allows developers to compare outputs of
different echo implementations, eg. from BSD or GNU etc, which helps assist
debugging. It also helps in generating an expected output file, and for
ensuring there is no regression between different versions (ie: old vs new)
of sbase itself.

This setting of TESTEXE outside the test script is entirely optional,
because the default "../echo" is used if the user hasn't set TESTEXE to any
value.

I've put my remaining comments about other specific points below...

On Thu, 6 Nov 2025 at 04:49, Roberto E. Vargas Caballero <k0ga_AT_shike2.net>
wrote:

> Hi,
>
> On Wed, Jun 04, 2025 at 11:30:32AM +1000, Xan Phung wrote:
> > ---
> > .gitignore | 2 +
> > Makefile | 11 ++--
> > tests/README | 109 +++++++++++++++++++++++++++++++++++++++
> > tests/echo-n.sh | 10 ++++
> > tests/echo-n.sh.expected | 1 +
> > 5 files changed, 130 insertions(+), 3 deletions(-)
> > create mode 100644 tests/README
> > create mode 100755 tests/echo-n.sh
> > create mode 100644 tests/echo-n.sh.expected
>
> Good point, this is something that we discussed many times but we
> didn't do before just because we were very lazy. I think it is a
> good moment to begin with the tests.
>
> Saying that, I think the test no-framework should be much more
> simple, and encapsulated in the test directory.


Agreed


Every shell script
> test should be independent,


Agreed


not sharing magic environment variables
>

(Discussed above) shell variables can just be an optional cosmetic
readability programming practice, and won't/don't change the functioning or
interdependence of scripts


and being responsible of how to test, instead of relaying in having
> an expected file.


Agreed test scripts be responsible for how to test. (Within the test
script, they can use cmp or diff against an expected file to do the test.)

On the issue of scripts self-generating expected file (eg. like the
0001-echo.sh script) ... isn't it a downside of doing this the potential
maintenance burden?

The expected output might change with bug fixes, new feature sets,
different version numbers, help/usage printouts, changed input data etc.
With a separate expected file, only the expected file will change - but
with a merged expected+test script, then there is always the need to check
this won't impact the rest of the shell script code, and it will be less
clear on patch or git what is just a data change and what is a script code
change.


Having the expected file works in some cases, but
> in our case where in many cases we have to deal with signals it
> does not behave very well.


>
Also, having just a set of shell scripts and using their exit status
> remove the need of any documentation. Just try to get a successful
> shell script execution.
>

Agreed (my test scripts use exit status 1 to mean test failure and 127 to
mean test can't be run due to other reasons, eg. missing executable,
missing data, no systems permissions etc).



>
> My proposal is this:
>
> 8< -----
>
> From a75639f1799b03f164592c6f0d632a96f7000da7 Mon Sep 17 00:00:00 2001
> From: "Roberto E. Vargas Caballero" <k0ga_AT_shike2.net>
> Date: Wed, 5 Nov 2025 18:41:15 +0100
> Subject: [PATCH] tests: Add initial support for tests
>
> ---
> .gitignore | 2 ++
> Makefile | 9 +++++++--
> tests/0001-echo.sh | 23 +++++++++++++++++++++++
> tests/Makefile | 6 ++++++
> tests/runtests.sh | 22 ++++++++++++++++++++++
> 5 files changed, 60 insertions(+), 2 deletions(-)
> create mode 100755 tests/0001-echo.sh
> create mode 100644 tests/Makefile
> create mode 100755 tests/runtests.sh
>
> diff --git a/.gitignore b/.gitignore
> index f338199..0060cdd 100644
> --- a/.gitignore
> +++ b/.gitignore
> _AT_@ -104,3 +104,5 @@
> /xargs
> /xinstall
> /yes
> +/tests/test.log
> +/tests/test.lst
> diff --git a/Makefile b/Makefile
> index 6596063..c478a2f 100644
> --- a/Makefile
> +++ b/Makefile
> _AT_@ -202,7 +202,7 @@ MAKEOBJ =\
> OBJ = $(LIBUTFOBJ) $(LIBUTILOBJ) $(MAKEOBJ)
>
> all: scripts/make
> - $(SMAKE) $(BIN)
> + +_AT_$(SMAKE) $(BIN)
>
> scripts/make:
> $(CC) -o $_AT_ make/*.c
> _AT_@ -252,9 +252,13 @@ sbase-box-uninstall: sbase-box proto
> $(DESTDIR)$(PREFIX)/bin/sbase-box -d $(DESTDIR)$(PREFIX)/bin/
> scripts/uninstall proto
>
> +tests: all
> + _AT_cd $@ && $(MAKE)
> +
> dist: clean
> mkdir -p sbase
> - cp -R LICENSE Makefile README TODO config.mk *.c *.1 *.h libutf
> libutil make scripts sbase
> + cp LICENSE Makefile README TODO config.mk *.c *.1 *.h sbase
> + cp -R libutf libutil make scripts tests sbase
> mv sbase sbase-$(VERSION)
> tar -cf sbase-$(VERSION).tar sbase-$(VERSION)
> gzip sbase-$(VERSION).tar
> _AT_@ -265,6 +269,7 @@ sbase-box: $(BIN)
> $(CC) $(CFLAGS) $(CPPFLAGS) $(LDFLAGS) -o $_AT_ build/*.c $(LIB)
>
> clean:
> + _AT_cd tests && $(MAKE) clean
> rm -f $(BIN) $(OBJ) $(LIB) sbase-box sbase-$(VERSION).tar.gz
> rm -f scripts/make
> rm -f getconf.h
> diff --git a/tests/0001-echo.sh b/tests/0001-echo.sh
> new file mode 100755
> index 0000000..7eb961f
> --- /dev/null
> +++ b/tests/0001-echo.sh
> _AT_@ -0,0 +1,23 @@
> +#!/bin/sh
> +
> +set -e
> +
> +tmp1=tmp1.$$
> +tmp2=tmp2.$$
> +
>

I recommend instead the following

tmp1="$0.out"
tmp2="$0.expected"



> +cleanup()
> +{
> + st=$?
> + rm -f $tmp1 $tmp2
> + exit $st
> +}
>
>
Would "$test_status" be a more easily understood shell variable name than
"$st"?
(In my script, the equivalent was "$fail" but in hindsight "exit
$test_status" reads better than "exit $fail")

(Also see previous comments about not doing the "rm -f <outputfile>
<expectedfile?" cleanup at all, and keeping these files as logs for user to
inspect). If we don't need the rm, you can drop this entire cleanup()
function and just exit directly from the script.


+trap cleanup EXIT HUP INT TERM
> +
> +cat <<'EOF' | tr -d '\n' > $tmp1
> +--hello-- --world--!
> +EOF
>

I notice you need to the tr -d '\n' to generate your expected file
correctly. This shows it can be quite complicated for scripts to self
generate an expected file, and ensure all the whitespace, control chars,
tabs vs spaces, escape chars etc are all done correctly - whereas this
problem can be avoided where a separate expected file is supplied instead
of script-generated.



> +../echo -n --hello-- --world--! > $tmp2
> +
> +diff -u $tmp1 $tmp2
> diff --git a/tests/Makefile b/tests/Makefile
> new file mode 100644
> index 0000000..1311918
> --- /dev/null
> +++ b/tests/Makefile
> _AT_@ -0,0 +1,6 @@
> +all:
> + _AT_./runtests.sh
> +
> +clean:
> + rm -f test.log
> + rm -f tmp*
> diff --git a/tests/runtests.sh b/tests/runtests.sh
> new file mode 100755
> index 0000000..9433ae2
> --- /dev/null
> +++ b/tests/runtests.sh
> _AT_@ -0,0 +1,22 @@
> +#!/bin/sh
> +
> +export TZ=UTC
> +
> +cleanup()
> +{
> + st=$?
> + rm -f test.res
> + exit $st
> +}
> +
> +trap cleanup EXIT HUP INT TERM
> +
> +for i in *-*.sh
> +do
> + printf "Test: %s\n\n" $i >> test.log
> + (./$i >> test.log 2>&1 && printf '[PASS]\t' || printf '[FAIL]\t'
> + echo "$i") | tee -a test.log
> +done |
> +tee test.res
> +
> +! grep FAIL test.res >/dev/null
> --
> 2.46.1
>
>
Received on Thu Nov 06 2025 - 11:47:56 CET