From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id D17B26DE00F5 for ; Tue, 3 Jan 2017 09:22:05 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.516 X-Spam-Level: X-Spam-Status: No, score=0.516 tagged_above=-999 required=5 tests=[AWL=-0.136, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id mQrg4Dm1LETb for ; Tue, 3 Jan 2017 09:22:05 -0800 (PST) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id 6FE246DE00AC for ; Tue, 3 Jan 2017 09:22:03 -0800 (PST) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id F2F361001A3; Tue, 3 Jan 2017 19:21:59 +0200 (EET) From: Tomi Ollila To: mp39590@gmail.com, notmuch@notmuchmail.org Subject: Re: [PATCH] tests: add compatibility layer In-Reply-To: References: <87lguwfjhw.fsf@rocinante.cs.unb.ca> <20170102135721.93882-1-mp39590@gmail.com> User-Agent: Notmuch/0.23.3+85~g2b85e66 (https://notmuchmail.org) Emacs/24.5.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Jan 2017 17:22:05 -0000 On Tue, Jan 03 2017, Tomi Ollila wrote: > On Mon, Jan 02 2017, mp39590@gmail.com wrote: > >> From: Mikhail >> >> 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... What I mean that the IMO the tone should more like suggestive than impeartive -- it looks more like users have a choice... > > ... > >> + >> 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} > Sneaky fix ;) > 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. actually, the second line, to be consistent: + . ./test-lib-$PLATFORM.sh || exit 1 > > Tomi > >> if test -n "$valgrind" >> then >> make_symlink () { >> --