[PATCH] test: Make the emacsclient binary user-configurable

[PATCH] test: Make the emacsclient binary user-configurable

[Austin Clements]

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.
---
 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

Re: [PATCH] test: Make the emacsclient binary user-configurable

[Tomi Ollila]

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

Re: [PATCH] test: Make the emacsclient binary user-configurable

[David Bremner]

Austin Clements <amdragon@MIT.EDU> writes:

> And require that if TEST_EMACS is specified, so is TEST_EMACSCLIENT.
>

pushed.

d