Hi Silvan,

On 2018-09-09, Silvan Jegen <s.je...@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
> @@ -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.

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

Reply via email to