From: Tomi Ollila <tomi.ollila@iki.fi>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH] test: Make the emacsclient binary user-configurable
In-Reply-To: <1354073099-14582-1-git-send-email-amdragon@mit.edu>
References: <1354073099-14582-1-git-send-email-amdragon@mit.edu>
User-Agent: Notmuch/0.14+116~g29fcdb5 (http://notmuchmail.org) Emacs/24.2.1
	(x86_64-unknown-linux-gnu)
Date: Wed, 28 Nov 2012 10:05:12 +0200
Message-ID: <m2zk22w3t3.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Wed, Nov 28 2012, Austin Clements <amdragon@MIT.EDU> wrote:

> And require that if TEST_EMACS is specified, so is TEST_EMACSCLIENT.
>
> Previously, the test framework always used "emacsclient", even if the
> Emacs in use was overridden by TEST_EMACS.  This causes problems if
> both Emacs 23 and Emacs 24 are installed, the Emacs 23 emacsclient is
> the system default, but TEST_EMACS is set to emacs24.  Specifically,
> with an Emacs 24 server and an Emacs 23 client, emacs tests that run
> very quickly may produce no output from emacsclient, causing the test
> to fail.
>
> The Emacs server uses a very simple line-oriented protocol in which
> the client sends a request to evaluate an expression and the server
> sends a request to print the result of evaluation.  Prior to Emacs bzr
> commit 107565 on March 11th, 2012 (released in Emacs 24.1), if
> multiple commands were sent to the emacsclient between when it sent
> the evaluation command and when it entered its receive loop, it would
> only process the first response command, ignoring the rest of the
> received buffer.  This wasn't a problem with the Emacs 23 server
> because it sent only the command to print the evaluation result.
> However, the Emacs 24 server first sends an unprompted command
> specifying the PID of the Emacs server, then processes the evaluation
> request, then sends the command to print the result.  If the
> evaluation is fast enough, it can send both of these commands before
> emacsclient enters the receive loop.  Hence, if an Emacs 24 server is
> used with an Emacs 23 emacsclient, it may miss the response printing
> command, ultimately causing intermittent notmuch test failures.
> ---

Looks good to me (especially commit message)

I guess buildbot configuration needs to be fixed to set
TEST_EMACSCLIENT too (now that it is possible as it probably
sets TEST_EMACS already...)

Tomi


>  test/README      |   10 +++++-----
>  test/test-lib.sh |   12 +++++++++---
>  2 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/test/README b/test/README
> index 6dc3034..81c232d 100644
> --- a/test/README
> +++ b/test/README
> @@ -69,12 +69,12 @@ can be specified as follows:
>  
>  	make test OPTIONS="--verbose"
>  
> -You can choose an emacs binary to run the tests in one of the
> -following ways.
> +You can choose an emacs binary (and corresponding emacsclient) to run
> +the tests in one of the following ways.
>  
> -	TEST_EMACS=my-special-emacs make test
> -	TEST_EMACS=my-special-emacs ./emacs
> -	make test TEST_EMACS=my-special-emacs
> +	TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient make test
> +	TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient ./emacs
> +	make test TEST_EMACS=my-special-emacs TEST_EMACSCLIENT=my-emacsclient
>  
>  Skipping Tests
>  --------------
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 467b83c..e092231 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -49,7 +49,13 @@ TZ=UTC
>  TERM=dumb
>  export LANG LC_ALL PAGER TERM TZ
>  GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
> +if [[ ( -n "$TEST_EMACS" && -z "$TEST_EMACSCLIENT" ) || \
> +      ( -z "$TEST_EMACS" && -n "$TEST_EMACSCLIENT" ) ]]; then
> +    echo "error: must specify both or neither of TEST_EMACS and TEST_EMACSCLIENT" >&2
> +    exit 1
> +fi
>  TEST_EMACS=${TEST_EMACS:-${EMACS:-emacs}}
> +TEST_EMACSCLIENT=${TEST_EMACSCLIENT:-emacsclient}
>  
>  # Protect ourselves from common misconfiguration to export
>  # CDPATH into the environment
> @@ -969,7 +975,7 @@ test_emacs () {
>  	missing_dependencies=
>  	test_require_external_prereq dtach || missing_dependencies=1
>  	test_require_external_prereq emacs || missing_dependencies=1
> -	test_require_external_prereq emacsclient || missing_dependencies=1
> +	test_require_external_prereq ${TEST_EMACSCLIENT} || missing_dependencies=1
>  	test -z "$missing_dependencies" || return
>  
>  	if [ -z "$EMACS_SERVER" ]; then
> @@ -1005,7 +1011,7 @@ test_emacs () {
>  	rm -f OUTPUT
>  	touch OUTPUT
>  
> -	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
> +	${TEST_EMACSCLIENT} --socket-name="$EMACS_SERVER" --eval "(progn $@)"
>  }
>  
>  test_python() {
> @@ -1158,7 +1164,7 @@ rm -f y
>  # declare prerequisites for external binaries used in tests
>  test_declare_external_prereq dtach
>  test_declare_external_prereq emacs
> -test_declare_external_prereq emacsclient
> +test_declare_external_prereq ${TEST_EMACSCLIENT}
>  test_declare_external_prereq gdb
>  test_declare_external_prereq gpg
>  test_declare_external_prereq python
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch