Re: [hackers][sbase][PATCH] testing: add first shell-based tests

From: Michael Forney <mforney_AT_mforney.org>
Date: Mon, 24 Sep 2018 12:41:39 -0700

Hi Silvan,

On 2018-09-09, Silvan Jegen <s.jegen_AT_gmail.com> wrote:
> We add some shell helper functions to test the expected output of sbase
> tools. In addition to the helper functions themselves we add some tests
> for 'dirname'.
> ---
> Hi
>
> I fixed some of the issues pointed out by Mattias and made the tests
> runnable from the Makefile.
>
> Let me know if there is a chance that we can get some shell-based testing
> like this into sbase.

I do like the idea of a test suite for sbase. It is too easy to
accidentally break something when fixing another bug.

Shell-based testing seems fine to me. However, I wonder if it might be
better to make this a separate repository (similar to musl and
libc-test). That way, you could test any implementation of the POSIX
utilities.

>
> Makefile | 6 +++++-
> tests/dirname.test | 20 ++++++++++++++++++++
> tests/runalltests | 5 +++++
> tests/test-common.sh | 37 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 67 insertions(+), 1 deletion(-)
> create mode 100755 tests/dirname.test
> create mode 100755 tests/runalltests
> create mode 100644 tests/test-common.sh
>
> diff --git a/Makefile b/Makefile
> index 0e421e7..9ddd873 100644
> --- a/Makefile
> +++ b/Makefile
> _AT_@ -274,4 +274,8 @@ clean:
> rm -f $(BIN) $(OBJ) $(LIB) sbase-box sbase-$(VERSION).tar.gz
> rm -f getconf.h
>
> -.PHONY: all install uninstall dist sbase-box sbase-box-install
> sbase-box-uninstall clean
> +testing:

If this were to go into the sbase repository, the `testing` target
should depend on the utilities being tested.

> + _AT_cd tests/; \
> + ./runalltests
> +
> +.PHONY: all install uninstall dist sbase-box sbase-box-install
> sbase-box-uninstall clean testing
> diff --git a/tests/dirname.test b/tests/dirname.test
> new file mode 100755
> index 0000000..3bc45ca
> --- /dev/null
> +++ b/tests/dirname.test
> _AT_@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +. ./test-common.sh
> +
> +# "test name" "command to execute" "expected output"
> +check_stdout "dirname-noarg" "../dirname" "" && \
> +check_stderr "dirname-noarg-stderr" "../dirname" "usage: ../dirname path\n"
> && \
> +check_stdout "dirname-non-existing" "../dirname a b c" "" && \
> +check_stdout "dirname-slash" "../dirname /" "/\n" && \
> +check_stdout "dirname-dashes-slash" "../dirname -- /" "/\n" && \
> +check_stdout "dirname-dashes-slash-a" "../dirname -- /a" "/\n" && \
> +check_stdout "dirname-doublequotes" "../dirname \"\"" ".\n" && \
> +check_stdout "dirname-slashes" "../dirname ///" "/\n" && \
> +check_stdout "dirname-a/b" "../dirname a/b" "a\n" && \
> +check_stdout "dirname-a/b/" "../dirname a/b/" "a\n" && \
> +check_stdout "dirname-a/b//" "../dirname a/b//" "a\n" && \
> +check_stdout "dirname-a" "../dirname a" ".\n" && \
> +check_stdout "dirname-a/" "../dirname a/" ".\n" && \
> +check_stdout "dirname-/a/b/c" "../dirname /a/b/c" "/a/b\n" && \
> +check_stdout "dirname-//a/b/c" "../dirname //a/b/c" "//a/b\n"
> diff --git a/tests/runalltests b/tests/runalltests
> new file mode 100755
> index 0000000..4cf7933
> --- /dev/null
> +++ b/tests/runalltests
> _AT_@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +for testfile in $(find -name '*.test' -type f); do
> + $testfile
> +done

I believe this will ignore any failures except in the last test.

Why not `for testfile in *.test`? Do you anticipate multiple
subdirectories of tests? If so, something like `find -name '*.test'
-type f -exec {} ';'` would be better.

> diff --git a/tests/test-common.sh b/tests/test-common.sh
> new file mode 100644
> index 0000000..e12fc78
> --- /dev/null
> +++ b/tests/test-common.sh
> _AT_@ -0,0 +1,37 @@
> +check_output() {
> + testname=$1
> + cmdtorun=$2
> + expectedOutput=$3
> + usestdout=$4
> + expOutFile=$(mktemp)
> + actualOutFile=$(mktemp)

I don't really like the mix of camel case, all lowercase, and underscore.

> + ret=0
> +
> + printf "$expectedOutput" > $expOutFile
> + if [ $usestdout -eq 1 ]; then
> + eval $cmdtorun > $actualOutFile 2> /dev/null
> + else
> + eval $cmdtorun 2> $actualOutFile 1> /dev/null
> + fi
> +
> + cmp $expOutFile $actualOutFile 2>&1 > /dev/null
> + if [ $? -eq 1 ]; then

You should probably also consider > 1, in which case there was an
error with cmp.

> + printf "$testname:\n"
> + printf "\tWanted:\n"
> + cat $expOutFile
> + printf "\n\tGot:\n"
> + cat $actualOutFile
> + ret=1
> + fi
> +
> + rm $expOutFile $actualOutFile
> + return $ret
> +}
> +
> +check_stdout() {
> + check_output "$1" "$2" "$3" 1
> +}
> +
> +check_stderr() {
> + check_output "$1" "$2" "$3" 0
> +}
> --
> 2.18.0
Received on Mon Sep 24 2018 - 21:41:39 CEST