From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Edmondson <dme@dme.org>, <notmuch@notmuchmail.org>
Subject: Re: [PATCH 2/3] test: Add `test_emacs_expect_t'.
In-Reply-To: <cunzkdk57yj.fsf@hotblack-desiato.hh.sledj.net>
References: <87pqei327k.fsf@gmail.com>
	<1326809224-22673-1-git-send-email-dme@dme.org>
	<1326809224-22673-2-git-send-email-dme@dme.org>
	<yf6zkdlawlc.fsf@taco2.nixu.fi>
	<cunzkdk57yj.fsf@hotblack-desiato.hh.sledj.net>
User-Agent: Notmuch/0.11+71~gefa5d6c (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Thu, 19 Jan 2012 12:32:21 +0200
Message-ID: <yf662g8asp6.fsf@taco2.nixu.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Thu, 19 Jan 2012 09:59:16 +0000, David Edmondson <dme@dme.org> wrote:
> On Wed, 18 Jan 2012 16:55:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> > > +	# Report success/failure.
> > > +	if ! test_skip "$test_subtest_name"
> > > +	then
> > > +		if [ "$result" == t ]
> > 
> > 		if [ "$result" = t ]
> > 
> > to be compatible with POSIX and consistent with rest code.
> 
> I'm happy to change this.

As your older patch has been marked obsolete and this really is the
only place where there is == in comparison code please do that change.

> > > +			test_failure_ "$test_subtest_name" "$(eval printf ${result})"
> > 
> > This added 'eval' made me investigate further... running 
> > 
> > emacsclient --eval '(print (concat "a" "b" "\t" "c" "\n" "z"))'
> > 
> > outputs "ab	c\nz" (tab between 'ab' and 'c', quotes (") around
> > the whole output and newlines as "\n" (even '\r' is converted)).
> > 
> > If emacs tests run via test_emacs_expect_t wrote their output 
> > to ${output} directly above code could be much cleaner in many
> > places. Environment variable could be used for the file name.
> 
> I'm reluctant to change this just to avoid calling eval.

Consider the following:

$ emacsclient --eval '(print "$(echo rm -rf /); echo `date +%Y`")'
"$(echo rm -rf /); echo `date +%Y`"

x='"$(echo rm -rf /); echo `date +%Y`"'

$echo $x
"$(echo rm -rf /); echo `date +%Y`"

$echo $(eval printf $x)
rm -rf /; echo 2012

x='"$(echo rm /); echo `date +%Y`"'

$(eval printf $x)
rm: cannot remove `/;': No such file or directory
rm: cannot remove `2012': No such file or directory

one just needs to make sure there is no $:s and `: in 
the output... hmm, nor ';' ...NOR '&':s ...

I am not absolutely sure this might actually happen
but surely I'm not sure it would not...

... The suggestion having environment variable would
now work, but what about

test_emacs "(setq test-output-file \"${output}\") $1"


Tomi

PS:

this needs to be changed:
	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
to
	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $*)"
then
	test_emacs   "(setq test-output-file \"${output}\")"   "$1"

would be safe