From: Tomi Ollila <tomi.ollila@iki.fi>
To: david@tethera.net, notmuch@notmuchmail.org
Subject: Re: [PATCH] test: replace $PWD with YYY in emacs & emacs-show tests
In-Reply-To: <1385166355-12924-1-git-send-email-david@tethera.net>
References: <1385154389-3243-1-git-send-email-tomi.ollila@iki.fi>
	<1385166355-12924-1-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.16+175~g19e97d6 (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Sat, 23 Nov 2013 11:55:59 +0200
Message-ID: <m2vbzjqw5s.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Sat, Nov 23 2013, david@tethera.net wrote:

> From: David Bremner <david@tethera.net>
>
> When executed command line is written to *Notmuch errors* buffer,
> shell-quote-argument will backslash-escape any char that is not in
> "POSIX filename characters" (i.e. matching "[^-0-9a-zA-Z_./\n]").
>
> Currently in two emacs tests shell has expanded $PWD as part of
> emacs variable, which will later be fed to #'shell-quote-argument
> and finally written to ERROR file. If $PWD contained non-POSIX
> filename characters, data in ERROR file will not match $PWD when
> later comparing in shell. Therefore, in these two particular cases
> the escaped $PWD is replaced with YYY in ERROR file and expected
> content is adjusted accordingly.
> ---
>
> The commit message is shamelessly boosted from Tomi's patch.  To be
> honest I didn't notice that Tomi had already sent a patch when I
> started on this, and then I was too stubborn to stop. I'm not sure
> which is better. It depends whether you think the cosmetic stuff is an
> improvement, or just gratuitous. For what it's worth (not that much as
> Tomi points out) this should work ok with spaces in the path. Better
> to light a candle than to curse the darkness, etc...

yesyes ;D

This patch has 2 (additional) improvements: 

1) it removes the unportable sed -i (GNU feature) 
2) informs developer from which files the comparison contents are coming from

There is also one problem: the notmuch_emacs_error_sanitize() expects
that the notmuch-command in question is always .../notmuch_fail. This 
doesn't look good in a (generic) function.

One option would be to change the regexp in notmuch_emacs_error_sanitize()
to something like:
>   -e 's|^\(command: \)[^ ]*/|\1YYY/|'

i.e. just touch the ^command: line, remove all nonspace(*) chars until last
slash (/) before space.

(*) we're back with this 'space' thing, but I dont's see any alternative.
Using set of allowed chars is impossible as users may have e.g. utf-8
chars in their paths... 


>
>  test/emacs       | 15 ++++++++-------
>  test/emacs-show  | 10 +++++-----
>  test/test-lib.sh |  9 +++++++++
>  3 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/test/emacs b/test/emacs
> index 3b3b14d..dd544f5e 100755
> --- a/test/emacs
> +++ b/test/emacs
> @@ -881,15 +881,16 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
>  	       (with-current-buffer \"*Notmuch errors*\"
>  		  (test-output \"ERROR\"))
>  	       (test-output))"
> -sed -i -e 's/^\[.*\]$/[XXX]/' ERROR
> -test_expect_equal "$(cat OUTPUT; echo ---; cat MESSAGES; echo ---; cat ERROR)" "\
> +
> +test_expect_equal "$(notmuch_emacs_error_sanitize OUTPUT MESSAGES ERROR)" "\
> +=== OUTPUT ===
>  End of search results.
> ----
> -$PWD/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)
> ----
> +=== MESSAGES ===
> +YYY/notmuch_fail exited with status 1 (see *Notmuch errors* for more details)
> +=== ERROR ===
>  [XXX]
> -$PWD/notmuch_fail exited with status 1
> -command: $PWD/notmuch_fail search --format\=sexp --format-version\=2 --sort\=newest-first tag\:inbox
> +YYY/notmuch_fail exited with status 1
> +command: YYY/notmuch_fail search --format\=sexp --format-version\=2 --sort\=newest-first tag\:inbox
>  exit status: 1"
>  
>  test_begin_subtest "Search handles subprocess warnings"
> diff --git a/test/emacs-show b/test/emacs-show
> index fb23db4..21f1c68 100755
> --- a/test/emacs-show
> +++ b/test/emacs-show
> @@ -181,14 +181,14 @@ test_emacs "(let ((notmuch-command \"$PWD/notmuch_fail\"))
>  	       (with-current-buffer \"*Notmuch errors*\"
>  		  (test-output \"ERROR\"))
>  	       (test-output))"
> -sed -i -e 's/^\[.*\]$/[XXX]/' ERROR
> -test_expect_equal "$(cat OUTPUT; echo ---; cat MESSAGES; echo ---; cat ERROR)" "\
> ----
> +test_expect_equal "$(notmuch_emacs_error_sanitize OUTPUT MESSAGES ERROR)" "\
> +=== OUTPUT ===
> +=== MESSAGES ===
>  This is an error (see *Notmuch errors* for more details)
> ----
> +=== ERROR ===
>  [XXX]
>  This is an error
> -command: $PWD/notmuch_fail show --format\\=sexp --format-version\\=1 --exclude\\=false \\' \\* \\'
> +command: YYY/notmuch_fail show --format\\=sexp --format-version\\=1 --exclude\\=false \\' \\* \\'
>  exit status: 1
>  stderr:
>  This is an error
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 2aa4dfc..611aee3 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -628,6 +628,15 @@ notmuch_json_show_sanitize ()
>  	-e 's|"filename": "/[^"]*",|"filename": "YYYYY",|g'
>  }
>  
> +notmuch_emacs_error_sanitize ()
> +{
> +    for file in "$@"; do
> +	echo "=== $file ==="
> +	cat "$file"
> +    done | sed  \
> +	-e 's/^\[.*\]$/[XXX]/' \
> +	-e 's|^\(command: \)\{0,1\}/.*/notmuch_fail|\1YYY/notmuch_fail|'
> +}
>  # End of notmuch helper functions
>  
>  # Use test_set_prereq to tell that a particular prerequisite is available.
> -- 
> 1.8.4.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch