Re: [PATCH] tests: add compatibility layer

[Tomi Ollila <tomi.ollila@iki.fi>]

On Mon, Jan 02 2017, mp39590@gmail.com wrote:

> From: Mikhail <mp39590@gmail.com>
>
> Make test-lib-common.sh load test-lib-<$PLATFORM>.sh to create
> additional shim for platform specifics.
>
> Use test-lib-FREEBSD.sh to call GNU utilities instead of native ones.

Ok, now I've git a bit of time to read and respond; If I'd read earlier
and not had time to respone, I'd have thought this issue too much in
between... so let's quickly process this :D

> ---
>  configure                | 3 +++
>  test/README              | 6 ++++++
>  test/test-lib-FREEBSD.sh | 8 ++++++++
>  test/test-lib-common.sh  | 5 +++++
>  4 files changed, 22 insertions(+)
>  create mode 100644 test/test-lib-FREEBSD.sh
>
> diff --git a/configure b/configure
> index 72db26df..7ba9b9eb 100755
> --- a/configure
> +++ b/configure
> @@ -1203,6 +1203,9 @@ NOTMUCH_PYTHON=${python}
>  # Are the ruby development files (and ruby) available? If not skip
>  # building/testing ruby bindings.
>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
> +
> +# Platform we are run on
> +PLATFORM=${platform}

This change looks good.

>  EOF
>  
>  # Finally, after everything configured, inform the user how to continue.
> diff --git a/test/README b/test/README
> index 104a120e..8376616f 100644
> --- a/test/README
> +++ b/test/README
> @@ -33,6 +33,12 @@ chosen directory to your PATH before running the tests.
>  
>  e.g. env PATH=/opt/gnu/bin:$PATH make test
>  
> +For FreeBSD you will need to install coreutils, which provides GNU
> +versions of basic utils like 'date' or 'wc'. Also you will need to
> +install latest gdb from ports or packages and provide path to it in
> +NOTMUCH_GDB variable before executing the tests, native FreeBSD gdb will
> +not work.

I think this is a bit too strong statement; one should not *need* to
install such a bloaty beast as a GNU coreutils to use instead of nice
native utils system is shipped with; IMO this should mention that
installing coreutils (in a way that these g-prefixed commands appear
in PATH!) may improve the success of test run... or something. 

Also, IMHO, native FreeBSD gdb does not work is much nicer than stating
it will not work -- but that may just be my feeling of how it sounds...

...

> +
>  Running Tests
>  -------------
>  The easiest way to run tests is to say "make test", (or simply run the
> diff --git a/test/test-lib-FREEBSD.sh b/test/test-lib-FREEBSD.sh
> new file mode 100644
> index 00000000..b8039705
> --- /dev/null
> +++ b/test/test-lib-FREEBSD.sh
> @@ -0,0 +1,8 @@
> +# Use GNU Coreutils instead of a native BSD utils
> +
> +date () { gdate "$@"; }
> +base64 () { gbase64 "$@"; }
> +gdb () { $NOTMUCH_GDB "$@"; }
> +wc () { gwc "$@"; }
> +sed () { gsed "$@"; }
> +sha256sum () { gsha256sum "$@"; }

The above is currently problematic when one does not install GNU coreutils;
tests will fail more when e.g. `gwc` does not exist but `wc` does. When I
tested with this patch a few days ago, test failure count increased from
~70 to ~140. That was the quickly written mkwrp() for...

I use my freebsd KVM virtual machine for e.g. portability testing; things
that work on freebsd (in addition to linux) have more chance to work on
netbsd, openbsd, macos, solaris, openindiana. With the above applied
verbatim I would always need to remember to dump the "compatibility file"
before running tests...

To fix the above, one could use (possibly better written) mkwrap()
implementation, or do all of those by hand: e.g.

if command -v gdate >/dev/null; then date () { gdate "$@"; }; fi
if command -v gbase64 >/dev/null; then base64 () { gbase64 "$@"; }; fi
...

the gdb wrapper could be dumped completely and have the following in
test-lib-common.sh:

: ${NOTMUCH_GDB:-gdb}

and then replace all (the few) gdb in code with $NOTMUCH_GDB (did not use
quotes like "$NOTMUCH_GDB" so that one can hack args there.

but if the gdb function were there, then the following might work:

gdb () { command ${NOTMUCH_GDB:-gdb} "$@"; }


> diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
> index 03ef1d2d..1c8d7f6e 100644
> --- a/test/test-lib-common.sh
> +++ b/test/test-lib-common.sh
> @@ -66,6 +66,11 @@ export LD_LIBRARY_PATH
>  # configure output
>  . $notmuch_path/sh.config || exit 1
>  
> +# load OS specifics
> +if [ -e ./test-lib-$PLATFORM.sh ]; then
> +	. ./test-lib-$PLATFORM.sh
> +fi
> +

This change looks good.


Tomi

>  if test -n "$valgrind"
>  then
>  	make_symlink () {
> --