[PATCH 1/3] test: update documentation for test_emacs in test/README

[PATCH 1/3] test: update documentation for test_emacs in test/README

[Dmitry Kurochkin]

Update test_emacs documentation in test/README according to the latest
changes in emacs tests.  Move the note regarding setting variables
from test/emacs to test/README.
---
 test/README |   10 +++++++---
 test/emacs  |    5 -----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/README b/test/README
index f9ac607..a245bf1 100644
--- a/test/README
+++ b/test/README
@@ -181,9 +181,13 @@ library for your script to use.
 
    This function executes the provided emacs lisp script within
    emacs. The script can be a sequence of emacs lisp expressions,
-   (that is, they will be evaluated within a progn form). The lisp
-   expressions can call `message' to generate output on stdout to be
-   examined by the calling test script.
+   (that is, they will be evaluated within a progn form). Emacs
+   stdout and stderr is not available, the common way to get output
+   is to save it to a file. There are some auxiliary functions
+   useful in emacs tests provided in test-lib.el. Do not use `setq'
+   for setting variables in Emacs tests because it affects other
+   tests that may run in the same Emacs instance.  Use `let' instead
+   so the scope of the changed variables is limited to a single test.
 
  test_done
 
diff --git a/test/emacs b/test/emacs
index 53f455a..f465e2b 100755
--- a/test/emacs
+++ b/test/emacs
@@ -1,10 +1,5 @@
 #!/usr/bin/env bash
 
-# Note: do not use `setq' for setting variables in Emacs tests because
-# it affects other tests that may run in the same Emacs instance.  Use
-# `let' instead so the scope of the changed variables is limited to a
-# single test.
-
 test_description="emacs interface"
 . test-lib.sh
 
-- 
1.7.5.4

[PATCH 2/3] test: improve known broken tests support

[Dmitry Kurochkin]

There is existing support for broken tests.  But it is not convenient
to use.  The primary issue is that we have to maintain a set of
test_expect_*_failure functions which are equivalent to the normal
test_expect_* counterparts except for what functions are called for
result reporting.  The patch adds test_subtest_known_broken function
which marks a subset as broken, making the normal test_expect_*
functions behave as test_expect_*_failure.  All test_expect_*_failure
functions are removed.  Test_known_broken_failure_ is changed to
format details the same way as test_failure_ does.

Another benefit of this change is that the diff when a broken test is
fixed would be small and nice.

Documentation is updated accordingly.
---
 test/README      |   17 ++++++++---------
 test/test-lib.sh |   53 +++++++++++++++--------------------------------------
 2 files changed, 23 insertions(+), 47 deletions(-)

diff --git a/test/README b/test/README
index a245bf1..f926b9f 100644
--- a/test/README
+++ b/test/README
@@ -132,20 +132,19 @@ library for your script to use.
    <script>.  If it yields success, test is considered
    successful.  <message> should state what it is testing.
 
- test_expect_failure <message> <script>
-
-   This is NOT the opposite of test_expect_success, but is used
-   to mark a test that demonstrates a known breakage.  Unlike
-   the usual test_expect_success tests, which say "ok" on
-   success and "FAIL" on failure, this will say "FIXED" on
-   success and "still broken" on failure.  Failures from these
-   tests won't cause -i (immediate) to stop.
-
  test_begin_subtest <message>
 
    Set the test description message for a subsequent test_expect_equal
    invocation (see below).
 
+ test_subtest_known_broken
+
+   Mark the current test as broken.  Such tests are expected to fail.
+   Unlike the normal tests, which say "PASS" on success and "FAIL" on
+   failure, these will say "FIXED" on success and "BROKEN" on failure.
+   Failures from these tests won't cause -i (immediate) to stop.  This
+   must be called before any test_expect_* function.
+
  test_expect_equal <output> <expected>
 
    This is an often-used convenience function built on top of
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 22e387e..0cd4170 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -424,6 +424,7 @@ test_begin_subtest ()
 	error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
     fi
     test_subtest_name="$1"
+    test_subtest_known_broken_=
     # Remember stdout and stderr file descriptors and redirect test
     # output to the previously prepared file descriptors 3 and 4 (see
     # below)
@@ -484,29 +485,6 @@ test_expect_equal_file ()
     fi
 }
 
-test_expect_equal_failure ()
-{
-	exec 1>&6 2>&7		# Restore stdout and stderr
-	inside_subtest=
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
-
-	output="$1"
-	expected="$2"
-	if ! test_skip "$@"
-	then
-		if [ "$output" = "$expected" ]; then
-			test_known_broken_ok_ "$test_subtest_name"
-		else
-			test_known_broken_failure_ "$test_subtest_name"
-			testname=$this_test.$test_count
-			echo "$expected" > $testname.expected
-			echo "$output" > $testname.output
-		fi
-    fi
-}
-
 NOTMUCH_NEW ()
 {
     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
@@ -568,12 +546,20 @@ test_have_prereq () {
 # the text_expect_* functions instead.
 
 test_ok_ () {
+	if [ "$test_subtest_known_broken_" = 1 ]; then
+		test_known_broken_ok_ "$@"
+		return
+	fi
 	test_success=$(($test_success + 1))
 	say_color pass "%-6s" "PASS"
 	echo " $@"
 }
 
 test_failure_ () {
+	if [ "$test_subtest_known_broken_" = 1 ]; then
+		test_known_broken_failure_ "$@"
+		return
+	fi
 	test_failure=$(($test_failure + 1))
 	say_color error "%-6s" "FAIL"
 	echo " $1"
@@ -592,7 +578,10 @@ test_known_broken_ok_ () {
 test_known_broken_failure_ () {
 	test_broken=$(($test_broken+1))
 	say_color pass "%-6s" "BROKEN"
-	echo " $@"
+	echo " $1"
+	shift
+	echo "$@" | sed -e 's/^/	/'
+	if test "$verbose" != "t"; then cat test.output; fi
 }
 
 test_debug () {
@@ -636,20 +625,8 @@ test_skip () {
 	esac
 }
 
-test_expect_failure () {
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
-	if ! test_skip "$@"
-	then
-		test_run_ "$2"
-		if [ "$?" = 0 -a "$eval_ret" = 0 ]
-		then
-			test_known_broken_ok_ "$1"
-		else
-			test_known_broken_failure_ "$1"
-		fi
-	fi
+test_subtest_known_broken () {
+	test_subtest_known_broken_=1
 }
 
 test_expect_success () {
-- 
1.7.5.4

Re: [PATCH 2/3] test: improve known broken tests support

[Austin Clements]

Great idea!

Just a heads-up, this will conflict with the first two patches in the
atomicity series (though git may not detect the second conflict).
Both are trivial patches, though, so the merge should be easy.

Three minor comments below.

On Sun, Jul 3, 2011 at 9:59 PM, Dmitry Kurochkin
<dmitry.kurochkin@gmail.com> wrote:
> There is existing support for broken tests.  But it is not convenient
> to use.  The primary issue is that we have to maintain a set of
> test_expect_*_failure functions which are equivalent to the normal
> test_expect_* counterparts except for what functions are called for
> result reporting.  The patch adds test_subtest_known_broken function
> which marks a subset as broken, making the normal test_expect_*
> functions behave as test_expect_*_failure.  All test_expect_*_failure
> functions are removed.  Test_known_broken_failure_ is changed to
> format details the same way as test_failure_ does.
>
> Another benefit of this change is that the diff when a broken test is
> fixed would be small and nice.
>
> Documentation is updated accordingly.
> ---
>  test/README      |   17 ++++++++---------
>  test/test-lib.sh |   53 +++++++++++++++--------------------------------------
>  2 files changed, 23 insertions(+), 47 deletions(-)
>
> diff --git a/test/README b/test/README
> index a245bf1..f926b9f 100644
> --- a/test/README
> +++ b/test/README
> @@ -132,20 +132,19 @@ library for your script to use.
>    <script>.  If it yields success, test is considered
>    successful.  <message> should state what it is testing.
>
> - test_expect_failure <message> <script>
> -
> -   This is NOT the opposite of test_expect_success, but is used
> -   to mark a test that demonstrates a known breakage.  Unlike
> -   the usual test_expect_success tests, which say "ok" on
> -   success and "FAIL" on failure, this will say "FIXED" on
> -   success and "still broken" on failure.  Failures from these
> -   tests won't cause -i (immediate) to stop.
> -
>  test_begin_subtest <message>
>
>    Set the test description message for a subsequent test_expect_equal
>    invocation (see below).
>
> + test_subtest_known_broken
> +
> +   Mark the current test as broken.  Such tests are expected to fail.
> +   Unlike the normal tests, which say "PASS" on success and "FAIL" on
> +   failure, these will say "FIXED" on success and "BROKEN" on failure.
> +   Failures from these tests won't cause -i (immediate) to stop.  This
> +   must be called before any test_expect_* function.

Perhaps "A test must call this before any test_expect_* function"?
There can be earlier test_expect_* calls from other tests.

> +
>  test_expect_equal <output> <expected>
>
>    This is an often-used convenience function built on top of
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 22e387e..0cd4170 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -424,6 +424,7 @@ test_begin_subtest ()
>        error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
>     fi
>     test_subtest_name="$1"
> +    test_subtest_known_broken_=
>     # Remember stdout and stderr file descriptors and redirect test
>     # output to the previously prepared file descriptors 3 and 4 (see
>     # below)
> @@ -484,29 +485,6 @@ test_expect_equal_file ()
>     fi
>  }
>
> -test_expect_equal_failure ()
> -{
> -       exec 1>&6 2>&7          # Restore stdout and stderr
> -       inside_subtest=
> -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> -       test "$#" = 2 ||
> -       error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
> -
> -       output="$1"
> -       expected="$2"
> -       if ! test_skip "$@"
> -       then
> -               if [ "$output" = "$expected" ]; then
> -                       test_known_broken_ok_ "$test_subtest_name"
> -               else
> -                       test_known_broken_failure_ "$test_subtest_name"
> -                       testname=$this_test.$test_count
> -                       echo "$expected" > $testname.expected
> -                       echo "$output" > $testname.output
> -               fi
> -    fi
> -}
> -
>  NOTMUCH_NEW ()
>  {
>     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
> @@ -568,12 +546,20 @@ test_have_prereq () {
>  # the text_expect_* functions instead.
>
>  test_ok_ () {
> +       if [ "$test_subtest_known_broken_" = 1 ]; then
> +               test_known_broken_ok_ "$@"
> +               return
> +       fi
>        test_success=$(($test_success + 1))
>        say_color pass "%-6s" "PASS"
>        echo " $@"
>  }
>
>  test_failure_ () {
> +       if [ "$test_subtest_known_broken_" = 1 ]; then
> +               test_known_broken_failure_ "$@"
> +               return
> +       fi
>        test_failure=$(($test_failure + 1))
>        say_color error "%-6s" "FAIL"
>        echo " $1"
> @@ -592,7 +578,10 @@ test_known_broken_ok_ () {
>  test_known_broken_failure_ () {
>        test_broken=$(($test_broken+1))
>        say_color pass "%-6s" "BROKEN"
> -       echo " $@"
> +       echo " $1"
> +       shift
> +       echo "$@" | sed -e 's/^/        /'
> +       if test "$verbose" != "t"; then cat test.output; fi

Would it be possible to keep just the one copy of the failure printing
code in test_failure_?

>  }
>
>  test_debug () {
> @@ -636,20 +625,8 @@ test_skip () {
>        esac
>  }
>
> -test_expect_failure () {
> -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> -       test "$#" = 2 ||
> -       error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
> -       if ! test_skip "$@"
> -       then
> -               test_run_ "$2"
> -               if [ "$?" = 0 -a "$eval_ret" = 0 ]
> -               then
> -                       test_known_broken_ok_ "$1"
> -               else
> -                       test_known_broken_failure_ "$1"
> -               fi
> -       fi
> +test_subtest_known_broken () {
> +       test_subtest_known_broken_=1

The convention in test-lib appears to be "t" for true variables.

>  }
>
>  test_expect_success () {
> --
> 1.7.5.4

Re: [PATCH 2/3] test: improve known broken tests support

[Dmitry Kurochkin]

On Sun, 3 Jul 2011 23:42:13 -0400, Austin Clements <amdragon@mit.edu> wrote:
> Great idea!
> 
> Just a heads-up, this will conflict with the first two patches in the
> atomicity series (though git may not detect the second conflict).
> Both are trivial patches, though, so the merge should be easy.
> 

Oh.  I hope atomicity will be there soon.

> Three minor comments below.
> 
> On Sun, Jul 3, 2011 at 9:59 PM, Dmitry Kurochkin
> <dmitry.kurochkin@gmail.com> wrote:
> > There is existing support for broken tests.  But it is not convenient
> > to use.  The primary issue is that we have to maintain a set of
> > test_expect_*_failure functions which are equivalent to the normal
> > test_expect_* counterparts except for what functions are called for
> > result reporting.  The patch adds test_subtest_known_broken function
> > which marks a subset as broken, making the normal test_expect_*
> > functions behave as test_expect_*_failure.  All test_expect_*_failure
> > functions are removed.  Test_known_broken_failure_ is changed to
> > format details the same way as test_failure_ does.
> >
> > Another benefit of this change is that the diff when a broken test is
> > fixed would be small and nice.
> >
> > Documentation is updated accordingly.
> > ---
> >  test/README      |   17 ++++++++---------
> >  test/test-lib.sh |   53 +++++++++++++++--------------------------------------
> >  2 files changed, 23 insertions(+), 47 deletions(-)
> >
> > diff --git a/test/README b/test/README
> > index a245bf1..f926b9f 100644
> > --- a/test/README
> > +++ b/test/README
> > @@ -132,20 +132,19 @@ library for your script to use.
> >    <script>.  If it yields success, test is considered
> >    successful.  <message> should state what it is testing.
> >
> > - test_expect_failure <message> <script>
> > -
> > -   This is NOT the opposite of test_expect_success, but is used
> > -   to mark a test that demonstrates a known breakage.  Unlike
> > -   the usual test_expect_success tests, which say "ok" on
> > -   success and "FAIL" on failure, this will say "FIXED" on
> > -   success and "still broken" on failure.  Failures from these
> > -   tests won't cause -i (immediate) to stop.
> > -
> >  test_begin_subtest <message>
> >
> >    Set the test description message for a subsequent test_expect_equal
> >    invocation (see below).
> >
> > + test_subtest_known_broken
> > +
> > +   Mark the current test as broken.  Such tests are expected to fail.
> > +   Unlike the normal tests, which say "PASS" on success and "FAIL" on
> > +   failure, these will say "FIXED" on success and "BROKEN" on failure.
> > +   Failures from these tests won't cause -i (immediate) to stop.  This
> > +   must be called before any test_expect_* function.
> 
> Perhaps "A test must call this before any test_expect_* function"?
> There can be earlier test_expect_* calls from other tests.
> 

makes sense

> > +
> >  test_expect_equal <output> <expected>
> >
> >    This is an often-used convenience function built on top of
> > diff --git a/test/test-lib.sh b/test/test-lib.sh
> > index 22e387e..0cd4170 100755
> > --- a/test/test-lib.sh
> > +++ b/test/test-lib.sh
> > @@ -424,6 +424,7 @@ test_begin_subtest ()
> >        error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
> >     fi
> >     test_subtest_name="$1"
> > +    test_subtest_known_broken_=
> >     # Remember stdout and stderr file descriptors and redirect test
> >     # output to the previously prepared file descriptors 3 and 4 (see
> >     # below)
> > @@ -484,29 +485,6 @@ test_expect_equal_file ()
> >     fi
> >  }
> >
> > -test_expect_equal_failure ()
> > -{
> > -       exec 1>&6 2>&7          # Restore stdout and stderr
> > -       inside_subtest=
> > -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> > -       test "$#" = 2 ||
> > -       error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
> > -
> > -       output="$1"
> > -       expected="$2"
> > -       if ! test_skip "$@"
> > -       then
> > -               if [ "$output" = "$expected" ]; then
> > -                       test_known_broken_ok_ "$test_subtest_name"
> > -               else
> > -                       test_known_broken_failure_ "$test_subtest_name"
> > -                       testname=$this_test.$test_count
> > -                       echo "$expected" > $testname.expected
> > -                       echo "$output" > $testname.output
> > -               fi
> > -    fi
> > -}
> > -
> >  NOTMUCH_NEW ()
> >  {
> >     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
> > @@ -568,12 +546,20 @@ test_have_prereq () {
> >  # the text_expect_* functions instead.
> >
> >  test_ok_ () {
> > +       if [ "$test_subtest_known_broken_" = 1 ]; then
> > +               test_known_broken_ok_ "$@"
> > +               return
> > +       fi
> >        test_success=$(($test_success + 1))
> >        say_color pass "%-6s" "PASS"
> >        echo " $@"
> >  }
> >
> >  test_failure_ () {
> > +       if [ "$test_subtest_known_broken_" = 1 ]; then
> > +               test_known_broken_failure_ "$@"
> > +               return
> > +       fi
> >        test_failure=$(($test_failure + 1))
> >        say_color error "%-6s" "FAIL"
> >        echo " $1"
> > @@ -592,7 +578,10 @@ test_known_broken_ok_ () {
> >  test_known_broken_failure_ () {
> >        test_broken=$(($test_broken+1))
> >        say_color pass "%-6s" "BROKEN"
> > -       echo " $@"
> > +       echo " $1"
> > +       shift
> > +       echo "$@" | sed -e 's/^/        /'
> > +       if test "$verbose" != "t"; then cat test.output; fi
> 
> Would it be possible to keep just the one copy of the failure printing
> code in test_failure_?
> 

Sure, it is possible if I am not so lazy.

> >  }
> >
> >  test_debug () {
> > @@ -636,20 +625,8 @@ test_skip () {
> >        esac
> >  }
> >
> > -test_expect_failure () {
> > -       test "$#" = 3 && { prereq=$1; shift; } || prereq=
> > -       test "$#" = 2 ||
> > -       error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
> > -       if ! test_skip "$@"
> > -       then
> > -               test_run_ "$2"
> > -               if [ "$?" = 0 -a "$eval_ret" = 0 ]
> > -               then
> > -                       test_known_broken_ok_ "$1"
> > -               else
> > -                       test_known_broken_failure_ "$1"
> > -               fi
> > -       fi
> > +test_subtest_known_broken () {
> > +       test_subtest_known_broken_=1
> 
> The convention in test-lib appears to be "t" for true variables.
> 

Thanks for the hint.

Will follow with an amend.

Regards,
  Dmitry

> >  }
> >
> >  test_expect_success () {
> > --
> > 1.7.5.4

[PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Human-friendly scenario:

* open a thread where a message which ends with an HTML part is
  followed by another message

* make the first message visible

* goto the beginning of the second message (first line, first colon)

* hit "RET"

Result: nothing happens except for "No URL at point" message

Expected result: the second message is shown/hidden

The root cause is that the HTML part has `keymap' text property set.
In particular, "RET" is bound to visit a URL at point.  The problem is
that `keymap' property affects the next character following the region
it is set to (see elisp manual [1]).  Hence, the first character of
the next message has a keymap inherited from the HTML part.

[1] http://www.gnu.org/software/emacs/elisp/html_node/Special-Properties.html
---
 test/emacs |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index f465e2b..8b627c7 100755
--- a/test/emacs
+++ b/test/emacs
@@ -342,4 +342,30 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
+test_begin_subtest 'Hiding message following HTML part'
+test_subtest_known_broken
+id='html-message@notmuchmail.org'
+emacs_deliver_message \
+    'HTML message' \
+    '' \
+    "(message-goto-eoh)
+     (insert \"Message-ID: <$id>\n\")
+     (message-goto-body)
+     (mml-insert-part \"text/html\")
+     (insert \"<body>This is a test HTML message</body>\")"
+emacs_deliver_message \
+    'Reply to HTML message' \
+    'This is a reply to the test HTML message' \
+    "(message-goto-eoh)
+     (insert \"In-Reply-To: <$id>\n\")"
+test_emacs "(notmuch-show \"id:$id\") \
+            (notmuch-show-next-message) \
+            (command-execute (key-binding (kbd \"RET\"))) \
+            (test-visible-output)"
+test_emacs "(notmuch-show \"id:$id\") \
+            (notmuch-show-next-message) \
+            (notmuch-show-toggle-message) \
+            (test-visible-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.5.4

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[David Bremner]

On Mon,  4 Jul 2011 05:59:03 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Result: nothing happens except for "No URL at point" message
> 
> Expected result: the second message is shown/hidden

I didn't track out why so far, but this test is show as FIXED when
compiled in a Debian chroot.

see e.g.
    
     https://buildd.debian.org/status/fetch.php?pkg=notmuch&arch=kfreebsd-amd64&ver=0.9%7Erc1-1&stamp=1316965004

Someone else also reported this on the IRC channel; not sure if that was
Debian or somewhere else.

d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Hi David.

On Mon, 26 Sep 2011 08:01:16 -0300, David Bremner <david@tethera.net> wrote:
> On Mon,  4 Jul 2011 05:59:03 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Result: nothing happens except for "No URL at point" message
> > 
> > Expected result: the second message is shown/hidden
> 
> I didn't track out why so far, but this test is show as FIXED when
> compiled in a Debian chroot.
> 
> see e.g.
>     
>      https://buildd.debian.org/status/fetch.php?pkg=notmuch&arch=kfreebsd-amd64&ver=0.9%7Erc1-1&stamp=1316965004
> 
> Someone else also reported this on the IRC channel; not sure if that was
> Debian or somewhere else.
> 

I see it failing on my system (Debian unstable).  But I have some more
patches on top of master.

Most likely the test passes because emacs is run in server mode and
visibility stuff works differently.  I sent a patch series [1] to run
emacs in screen exactly for this reason.  Please consider pushing it.
Then the test should fail as expected.

Note, if you push the series, a new dependency would be required to run
the tests - screen(1).  Also, the emacs tests should handle missing
screen(1) better using prereqs.  But there are no prereqs for emacs and
gpg anyway... [2]

Regards,
  Dmitry

[1] id:"1309496122-4965-1-git-send-email-dmitry.kurochkin@gmail.com"
[2] id:"874o2germq.fsf@gmail.com"

> d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[David Bremner]

On Mon, 26 Sep 2011 21:25:08 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Most likely the test passes because emacs is run in server mode and
> visibility stuff works differently.  I sent a patch series [1] to run
> emacs in screen exactly for this reason.  Please consider pushing it.
> Then the test should fail as expected.

> [1] id:"1309496122-4965-1-git-send-email-dmitry.kurochkin@gmail.com"
> [2] id:"874o2germq.fsf@gmail.com"

I'd like to hear more feedback from people on the list about using
screen in the tests. Is this a portability issue for non-linux users?

d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Hi David.

On Sat, 01 Oct 2011 08:51:20 -0300, David Bremner <david@tethera.net> wrote:
> On Mon, 26 Sep 2011 21:25:08 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Most likely the test passes because emacs is run in server mode and
> > visibility stuff works differently.  I sent a patch series [1] to run
> > emacs in screen exactly for this reason.  Please consider pushing it.
> > Then the test should fail as expected.
> 
> > [1] id:"1309496122-4965-1-git-send-email-dmitry.kurochkin@gmail.com"
> > [2] id:"874o2germq.fsf@gmail.com"
> 

I was wrong about it.  I am moving to a new system.  And I see the test
pass on one system and fail on another.  So it is not related to running
emacs tests inside screen.  At the moment, I have no idea why it behaves
differently on the two systems.  Both are Debian unstable (one is
current another 2 weeks old).  Could it be some extra emacs packages
installed?  I know notmuch disables system and user init files, but
perhaps it is not enough?

> I'd like to hear more feedback from people on the list about using
> screen in the tests. Is this a portability issue for non-linux users?
> 

Nevertheless, I believe running emacs tests inside screen is a
requirement for doing complex UI tests.  For example, `window-end'
function behaves differently when there is no emacs frame.  Of course,
it does not have to be screen, we may use any other similar tool
(e.g. tmux, dtach).  We may even use a dummy X server.  But screen seems
to be the best option.

I see no problem with not running emacs tests on systems which do not
have screen.  After all, emacs is a UI.  Core tests would run fine as
before.

BTW there were patches to support tmux (or maybe it was dtach).  IIRC
the consensus was that supporting it does not worth complicating test
lib.  I think this can be reconsidered if it turns out that many users
do not have screen, but do have tmux.  But you never know if the
patches are not applied.  They have been around for several months, I
doubt you would suddenly get feedback now.

Regards,
  Dmitry

> d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Thomas Jost]

[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]

On Sun, 02 Oct 2011 05:45:53 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> BTW there were patches to support tmux (or maybe it was dtach).  IIRC
> the consensus was that supporting it does not worth complicating test
> lib.  I think this can be reconsidered if it turns out that many users
> do not have screen, but do have tmux.  But you never know if the
> patches are not applied.  They have been around for several months, I
> doubt you would suddenly get feedback now.

I'm the one that wrote the tmux patch (and suggested dtach), and yes in
the end the consensus was that it wasn't worth it.

About adding prereqs to the test suite -- in my private branch I have a
rebased and fixed version of the patch series sent by Pieter Praet [1].
It worked fine last time I tested it on a system without screen and in a
chroot without emacs/gpg/screen, so if anyone is interested I can post
it here. (Cc'ing Pieter)

[1] id:"1307016220-17509-1-git-send-email-pieter@praet.org"

-- 
Thomas/Schnouki

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Hi Thomas.

On Mon, 03 Oct 2011 14:39:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> On Sun, 02 Oct 2011 05:45:53 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > BTW there were patches to support tmux (or maybe it was dtach).  IIRC
> > the consensus was that supporting it does not worth complicating test
> > lib.  I think this can be reconsidered if it turns out that many users
> > do not have screen, but do have tmux.  But you never know if the
> > patches are not applied.  They have been around for several months, I
> > doubt you would suddenly get feedback now.
> 
> I'm the one that wrote the tmux patch (and suggested dtach), and yes in
> the end the consensus was that it wasn't worth it.
> 
> About adding prereqs to the test suite -- in my private branch I have a
> rebased and fixed version of the patch series sent by Pieter Praet [1].
> It worked fine last time I tested it on a system without screen and in a
> chroot without emacs/gpg/screen, so if anyone is interested I can post
> it here. (Cc'ing Pieter)
> 

Cool.  You have Pieter's series plus a patch for screen, right?

Please post it, I am interested :)

Regards,
  Dmitry

> [1] id:"1307016220-17509-1-git-send-email-pieter@praet.org"
> 
> -- 
> Thomas/Schnouki

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[David Bremner]

On Mon, 03 Oct 2011 14:39:36 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
Non-text part: multipart/signed
> 
> About adding prereqs to the test suite -- in my private branch I have a
> rebased and fixed version of the patch series sent by Pieter Praet [1].
> It worked fine last time I tested it on a system without screen and in a
> chroot without emacs/gpg/screen, so if anyone is interested I can post
> it here. (Cc'ing Pieter)
> 

Yes please, rebased against current master (which already has the gdb
 pre-reqs). I think we should add prereqs first, then screen based tests.

David.

[PATCH 00/13] Test prereqs and screen-based Emacs tests

[Thomas Jost]

Here it is: a rebased version of Pieter's patch series adding prereqs for the
emacs and crypto tests [1], and Dmitry's patches for running emacs inside screen
in the test suite [2]. (Please note that this one also includes fixes to improve
hidden signatures handling in notmuch-show-advance-and-archive.)

I had to do several changes to the original patches:
- prereqs are not tested using test_expect_success as they were in Pieter's
  original patches, but using a new function called test_set_bin_prereq. I wrote
  this before the gdb prereq was added, hence the different way to set it.

- some fixes in Pieter's patches so that it actually works when gpg is not
  installed. Can't exactly remember what (...but you can just check his original
  patches), but in the end it was working fine in a chroot without gpg.

- I added a little patch to smtp-dummy that makes the test suite work again in
  Emacs 24 (tested with emacs-pretest 24.0.90).

Here are the results when running the test suite on my computer:
- without GNU Screen: 
    All 247 tests behaved as expected (1 expected failure).
    46 tests skipped.
- with GNU Screen:
    242/247 tests passed.
    2 broken tests failed as expected.
    3 tests failed.

(The 3 failed tests come from some trouble with Emacs 24, I'll try to fix this
later.)

*Many* thanks to Dmitry Kurochkin and Pieter Praet for their work!

Regards,
Thomas

[1] id:"1307016220-17509-1-git-send-email-pieter@praet.org"
[2] id:"1309496122-4965-1-git-send-email-dmitry.kurochkin@gmail.com"

Dmitry Kurochkin (7):
  test: run emacs inside screen
  test: avoid using screen(1) configuration files
  test: do not set frame width in emacs
  test: `notmuch-show-advance-and-archive' with invisible signature
  emacs: improve hidden signatures handling in
    notmuch-show-advance-and-archive
  emacs: remove no longer used functions from notmuch-show.el
  emacs: remove unused `point-invisible-p' function

Pieter Praet (4):
  test: add 'GnuPG' prereq to dependent 'crypto' tests
  test: add 'Emacs' prereq to dependent 'crypto' tests
  test: add 'Emacs' prereq to dependent 'emacs' tests
  test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer'
    tests

Thomas Jost (2):
  test: define a helper function for defining prereqs on executables
  test: make smtp-dummy work with Emacs 24

 emacs/notmuch-lib.el           |   15 -------
 emacs/notmuch-show.el          |   25 ++++-------
 test/crypto                    |   46 ++++++++++++++-------
 test/emacs                     |   85 ++++++++++++++++++++++++---------------
 test/emacs-large-search-buffer |    9 +++-
 test/smtp-dummy.c              |    2 +-
 test/test-lib.el               |    3 -
 test/test-lib.sh               |   31 +++++++++++++--
 8 files changed, 127 insertions(+), 89 deletions(-)

-- 
1.7.6.4

Re: [PATCH 00/13] Test prereqs and screen-based Emacs tests

[Pieter Praet]

On Mon,  3 Oct 2011 18:47:14 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> Here it is: a rebased version of Pieter's patch series adding prereqs for the
> emacs and crypto tests [1], and Dmitry's patches for running emacs inside screen
> in the test suite [2]. (Please note that this one also includes fixes to improve
> hidden signatures handling in notmuch-show-advance-and-archive.)
> 

I'm pleased to see these patches haven't moved to binary oblivion yet!

> I had to do several changes to the original patches:
> - prereqs are not tested using test_expect_success as they were in Pieter's
>   original patches, but using a new function called test_set_bin_prereq. I wrote
>   this before the gdb prereq was added, hence the different way to set it.
> 

Indeed preferrable to using `test_expect_success' due to no longer
overzealously reporting missing prereqs as failures, as well as getting
rid of some duplication;  This should also be used in the atomicity tests.

> - some fixes in Pieter's patches so that it actually works when gpg is not
>   installed. Can't exactly remember what (...but you can just check his original
>   patches), but in the end it was working fine in a chroot without gpg.
> 

Correct. I only added prereqs to actual tests (i.e. calls to
`test_expect_success', `test_expect_equal', and
`test_expect_equal_file'), while they should also have been added to
both the `add_gnupg_home' function and the initialization of
${FINGERPRINT}.

> - I added a little patch to smtp-dummy that makes the test suite work again in
>   Emacs 24 (tested with emacs-pretest 24.0.90).
> 
> Here are the results when running the test suite on my computer:
> - without GNU Screen: 
>     All 247 tests behaved as expected (1 expected failure).
>     46 tests skipped.
> - with GNU Screen:
>     242/247 tests passed.
>     2 broken tests failed as expected.
>     3 tests failed.
> 
> (The 3 failed tests come from some trouble with Emacs 24, I'll try to fix this
> later.)
> 
> *Many* thanks to Dmitry Kurochkin and Pieter Praet for their work!
> 

Thanks to you as well!  All this duplication of effort is a real shame
though.  I'd also have preferred your fixes being in separate commits.

I'll be commenting on these modified commits where needed, and have
re-submitted my original series (rebased to current master) in a new
thread [1].

> Regards,
> Thomas
> 
> [1] id:"1307016220-17509-1-git-send-email-pieter@praet.org"
> [2] id:"1309496122-4965-1-git-send-email-dmitry.kurochkin@gmail.com"
> 
> Dmitry Kurochkin (7):
>   test: run emacs inside screen
>   test: avoid using screen(1) configuration files
>   test: do not set frame width in emacs
>   test: `notmuch-show-advance-and-archive' with invisible signature
>   emacs: improve hidden signatures handling in
>     notmuch-show-advance-and-archive
>   emacs: remove no longer used functions from notmuch-show.el
>   emacs: remove unused `point-invisible-p' function
> 
> Pieter Praet (4):
>   test: add 'GnuPG' prereq to dependent 'crypto' tests
>   test: add 'Emacs' prereq to dependent 'crypto' tests
>   test: add 'Emacs' prereq to dependent 'emacs' tests
>   test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer'
>     tests
> 
> Thomas Jost (2):
>   test: define a helper function for defining prereqs on executables
>   test: make smtp-dummy work with Emacs 24
> 
>  emacs/notmuch-lib.el           |   15 -------
>  emacs/notmuch-show.el          |   25 ++++-------
>  test/crypto                    |   46 ++++++++++++++-------
>  test/emacs                     |   85 ++++++++++++++++++++++++---------------
>  test/emacs-large-search-buffer |    9 +++-
>  test/smtp-dummy.c              |    2 +-
>  test/test-lib.el               |    3 -
>  test/test-lib.sh               |   31 +++++++++++++--
>  8 files changed, 127 insertions(+), 89 deletions(-)
> 
> -- 
> 1.7.6.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch


Peace

-- 
Pieter

[1] id:"1320176954-4897-1-git-send-email-pieter@praet.org"

Re: [PATCH 00/13] Test prereqs and screen-based Emacs tests

[David Bremner]

On Tue, 01 Nov 2011 20:54:49 +0100, Pieter Praet <pieter@praet.org> wrote:
> 
> I'll be commenting on these modified commits where needed, and have
> re-submitted my original series (rebased to current master) in a new
> thread [1].
> 

I'm having trouble sorting out which of these series should be
considered for notmuch master. Is there something in the rebased version
of Pieter's patches that is not present in Thomas's rebased version?

d

Re: [PATCH 00/13] Test prereqs and screen-based Emacs tests

[Pieter Praet]

On Sun, 13 Nov 2011 13:46:30 -0400, David Bremner <david@tethera.net> wrote:
> On Tue, 01 Nov 2011 20:54:49 +0100, Pieter Praet <pieter@praet.org> wrote:
> > 
> > I'll be commenting on these modified commits where needed, and have
> > re-submitted my original series (rebased to current master) in a new
> > thread [1].
> > 
> 
> I'm having trouble sorting out which of these series should be
> considered for notmuch master. Is there something in the rebased version
> of Pieter's patches that is not present in Thomas's rebased version?
> 

Nope, I simply rebased my original series, warts and all.

Thomas' submission contains a number of fixes (missing prereqs) and
improvements (test_set_bin_prereq) that should definitely be included
before the series is applied, as well as some stuff that belongs in
separate commits (e.g. screen prereq, which should now check for dtach
instead) and a tiny mistake in the "Reply within emacs" subtest [1].

Also, both Jameson [2] and Ali [3] have brought up some valid
points/concerns re the way we find and set prereqs.

Thomas, could you please rebase your fixes on top of my rebased series,
and submit them in the new thread [4] ?  The whole deal could then be
applied on a feature branch and merged into mainline integrally.

> d


Peace

-- 
Pieter

[1] id:"87fwi7obso.fsf@praet.org"
[2] id:"87hb2n4k5c.fsf@servo.finestructure.net"
[3] id:"20111101202025.GA8248@hayalet"
[4] id:"1320176954-4897-1-git-send-email-pieter@praet.org"

[PATCH 01/13] test: define a helper function for defining prereqs on executables

[Thomas Jost]

While test_expect_success could be used to define these prereqs, this is
probably not a good idea: if a prereq is not available, using
test_expect_success would result in a test being reported as FAILED at the end
of the test suite (and its dependencies as skipped).

On the contrary, when using test_set_bin_prereq, no test will be reported as
FAILED.
---
 test/test-lib.sh |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f8df6a5..8e16a7e 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -542,6 +542,19 @@ test_have_prereq () {
 	esac
 }
 
+test_set_bin_prereq () {
+	bin=$1
+	name=$2
+	prereq=$3
+	if which $bin &>/dev/null
+	then
+		test_set_prereq $prereq
+	else
+		say_color info "%-6s" "INFO"
+		echo " Missing test prerequisite: $name"
+	fi
+}
+
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
 
-- 
1.7.6.4

[PATCH 02/13] test: add 'GnuPG' prereq to dependent 'crypto' tests

[Thomas Jost]

From: Pieter Praet <pieter@praet.org>

Adds a new test that checks for the presence of 'gpg',
and adds that test as a prereq to all subsequent tests
that rely on GnuPG.

This causes tests with unmet dependencies to be skipped.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/crypto |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/test/crypto b/test/crypto
index b49a4e0..4ee318f 100755
--- a/test/crypto
+++ b/test/crypto
@@ -7,11 +7,16 @@
 test_description='PGP/MIME signature verification and decryption'
 . ./test-lib.sh
 
+# GnuPG is a prereq.
+test_set_bin_prereq gpg "GnuPG" GPG
+
+
 add_gnupg_home ()
 {
     local output
     [ -d ${GNUPGHOME} ] && return
     mkdir -m 0700 "$GNUPGHOME"
+    test_have_prereq GPG || return
     gpg --no-tty --import <$TEST_DIRECTORY/gnupg-secret-key.asc >"$GNUPGHOME"/import.log 2>&1
     test_debug "cat $GNUPGHOME/import.log"
     if (gpg --quick-random --version >/dev/null 2>&1) ; then
@@ -25,13 +30,13 @@ add_gnupg_home ()
 
 add_gnupg_home
 # get key fingerprint
-FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10)
+test_have_prereq GPG && FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-colons --fingerprint | grep '^fpr:' | cut -d: -f10)
 
 # for some reason this is needed for emacs_deliver_message to work,
 # although I can't figure out why
 add_email_corpus
 
-test_expect_success 'emacs delivery of signed message' \
+test_expect_success GPG 'emacs delivery of signed message' \
 'emacs_deliver_message \
     "test signed message 001" \
     "This is a test signed message." \
@@ -64,7 +69,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -99,7 +104,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -132,7 +137,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
@@ -141,7 +146,7 @@ mv "${GNUPGHOME}"{.bak,}
 cat <<EOF >TESTATTACHMENT
 This is a test file.
 EOF
-test_expect_success 'emacs delivery of encrypted message with attachment' \
+test_expect_success GPG 'emacs delivery of encrypted message with attachment' \
 'emacs_deliver_message \
     "test encrypted message 001" \
     "This is a test encrypted message.\n" \
@@ -175,7 +180,7 @@ Non-text part: application/octet-stream
 \fpart}
 \fbody}
 \fmessage}'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -210,7 +215,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/octet-stream",
  "filename": "TESTATTACHMENT"}]}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -221,7 +226,7 @@ output=$(notmuch show --format=json --part=4 --decrypt subject:"test encrypted m
 expected='{"id": 4,
  "content-type": "text/plain",
  "content": "This is a test encrypted message.\n"}'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -231,7 +236,7 @@ notmuch show \
     --part=5 \
     --decrypt \
     subject:"test encrypted message 001" >OUTPUT
-test_expect_equal_file OUTPUT TESTATTACHMENT
+test_expect_equal_file GPG OUTPUT TESTATTACHMENT
 
 test_begin_subtest "decryption failure with missing key"
 mv "${GNUPGHOME}"{,.bak}
@@ -258,12 +263,12 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/octet-stream"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
 
-test_expect_success 'emacs delivery of encrypted + signed message' \
+test_expect_success GPG 'emacs delivery of encrypted + signed message' \
 'emacs_deliver_message \
     "test encrypted message 002" \
     "This is another test encrypted message.\n" \
@@ -298,7 +303,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is another test encrypted message.\n"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -312,7 +317,7 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
 Non-text part: multipart/encrypted
 Non-text part: application/pgp-encrypted
 > This is another test encrypted message.'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
@@ -353,7 +358,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal \
+test_expect_equal GPG \
     "$output" \
     "$expected"
 
-- 
1.7.6.4

[PATCH 03/13] test: add 'Emacs' prereq to dependent 'crypto' tests

[Thomas Jost]

From: Pieter Praet <pieter@praet.org>

Adds a new test that checks for the presence of 'emacs',
and adds that test as a prereq to all subsequent tests
that rely on Emacs.

This causes tests with unmet dependencies to be skipped.

Right now, all crypto tests do depend on Emacs, because it
is used to generate the signed/encrypted messages that are
needed by the tests.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/crypto |   39 +++++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/test/crypto b/test/crypto
index 4ee318f..cd64275 100755
--- a/test/crypto
+++ b/test/crypto
@@ -7,9 +7,20 @@
 test_description='PGP/MIME signature verification and decryption'
 . ./test-lib.sh
 
+# Emacs is a prereq.
+test_set_bin_prereq screen "GNU Screen" SCREEN
+test_have_prereq SCREEN && test_set_bin_prereq emacs "Emacs" EMACS
+
 # GnuPG is a prereq.
 test_set_bin_prereq gpg "GnuPG" GPG
 
+# Some tests have multiple prereqs, but the test_expect_* functions
+# accept only a single argument as prereq tag, and using test_have_prereq
+# in and around tests causes various errors for me, so a dirty workaround
+# will have to do for the time being.
+test_have_prereq EMACS && test_have_prereq GPG \
+    && test_set_prereq EMACS+GPG
+
 
 add_gnupg_home ()
 {
@@ -36,7 +47,7 @@ test_have_prereq GPG && FINGERPRINT=$(gpg --no-tty --list-secret-keys --with-col
 # although I can't figure out why
 add_email_corpus
 
-test_expect_success GPG 'emacs delivery of signed message' \
+test_expect_success EMACS+GPG 'emacs delivery of signed message' \
 'emacs_deliver_message \
     "test signed message 001" \
     "This is a test signed message." \
@@ -69,7 +80,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -104,7 +115,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -137,7 +148,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
@@ -146,7 +157,7 @@ mv "${GNUPGHOME}"{.bak,}
 cat <<EOF >TESTATTACHMENT
 This is a test file.
 EOF
-test_expect_success GPG 'emacs delivery of encrypted message with attachment' \
+test_expect_success EMACS+GPG 'emacs delivery of encrypted message with attachment' \
 'emacs_deliver_message \
     "test encrypted message 001" \
     "This is a test encrypted message.\n" \
@@ -180,7 +191,7 @@ Non-text part: application/octet-stream
 \fpart}
 \fbody}
 \fmessage}'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -215,7 +226,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "application/octet-stream",
  "filename": "TESTATTACHMENT"}]}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -226,7 +237,7 @@ output=$(notmuch show --format=json --part=4 --decrypt subject:"test encrypted m
 expected='{"id": 4,
  "content-type": "text/plain",
  "content": "This is a test encrypted message.\n"}'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -236,7 +247,7 @@ notmuch show \
     --part=5 \
     --decrypt \
     subject:"test encrypted message 001" >OUTPUT
-test_expect_equal_file GPG OUTPUT TESTATTACHMENT
+test_expect_equal_file EMACS+GPG OUTPUT TESTATTACHMENT
 
 test_begin_subtest "decryption failure with missing key"
 mv "${GNUPGHOME}"{,.bak}
@@ -263,12 +274,12 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/octet-stream"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 mv "${GNUPGHOME}"{.bak,}
 
-test_expect_success GPG 'emacs delivery of encrypted + signed message' \
+test_expect_success EMACS+GPG 'emacs delivery of encrypted + signed message' \
 'emacs_deliver_message \
     "test encrypted message 002" \
     "This is another test encrypted message.\n" \
@@ -303,7 +314,7 @@ expected='[[[{"id": "XXXXX",
  "content-type": "text/plain",
  "content": "This is another test encrypted message.\n"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -317,7 +328,7 @@ On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> w
 Non-text part: multipart/encrypted
 Non-text part: application/pgp-encrypted
 > This is another test encrypted message.'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
@@ -358,7 +369,7 @@ expected='[[[{"id": "XXXXX",
  {"id": 3,
  "content-type": "application/pgp-signature"}]}]},
  []]]]'
-test_expect_equal GPG \
+test_expect_equal EMACS+GPG \
     "$output" \
     "$expected"
 
-- 
1.7.6.4

Re: [PATCH 03/13] test: add 'Emacs' prereq to dependent 'crypto' tests

[Pieter Praet]

On Mon,  3 Oct 2011 18:47:17 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> [...]

The SCREEN prereq would preferrably be added in a separate commit.

Also, you appear to have given *every* test the EMACS+GPG prereq,
while only the ones using `emacs_deliver_message' require EMACS.


Peace

-- 
Pieter

Re: [PATCH 03/13] test: add 'Emacs' prereq to dependent 'crypto' tests

[Pieter Praet]

On Tue, 01 Nov 2011 20:56:17 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Mon,  3 Oct 2011 18:47:17 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> [...]
>
> Also, you appear to have given *every* test the EMACS+GPG prereq,
> while only the ones using `emacs_deliver_message' require EMACS.
>
> [...]

Again, my mistake. All subsequent tests appear to be using the message
sent using Emacs (is this a good idea?), so the new prereqs are valid.


Peace

-- 
Pieter

[PATCH 04/13] test: add 'Emacs' prereq to dependent 'emacs' tests

[Thomas Jost]

From: Pieter Praet <pieter@praet.org>

Adds a new test that checks for the presence of 'emacs',
and adds that test as a prereq to all subsequent tests
that rely on Emacs.

This causes tests with unmet dependencies to be skipped.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs |   71 ++++++++++++++++++++++++++++++++---------------------------
 1 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/test/emacs b/test/emacs
index 8b627c7..50958ec 100755
--- a/test/emacs
+++ b/test/emacs
@@ -3,6 +3,11 @@
 test_description="emacs interface"
 . test-lib.sh
 
+# Emacs is a prereq.
+test_set_bin_prereq screen "GNU Screen" SCREEN
+test_have_prereq SCREEN && test_set_bin_prereq emacs "Emacs" EMACS
+
+
 EXPECTED=$TEST_DIRECTORY/emacs.expected-output
 
 add_email_corpus
@@ -10,7 +15,7 @@ add_email_corpus
 test_begin_subtest "Basic notmuch-hello view in emacs"
 test_emacs '(notmuch-hello)
 	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-hello
 
 test_begin_subtest "Saved search with 0 results"
 test_emacs '(let ((notmuch-show-empty-saved-searches t)
@@ -20,20 +25,20 @@ test_emacs '(let ((notmuch-show-empty-saved-searches t)
 			("empty" . "tag:doesnotexist"))))
 	      (notmuch-hello)
 	      (test-output))'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-with-empty
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-hello-with-empty
 
 test_begin_subtest "No saved searches displayed (all with 0 results)"
 test_emacs '(let ((notmuch-saved-searches
 		   '\''(("empty" . "tag:doesnotexist"))))
 	      (notmuch-hello)
 	      (test-output))'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-hello-no-saved-searches
 
 test_begin_subtest "Basic notmuch-search view in emacs"
 test_emacs '(notmuch-search "tag:inbox")
 	    (notmuch-test-wait)
 	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-search-tag-inbox
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-search-tag-inbox
 
 test_begin_subtest "Navigation of notmuch-hello to search results"
 test_emacs '(notmuch-hello)
@@ -42,13 +47,13 @@ test_emacs '(notmuch-hello)
 	    (widget-button-press (point))
 	    (notmuch-test-wait)
 	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-hello-view-inbox
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-hello-view-inbox
 
 test_begin_subtest "Basic notmuch-show view in emacs"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
 test_emacs "(notmuch-show \"$maildir_storage_thread\")
 	    (test-output)"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "notmuch-show for message with invalid From"
 add_message "[subject]=\"message-with-invalid-from\"" \
@@ -64,7 +69,7 @@ Date: Tue, 05 Jan 2001 15:43:57 -0000
 
 This is just a test message (#1)
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "Navigation of notmuch-search to thread view"
 test_emacs '(notmuch-search "tag:inbox")
@@ -74,7 +79,7 @@ test_emacs '(notmuch-search "tag:inbox")
 	    (notmuch-search-show-thread)
 	    (notmuch-test-wait)
 	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Add tag from search view"
 os_x_darwin_thread=$(notmuch search --output=threads id:ddd65cda0911171950o4eea4389v86de9525e46052d3@mail.gmail.com)
@@ -82,26 +87,26 @@ test_emacs "(notmuch-search \"$os_x_darwin_thread\")
 	    (notmuch-test-wait)
 	    (notmuch-search-add-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-search-view unread)"
 
 test_begin_subtest "Remove tag from search view"
 test_emacs "(notmuch-search \"$os_x_darwin_thread\")
 	    (notmuch-test-wait)
 	    (notmuch-search-remove-tag \"tag-from-search-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Add tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 	    (notmuch-show-add-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox tag-from-show-view unread)"
 
 test_begin_subtest "Remove tag from notmuch-show view"
 test_emacs "(notmuch-show \"$os_x_darwin_thread\")
 	    (notmuch-show-remove-tag \"tag-from-show-view\")"
 output=$(notmuch search $os_x_darwin_thread | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
+test_expect_equal EMACS "$output" "thread:XXX   2009-11-18 [4/4] Jjgod Jiang, Alexander Botero-Lowry; [notmuch] Mac OS X/Darwin compatibility issues (inbox unread)"
 
 test_begin_subtest "Message with .. in Message-Id:"
 add_message [id]=123..456@example '[subject]="Message with .. in Message-Id"'
@@ -116,10 +121,10 @@ test_emacs '(notmuch-search "id:\"123..456@example\"")
 	    (notmuch-show-add-tag "show-remove")
 	    (notmuch-show-remove-tag "show-remove")'
 output=$(notmuch search 'id:"123..456@example"' | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
+test_expect_equal EMACS "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Message with .. in Message-Id (inbox search-add show-add)"
 
 test_begin_subtest "Sending a message via (fake) SMTP"
-emacs_deliver_message \
+test_have_prereq EMACS && emacs_deliver_message \
     'Testing message sent via SMTP' \
     'This is a test that messages are sent via SMTP' \
     '(message-goto-to)
@@ -140,12 +145,12 @@ Content-Type: text/plain; charset=us-ascii
 
 This is a test that messages are sent via SMTP
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "Verify that sent messages are saved/searchable (via FCC)"
 notmuch new > /dev/null
 output=$(notmuch search 'subject:"testing message sent via SMTP"' | notmuch_search_sanitize)
-test_expect_equal "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
+test_expect_equal EMACS "$output" "thread:XXX   2000-01-01 [1/1] Notmuch Test Suite; Testing message sent via SMTP (inbox)"
 
 test_begin_subtest "notmuch-fcc-dirs set to nil"
 test_emacs "(let ((notmuch-fcc-dirs nil))
@@ -157,7 +162,7 @@ To:
 Subject: 
 --text follows this line--
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 # Make another FCC maildir specific for the next test
 mkdir -p mail/sent-string/cur
@@ -175,7 +180,7 @@ Subject:
 Fcc: ${MAIL_DIR}/sent-string
 --text follows this line--
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 # Make more FCC maildirs specific for the next test
 mkdir -p mail/sent-list-match/cur
@@ -198,7 +203,7 @@ Subject:
 Fcc: ${MAIL_DIR}/sent-list-match
 --text follows this line--
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 # Make another FCC maildir specific for the next test
 mkdir -p mail/sent-list-catch-all/cur
@@ -218,7 +223,7 @@ Subject:
 Fcc: ${MAIL_DIR}/sent-list-catch-all
 --text follows this line--
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "notmuch-fcc-dirs set to a list (no match)"
 test_emacs "(let ((notmuch-fcc-dirs
@@ -232,14 +237,14 @@ To:
 Subject: 
 --text follows this line--
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "Reply within emacs"
 test_emacs '(notmuch-search "subject:\"testing message sent via SMTP\"")
 	    (notmuch-test-wait)
 	    (notmuch-search-reply-to-thread)
 	    (test-output)'
-sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT
+sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' EMACS OUTPUT
 cat <<EOF >EXPECTED
 From: Notmuch Test Suite <test_suite@notmuchmail.org>
 To: user@example.com
@@ -250,26 +255,26 @@ Fcc: $(pwd)/mail/sent
 On 01 Jan 2000 12:00:00 -0000, Notmuch Test Suite <test_suite@notmuchmail.org> wrote:
 > This is a test that messages are sent via SMTP
 EOF
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-attachments"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment1.gz\""))
 	      (notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
 	      (notmuch-show-save-attachments))' > /dev/null 2>&1
-test_expect_equal_file attachment1.gz "$EXPECTED/attachment"
+test_expect_equal_file EMACS attachment1.gz "$EXPECTED/attachment"
 
 test_begin_subtest "Save attachment from within emacs using notmuch-show-save-part"
 # save as archive to test that Emacs does not re-compress .gz
 test_emacs '(let ((standard-input "\"attachment2.gz\""))
 	      (notmuch-show-save-part "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com" 5))' > /dev/null 2>&1
-test_expect_equal_file attachment2.gz "$EXPECTED/attachment"
+test_expect_equal_file EMACS attachment2.gz "$EXPECTED/attachment"
 
 test_begin_subtest "View raw message within emacs"
 test_emacs '(notmuch-show "id:cf0c4d610911171136h1713aa59w9cf9aa31f052ad0a@mail.gmail.com")
 	    (notmuch-show-view-raw-message)
 	    (test-output)'
-test_expect_equal_file OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
+test_expect_equal_file EMACS OUTPUT $EXPECTED/raw-message-cf0c4d-52ad0a
 
 test_begin_subtest "Hiding/showing signature in notmuch-show view"
 maildir_storage_thread=$(notmuch search --output=threads id:20091117190054.GU3165@dottiness.seas.harvard.edu)
@@ -279,7 +284,7 @@ test_emacs "(notmuch-show \"$maildir_storage_thread\")
 	    (search-backward \"Click/Enter to hide.\")
 	    (button-activate (button-at (point)))
 	    (test-output)"
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-show-thread-maildir-storage
 
 test_begin_subtest "Detection and hiding of top-post quoting of message"
 add_message '[subject]="The problem with top-posting"' \
@@ -326,13 +331,13 @@ Thanks for the advice! I will be sure to put it to good use.
 -Top Poster
 
 [ 9-line hidden original message. Click/Enter to show. ]" > EXPECTED
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_begin_subtest "Hiding message in notmuch-show view"
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
 	    (notmuch-show-toggle-message)
 	    (test-visible-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
 test_begin_subtest "Hiding message with visible citation in notmuch-show view"
 test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.gmail.com")
@@ -340,12 +345,12 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 	    (button-activate (button-at (point)))
 	    (notmuch-show-toggle-message)
 	    (test-visible-output)'
-test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
+test_expect_equal_file EMACS OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
 test_begin_subtest 'Hiding message following HTML part'
 test_subtest_known_broken
 id='html-message@notmuchmail.org'
-emacs_deliver_message \
+test_have_prereq EMACS && emacs_deliver_message \
     'HTML message' \
     '' \
     "(message-goto-eoh)
@@ -353,7 +358,7 @@ emacs_deliver_message \
      (message-goto-body)
      (mml-insert-part \"text/html\")
      (insert \"<body>This is a test HTML message</body>\")"
-emacs_deliver_message \
+test_have_prereq EMACS && emacs_deliver_message \
     'Reply to HTML message' \
     'This is a reply to the test HTML message' \
     "(message-goto-eoh)
@@ -366,6 +371,6 @@ test_emacs "(notmuch-show \"id:$id\") \
             (notmuch-show-next-message) \
             (notmuch-show-toggle-message) \
             (test-visible-output \"EXPECTED\")"
-test_expect_equal_file OUTPUT EXPECTED
+test_expect_equal_file EMACS OUTPUT EXPECTED
 
 test_done
-- 
1.7.6.4

Re: [PATCH 04/13] test: add 'Emacs' prereq to dependent 'emacs' tests

[Pieter Praet]

On Mon,  3 Oct 2011 18:47:18 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> [...]

Nice catches @

  - "Sending a message via (fake) SMTP"
  - "Hiding message following HTML part"

(though I think the latter test wasn't there yet when I submitted my
patch series)


New "issues":

  - The SCREEN prereq would preferrably be added in a separate commit.

  - @ "Verify that sent messages are saved/searchable (via FCC)":

    This doesn't need an EMACS prereq.

  - @ "Reply within emacs":

    `sed' will want to treat "EMACS" as an input file, so I think you
    meant to do this:

      test_have_prereq EMACS \
        && sed -i -e 's/^In-Reply-To: <.*>$/In-Reply-To: <XXX>/' OUTPUT


Peace

-- 
Pieter

Re: [PATCH 04/13] test: add 'Emacs' prereq to dependent 'emacs' tests

[Pieter Praet]

On Tue, 01 Nov 2011 20:57:59 +0100, Pieter Praet <pieter@praet.org> wrote:
> On Mon,  3 Oct 2011 18:47:18 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> [...]
>   - @ "Verify that sent messages are saved/searchable (via FCC)":
> 
>     This doesn't need an EMACS prereq.
> [...]

My mistake, scrap this one.

Without Emacs, there wouldn't be a sent message to begin with, so the
prereq is valid.


Peace

-- 
Pieter

[PATCH 05/13] test: add 'Emacs' prereq to dependent 'emacs-large-search-buffer' tests

[Thomas Jost]

From: Pieter Praet <pieter@praet.org>

Adds a new test that checks for the presence of 'emacs',
and adds that test as a prereq to all subsequent tests
that rely on Emacs.

This causes tests with unmet dependencies to be skipped.

Signed-off-by: Pieter Praet <pieter@praet.org>
---
 test/emacs-large-search-buffer |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/test/emacs-large-search-buffer b/test/emacs-large-search-buffer
index 6095e9d..018333a 100755
--- a/test/emacs-large-search-buffer
+++ b/test/emacs-large-search-buffer
@@ -2,6 +2,11 @@
 test_description="Emacs with large search results buffer"
 . test-lib.sh
 
+# Emacs is a prereq.
+test_set_bin_prereq screen "GNU Screen" SCREEN
+test_have_prereq SCREEN && test_set_bin_prereq emacs "Emacs" EMACS
+
+
 x=xxxxxxxxxx # 10
 x=$x$x$x$x$x$x$x$x$x$x # 100
 x=$x$x$x$x$x$x$x$x$x # 900
@@ -27,7 +32,7 @@ test_emacs '(notmuch-search "*")
 	    (notmuch-test-wait)
 	    (test-output)'
 sed -i -e s',  *, ,g' -e 's/xxx*/[BLOB]/g' OUTPUT
-test_expect_equal_file OUTPUT EXPEXTED
+test_expect_equal_file EMACS OUTPUT EXPEXTED
 
 test_begin_subtest "Ensure that emacs doesn't drop error messages"
 test_emacs '(notmuch-search "--this-option-does-not-exist")
@@ -38,6 +43,6 @@ Error: Unexpected output from notmuch search:
 Unrecognized option: --this-option-does-not-exist
 End of search results. (process returned 1)
 EOF
-test_expect_equal_file OUTPUT EXPEXTED
+test_expect_equal_file EMACS OUTPUT EXPEXTED
 
 test_done
-- 
1.7.6.4

[PATCH 06/13] test: run emacs inside screen

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

Before the change, emacs run in daemon mode without any visible
buffers.  Turns out that this affects emacs behavior in some
cases.  In particular, `window-end' function returns `point-max'
instead of the last visible position.  That makes it hard or
impossible to implement some tests.  The patch runs emacs in a
detached screen(1) session.  So that it works exactly as if it
has a visible window.

Note: screen terminates when emacs exits.  So the patch does not
introduce new "running processes left behind" issues.
---
 test/test-lib.sh |   16 ++++++++++++----
 1 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 8e16a7e..f9fd73e 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -852,14 +852,22 @@ EOF
 
 test_emacs () {
 	if [ -z "$EMACS_SERVER" ]; then
+		# start a detached screen session with an emacs server
+		which screen &>/dev/null || return
 		EMACS_SERVER="notmuch-test-suite-$$"
-		"$TMP_DIRECTORY/run_emacs" \
-			--daemon \
+		screen -S "$EMACS_SERVER" -d -m "$TMP_DIRECTORY/run_emacs" \
+			--no-window-system \
 			--eval "(setq server-name \"$EMACS_SERVER\")" \
+			--eval '(server-start)' \
 			--eval "(orphan-watchdog $$)" || return
+		# wait until the emacs server is up
+		until test_emacs '()' 2>/dev/null; do
+			sleep 1
+		done
+	fi
+	if [ "$EMACS_SERVER" ]; then
+		emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 	fi
-
-	emacsclient --socket-name="$EMACS_SERVER" --eval "(progn $@)"
 }
 
 
-- 
1.7.6.4

Re: [PATCH 06/13] test: run emacs inside screen

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 202 bytes --]

So I just found out (via git bisect) that this patch is causing tests
that use smtp-dummy to hang for me.  Has no one else seen the same
problem?  Any suggestions about what the issue could be?

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 06/13] test: run emacs inside screen

[Thomas Jost]

[-- Attachment #1: Type: text/plain, Size: 751 bytes --]

On Wed, 09 Nov 2011 23:36:12 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> So I just found out (via git bisect) that this patch is causing tests
> that use smtp-dummy to hang for me.  Has no one else seen the same
> problem?  Any suggestions about what the issue could be?
> 
> jamie.

The only time I've seen such an issue, it was with Emacs 24 (pretest),
and I posted a fix here:
id:"1317660447-27520-14-git-send-email-schnouki@schnouki.net"

If it's a problem with smtp-dummy, I suggest you add printf() statements
in smtp-dummy.c and run it in another term, *then* run the test suite,
so that you can see *where* it hangs exactly. That's what I did for the
Emacs 24 issue :)

Regards,

-- 
Thomas/Schnouki

[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]

[PATCH 07/13] test: avoid using screen(1) configuration files

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

Set SCREENRC and SYSSCREENRC environment variables to "/dev/null"
as suggested by Jim Paris to avoid potential problems with
screen(1) configuration files.
---
 test/test-lib.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f9fd73e..b22a25c 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -50,6 +50,8 @@ TZ=UTC
 TERM=dumb
 export LANG LC_ALL PAGER TERM TZ
 GIT_TEST_CMP=${GIT_TEST_CMP:-diff -u}
+export SCREENRC=/dev/null
+export SYSSCREENRC=/dev/null
 
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
-- 
1.7.6.4

[PATCH 08/13] test: do not set frame width in emacs

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

No need for `set-frame-width' in emacs tests since it runs in
screen now.
---
 test/test-lib.el |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/test/test-lib.el b/test/test-lib.el
index a783936..97ae593 100644
--- a/test/test-lib.el
+++ b/test/test-lib.el
@@ -20,9 +20,6 @@
 ;;
 ;; Authors: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>
 
-;; avoid crazy 10-column default of --batch
-(set-frame-width (window-frame (get-buffer-window)) 80)
-
 ;; `read-file-name' by default uses `completing-read' function to read
 ;; user input.  It does not respect `standard-input' variable which we
 ;; use in tests to provide user input.  So replace it with a plain
-- 
1.7.6.4

[PATCH 09/13] test: `notmuch-show-advance-and-archive' with invisible signature

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

Add Emacs test to check that `notmuch-show-advance-and-archive'
works for the last message in thread with invisible signature.
---
 test/emacs |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index 50958ec..fddb5cf 100755
--- a/test/emacs
+++ b/test/emacs
@@ -373,4 +373,18 @@ test_emacs "(notmuch-show \"id:$id\") \
             (test-visible-output \"EXPECTED\")"
 test_expect_equal_file EMACS OUTPUT EXPECTED
 
+test_begin_subtest 'notmuch-show-advance-and-archive with invisible signature'
+message1='id:20091118010116.GC25380@dottiness.seas.harvard.edu'
+message2='id:1258491078-29658-1-git-send-email-dottedmag@dottedmag.net'
+test_emacs "(notmuch-search \"$message1 or $message2\")
+	    (notmuch-test-wait)
+	    (notmuch-search-show-thread)
+	    (goto-char (point-max))
+	    (redisplay)
+	    (notmuch-show-advance-and-archive)
+	    (test-output)"
+test_emacs "(notmuch-show \"$message2\")
+	    (test-output \"EXPECTED\")"
+test_expect_equal_file EMACS OUTPUT EXPECTED
+
 test_done
-- 
1.7.6.4

[PATCH 10/13] emacs: improve hidden signatures handling in notmuch-show-advance-and-archive

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

Use `previous-single-char-property-change' instead of going
through each character by hand and testing it's visibility.  This
fixes `notmuch-show-advance-and-archive' to work for the last
message in thread with hidden signature.
---
 emacs/notmuch-show.el |   17 +++++++++--------
 1 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index 90f9af7..bf267e8 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -1106,17 +1106,18 @@ thread, (remove the \"inbox\" tag from each message). Also kill
 this buffer, and display the next thread from the search from
 which this thread was originally shown."
   (interactive)
-  (let ((end-of-this-message (notmuch-show-message-bottom)))
+  (let* ((end-of-this-message (notmuch-show-message-bottom))
+	 (visible-end-of-this-message (1- end-of-this-message)))
+    (while (invisible-p visible-end-of-this-message)
+      (setq visible-end-of-this-message
+	    (previous-single-char-property-change visible-end-of-this-message
+						  'invisible)))
     (cond
      ;; Ideally we would test `end-of-this-message' against the result
      ;; of `window-end', but that doesn't account for the fact that
-     ;; the end of the message might be hidden, so we have to actually
-     ;; go to the end, walk back over invisible text and then see if
-     ;; point is visible.
-     ((save-excursion
-	(goto-char (- end-of-this-message 1))
-	(notmuch-show-move-past-invisible-backward)
-	(> (point) (window-end)))
+     ;; the end of the message might be hidden.
+     ((and visible-end-of-this-message
+	   (> visible-end-of-this-message (window-end)))
       ;; The bottom of this message is not visible - scroll.
       (scroll-up nil))
 
-- 
1.7.6.4

[PATCH 11/13] emacs: remove no longer used functions from notmuch-show.el

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

Remove `notmuch-show-move-past-invisible-backward' and
`notmuch-show-move-past-invisible-forward' functions which are
unused.
---
 emacs/notmuch-show.el |    8 --------
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/emacs/notmuch-show.el b/emacs/notmuch-show.el
index bf267e8..e6265e8 100644
--- a/emacs/notmuch-show.el
+++ b/emacs/notmuch-show.el
@@ -970,14 +970,6 @@ All currently available key bindings:
     (notmuch-show-move-to-message-top)
     t))
 
-(defun notmuch-show-move-past-invisible-forward ()
-  (while (point-invisible-p)
-    (forward-char)))
-
-(defun notmuch-show-move-past-invisible-backward ()
-  (while (point-invisible-p)
-    (backward-char)))
-
 ;; Functions relating to the visibility of messages and their
 ;; components.
 
-- 
1.7.6.4

[PATCH 12/13] emacs: remove unused `point-invisible-p' function

[Thomas Jost]

From: Dmitry Kurochkin <dmitry.kurochkin@gmail.com>

`point-invisible-p' does not work correctly when `invisible'
property is a list.  There are standard `invisible-p' and related
functions that should be used instead.
---
 emacs/notmuch-lib.el |   15 ---------------
 1 files changed, 0 insertions(+), 15 deletions(-)

diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el
index f93c957..0f856bf 100644
--- a/emacs/notmuch-lib.el
+++ b/emacs/notmuch-lib.el
@@ -105,21 +105,6 @@ the user hasn't set this variable with the old or new value."
 
 ;;
 
-;; XXX: This should be a generic function in emacs somewhere, not
-;; here.
-(defun point-invisible-p ()
-  "Return whether the character at point is invisible.
-
-Here visibility is determined by `buffer-invisibility-spec' and
-the invisible property of any overlays for point. It doesn't have
-anything to do with whether point is currently being displayed
-within the current window."
-  (let ((prop (get-char-property (point) 'invisible)))
-    (if (eq buffer-invisibility-spec t)
-	prop
-      (or (memq prop buffer-invisibility-spec)
-	  (assq prop buffer-invisibility-spec)))))
-
 (defun notmuch-remove-if-not (predicate list)
   "Return a copy of LIST with all items not satisfying PREDICATE removed."
   (let (out)
-- 
1.7.6.4

[PATCH 13/13] test: make smtp-dummy work with Emacs 24

[Thomas Jost]

In Emacs 24, a space is expected after a SMTP response code. If we don't respect
that, smtpmail-send-it will wait forever.
---
 test/smtp-dummy.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/smtp-dummy.c b/test/smtp-dummy.c
index 9da8202..021af11 100644
--- a/test/smtp-dummy.c
+++ b/test/smtp-dummy.c
@@ -71,7 +71,7 @@ static int
 process_command (FILE *peer, FILE *output, const char *command)
 {
 	if (STRNCMP_LITERAL (command, "EHLO ") == 0) {
-		fprintf (peer, "502\r\n");
+		fprintf (peer, "502 not implemented\r\n");
 		fflush (peer);
 	} else if (STRNCMP_LITERAL (command, "HELO ") == 0) {
 		fprintf (peer, "250 localhost\r\n");
-- 
1.7.6.4

Re: [PATCH 13/13] test: make smtp-dummy work with Emacs 24

[David Bremner]

On Mon,  3 Oct 2011 18:47:27 +0200, Thomas Jost <schnouki@schnouki.net> wrote:
> In Emacs 24, a space is expected after a SMTP response code. If we don't respect
> that, smtpmail-send-it will wait forever.

pushed.

d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[David Bremner]

On Mon,  4 Jul 2011 05:59:03 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Result: nothing happens except for "No URL at point" message
> 
> Expected result: the second message is shown/hidden

Unfortunately this patch is unreliable, reporting BROKEN for some
environments and FIXED for others; I think you mentioned it depends
on whether w3m-el is installed? I have reverted it for now to reduce the
noise in the test logs.

d

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Tom Prince]

On Thu, 15 Dec 2011 08:14:08 -0400, David Bremner <david@tethera.net> wrote:
> On Mon,  4 Jul 2011 05:59:03 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Result: nothing happens except for "No URL at point" message
> > 
> > Expected result: the second message is shown/hidden
> 
> Unfortunately this patch is unreliable, reporting BROKEN for some
> environments and FIXED for others; I think you mentioned it depends
> on whether w3m-el is installed? I have reverted it for now to reduce the
> noise in the test logs.

Would a proper fix for this be to explicitly select which html renderer
is used, and then test it with each renderer in turn? If it is the
renderer that is causing the BROKEN/FIXED distinction, this would
trigger all possbile behaviors.

  Tom

Re: [PATCH 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Hi Tom.

On Thu, 15 Dec 2011 10:27:33 -0700, Tom Prince <tom.prince@ualberta.net> wrote:
> On Thu, 15 Dec 2011 08:14:08 -0400, David Bremner <david@tethera.net> wrote:
> > On Mon,  4 Jul 2011 05:59:03 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > > Result: nothing happens except for "No URL at point" message
> > > 
> > > Expected result: the second message is shown/hidden
> > 
> > Unfortunately this patch is unreliable, reporting BROKEN for some
> > environments and FIXED for others; I think you mentioned it depends
> > on whether w3m-el is installed? I have reverted it for now to reduce the
> > noise in the test logs.
> 
> Would a proper fix for this be to explicitly select which html renderer
> is used, and then test it with each renderer in turn? If it is the
> renderer that is causing the BROKEN/FIXED distinction, this would
> trigger all possbile behaviors.
> 

I know that the issue affects w3m-el renderer and does not affect
w3m-standalone.  Though it may affect other renderers as well.  So we
could explicitly set the renderer to w3m-el for the test.  But we need
to make sure we skip the test if w3m-el is not available.

The bug itself is related to keymap property that is set for the
rendered HTML.  It is set for the final newline as well.  And it is
rear-sticky.  I am not sure about how to better fix this.

I do not plan to work on this issue in the near future, so I do not want
to spend time on fixing this test case now.  If you feel like working on
it, you are welcome.

Regards,
  Dmitry

>   Tom

[PATCH v2 0/3] improved broken tests support and test for a bug

[Dmitry Kurochkin]

This version implements suggestions by Austin.

Regards,
  Dmitry

[PATCH v2 1/3] test: update documentation for test_emacs in test/README

[Dmitry Kurochkin]

Update test_emacs documentation in test/README according to the latest
changes in emacs tests.  Move the note regarding setting variables
from test/emacs to test/README.
---
 test/README |   10 +++++++---
 test/emacs  |    5 -----
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/test/README b/test/README
index f9ac607..a245bf1 100644
--- a/test/README
+++ b/test/README
@@ -181,9 +181,13 @@ library for your script to use.
 
    This function executes the provided emacs lisp script within
    emacs. The script can be a sequence of emacs lisp expressions,
-   (that is, they will be evaluated within a progn form). The lisp
-   expressions can call `message' to generate output on stdout to be
-   examined by the calling test script.
+   (that is, they will be evaluated within a progn form). Emacs
+   stdout and stderr is not available, the common way to get output
+   is to save it to a file. There are some auxiliary functions
+   useful in emacs tests provided in test-lib.el. Do not use `setq'
+   for setting variables in Emacs tests because it affects other
+   tests that may run in the same Emacs instance.  Use `let' instead
+   so the scope of the changed variables is limited to a single test.
 
  test_done
 
diff --git a/test/emacs b/test/emacs
index 53f455a..f465e2b 100755
--- a/test/emacs
+++ b/test/emacs
@@ -1,10 +1,5 @@
 #!/usr/bin/env bash
 
-# Note: do not use `setq' for setting variables in Emacs tests because
-# it affects other tests that may run in the same Emacs instance.  Use
-# `let' instead so the scope of the changed variables is limited to a
-# single test.
-
 test_description="emacs interface"
 . test-lib.sh
 
-- 
1.7.5.4

[PATCH v2 2/3] test: improve known broken tests support

[Dmitry Kurochkin]

There is existing support for broken tests.  But it is not convenient
to use.  The primary issue is that we have to maintain a set of
test_expect_*_failure functions which are equivalent to the normal
test_expect_* counterparts except for what functions are called for
result reporting.  The patch adds test_subtest_known_broken function
which marks a subset as broken, making the normal test_expect_*
functions behave as test_expect_*_failure.  All test_expect_*_failure
functions are removed.  Test_known_broken_failure_ is changed to
format details the same way as test_failure_ does.

Another benefit of this change is that the diff when a broken test is
fixed would be small and nice.

Documentation is updated accordingly.
---
 test/README      |   17 ++++++-------
 test/test-lib.sh |   63 +++++++++++++++++------------------------------------
 2 files changed, 28 insertions(+), 52 deletions(-)

diff --git a/test/README b/test/README
index a245bf1..0b54748 100644
--- a/test/README
+++ b/test/README
@@ -132,20 +132,19 @@ library for your script to use.
    <script>.  If it yields success, test is considered
    successful.  <message> should state what it is testing.
 
- test_expect_failure <message> <script>
-
-   This is NOT the opposite of test_expect_success, but is used
-   to mark a test that demonstrates a known breakage.  Unlike
-   the usual test_expect_success tests, which say "ok" on
-   success and "FAIL" on failure, this will say "FIXED" on
-   success and "still broken" on failure.  Failures from these
-   tests won't cause -i (immediate) to stop.
-
  test_begin_subtest <message>
 
    Set the test description message for a subsequent test_expect_equal
    invocation (see below).
 
+ test_subtest_known_broken
+
+   Mark the current test as broken.  Such tests are expected to fail.
+   Unlike the normal tests, which say "PASS" on success and "FAIL" on
+   failure, these will say "FIXED" on success and "BROKEN" on failure.
+   Failures from these tests won't cause -i (immediate) to stop.  A
+   test must call this before any test_expect_* function.
+
  test_expect_equal <output> <expected>
 
    This is an often-used convenience function built on top of
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 22e387e..196ef49 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -424,6 +424,7 @@ test_begin_subtest ()
 	error "bug in test script: Missing test_expect_equal in ${BASH_SOURCE[1]}:${BASH_LINENO[0]}"
     fi
     test_subtest_name="$1"
+    test_subtest_known_broken_=
     # Remember stdout and stderr file descriptors and redirect test
     # output to the previously prepared file descriptors 3 and 4 (see
     # below)
@@ -484,29 +485,6 @@ test_expect_equal_file ()
     fi
 }
 
-test_expect_equal_failure ()
-{
-	exec 1>&6 2>&7		# Restore stdout and stderr
-	inside_subtest=
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test_expect_equal"
-
-	output="$1"
-	expected="$2"
-	if ! test_skip "$@"
-	then
-		if [ "$output" = "$expected" ]; then
-			test_known_broken_ok_ "$test_subtest_name"
-		else
-			test_known_broken_failure_ "$test_subtest_name"
-			testname=$this_test.$test_count
-			echo "$expected" > $testname.expected
-			echo "$output" > $testname.output
-		fi
-    fi
-}
-
 NOTMUCH_NEW ()
 {
     notmuch new | grep -v -E -e '^Processed [0-9]*( total)? file|Found [0-9]* total file'
@@ -568,19 +546,31 @@ test_have_prereq () {
 # the text_expect_* functions instead.
 
 test_ok_ () {
+	if test "$test_subtest_known_broken_" = "t"; then
+		test_known_broken_ok_ "$@"
+		return
+	fi
 	test_success=$(($test_success + 1))
 	say_color pass "%-6s" "PASS"
 	echo " $@"
 }
 
 test_failure_ () {
+	if test "$test_subtest_known_broken_" = "t"; then
+		test_known_broken_failure_ "$@"
+		return
+	fi
 	test_failure=$(($test_failure + 1))
-	say_color error "%-6s" "FAIL"
-	echo " $1"
-	shift
+	test_failure_message_ "FAIL" "$@"
+	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
+}
+
+test_failure_message_ () {
+	say_color error "%-6s" "$1"
+	echo " $2"
+	shift 2
 	echo "$@" | sed -e 's/^/	/'
 	if test "$verbose" != "t"; then cat test.output; fi
-	test "$immediate" = "" || { GIT_EXIT_OK=t; exit 1; }
 }
 
 test_known_broken_ok_ () {
@@ -591,8 +581,7 @@ test_known_broken_ok_ () {
 
 test_known_broken_failure_ () {
 	test_broken=$(($test_broken+1))
-	say_color pass "%-6s" "BROKEN"
-	echo " $@"
+	test_failure_message_ "BROKEN" "$@"
 }
 
 test_debug () {
@@ -636,20 +625,8 @@ test_skip () {
 	esac
 }
 
-test_expect_failure () {
-	test "$#" = 3 && { prereq=$1; shift; } || prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
-	if ! test_skip "$@"
-	then
-		test_run_ "$2"
-		if [ "$?" = 0 -a "$eval_ret" = 0 ]
-		then
-			test_known_broken_ok_ "$1"
-		else
-			test_known_broken_failure_ "$1"
-		fi
-	fi
+test_subtest_known_broken () {
+	test_subtest_known_broken_=t
 }
 
 test_expect_success () {
-- 
1.7.5.4

[PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file

[david]

From: David Bremner <bremner@debian.org>

Commit 4cc6727 introduced the library function
test_subtest_known_broken which sets a variable
test_subtest_known_broken_ . Unfortunately this variable is not reset
if test_begin_subtest is not called before the next
test_expect_success or test_expect_failure.

This commit remedies that, under the assumption that exactly one
test_expect_equal or test_expect_equal_file will follow a
test_begin_subtest
---

Any comments on this? I didn't follow a lot of the original
discussions on the test API very closely. Mainly I want to know if the 
assumption at the end of the commit message seems reasonable.

 test/test-lib.sh |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 196ef49..3c2768c 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -460,6 +460,7 @@ test_expect_equal ()
 			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
 		fi
     fi
+       test_subtest_known_broken_=
 }
 
 test_expect_equal_file ()
@@ -483,6 +484,7 @@ test_expect_equal_file ()
 			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
 		fi
     fi
+	test_subtest_known_broken_=
 }
 
 NOTMUCH_NEW ()
-- 
1.7.5.4

Re: [PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file

[Dmitry Kurochkin]

Hi David.

On Sun, 11 Sep 2011 20:11:54 -0300, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
> 
> Commit 4cc6727 introduced the library function
> test_subtest_known_broken which sets a variable
> test_subtest_known_broken_ . Unfortunately this variable is not reset
> if test_begin_subtest is not called before the next
> test_expect_success or test_expect_failure.
> 
> This commit remedies that, under the assumption that exactly one
> test_expect_equal or test_expect_equal_file will follow a
> test_begin_subtest
> ---
> 
> Any comments on this? I didn't follow a lot of the original
> discussions on the test API very closely. Mainly I want to know if the 
> assumption at the end of the commit message seems reasonable.
> 

IMHO this is not a good idea, because:

1. It introduces multiple places where the flag is reset.  If new
   test_expect_* functions are added in the future, there would be more
   of these.  So it brings us more complex code, code duplication, more
   chances for bugs, etc.

   This may be solved by code refactoring, but I am not sure.

2. No support for tests with multiple test_expect_* calls.  I do not
   know if it is used or works now, but the patch certainly does not
   help in this respect.

3. I thought that every test must start with a test_begin_subtest call.
   So it is the right place to put all subtest initialization code to.
   Is this not correct?  If it is correct, then I do not understand why
   we should support buggy tests by hiding (some of) their bugs.  Why do
   we need it?

Regards,
  Dmitry

>  test/test-lib.sh |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 196ef49..3c2768c 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -460,6 +460,7 @@ test_expect_equal ()
>  			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
>  		fi
>      fi
> +       test_subtest_known_broken_=
>  }
>  
>  test_expect_equal_file ()
> @@ -483,6 +484,7 @@ test_expect_equal_file ()
>  			test_failure_ "$test_subtest_name" "$(diff -u $testname.expected $testname.output)"
>  		fi
>      fi
> +	test_subtest_known_broken_=
>  }
>  
>  NOTMUCH_NEW ()
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file

[David Bremner]

On Mon, 12 Sep 2011 03:30:54 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> Hi David.
> IMHO this is not a good idea, because:
> 
> 1. It introduces multiple places where the flag is reset.  If new
>    test_expect_* functions are added in the future, there would be more
>    of these.  So it brings us more complex code, code duplication, more
>    chances for bugs, etc.

Well, I'm not sure how to solve this without code duplication, since
there is no test_end_subtest. We could make one, but that seems pretty
intrusive.

> 2. No support for tests with multiple test_expect_* calls.  I do not
>    know if it is used or works now, but the patch certainly does not

Looking at the code for test_expect_equal_* (note that these two are
very different than test_expect_success and test_expect_failure), 
several things are reset already, so I don't think it will work even
before my patch to call those routines twice.

> 3. I thought that every test must start with a test_begin_subtest call.
>    So it is the right place to put all subtest initialization code to.
>    Is this not correct?  If it is correct, then I do not understand why
>    we should support buggy tests by hiding (some of) their bugs.  Why do
>    we need it?

I could not find anything in the docs (or test-lib.sh) that says
test_begin_subtest must be called before test_expect_success or
test_expect_failure. Indeed if test_begin_subtest is called first, the
extra message argument to those two does not really make sense.

What brought this to my attention is the atomicity test introduced by
amdragon, which does exactly

test_begin_subtest
test_expect_equal_failure
test_expect_success

d

Re: [PATCH] test: reset known_broken status in test_expect_equal and test_expect_equal_file

[Dmitry Kurochkin]

On Sun, 11 Sep 2011 20:51:47 -0300, David Bremner <david@tethera.net> wrote:
> On Mon, 12 Sep 2011 03:30:54 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > Hi David.
> > IMHO this is not a good idea, because:
> > 
> > 1. It introduces multiple places where the flag is reset.  If new
> >    test_expect_* functions are added in the future, there would be more
> >    of these.  So it brings us more complex code, code duplication, more
> >    chances for bugs, etc.
> 
> Well, I'm not sure how to solve this without code duplication, since
> there is no test_end_subtest. We could make one, but that seems pretty
> intrusive.
> 

I agree that introducing test_end_subtest for the flag reset is an
overkill.

> > 2. No support for tests with multiple test_expect_* calls.  I do not
> >    know if it is used or works now, but the patch certainly does not
> 
> Looking at the code for test_expect_equal_* (note that these two are
> very different than test_expect_success and test_expect_failure), 
> several things are reset already, so I don't think it will work even
> before my patch to call those routines twice.
> 
> > 3. I thought that every test must start with a test_begin_subtest call.
> >    So it is the right place to put all subtest initialization code to.
> >    Is this not correct?  If it is correct, then I do not understand why
> >    we should support buggy tests by hiding (some of) their bugs.  Why do
> >    we need it?
> 
> I could not find anything in the docs (or test-lib.sh) that says
> test_begin_subtest must be called before test_expect_success or
> test_expect_failure. Indeed if test_begin_subtest is called first, the
> extra message argument to those two does not really make sense.
> 
> What brought this to my attention is the atomicity test introduced by
> amdragon, which does exactly
> 
> test_begin_subtest
> test_expect_equal_failure
> test_expect_success
> 

So it seems we have 2 types of tests.  Those who start with
test_begin_subtest() and end with test_expect_equal_*() call.  And those
who are implemented as a single test_expect_[success|code]() call
(test-lib.sh mentions test_expect_failure(), but apparently it is not
available).  I see several options, starting with the simplest:

1. Support broken tests only for the first type of tests.
   Test_begin_subtest() sets inside_subtest variable.  We can check for
   broken tests only when it is set (currently it is cleared to early,
   but that is easy to fix).

2. Single-command tests use test_run_() function internally to run the
   command.  We can reset broken flag in the beginning of it.  That way
   test_subtest_known_broken() should work fine for both types of tests.

3. Remove the single-command tests, and just stick with
   test_begin_subtest() and friends.

The last one may be invasive and perhaps others have some good reasons
against it.  But I like it the most because it would make the test
system simpler and more consistent.

Point 2 should be pretty easy to implement (1 line if I do not miss
anything) and it should work fine.  So I would go for it.


Hmm... looks like there is one more way to run tests - test_external()
function.  It does not use test_run_(), so it another place where we
need to reset the flag.  Instead of resetting the flag in 3 different
places, we should have a function like test_init_subtest_() which does
all subtest-related initialization.

Regards,
  Dmitry

> d

[PATCH] test: reset test_subtest_known_broken_ after each success/failure.

[david]

From: David Bremner <bremner@debian.org>

This means that test_subtest_known_broken needs to be called before
every known broken subtest, which is no different than what is
documented for the test_begin_subtest case.

The assumption is that every test ends up calling either skipping,
calling test_ok_ or test_failure_ and and the latter in turn delegate
to the known_broken versions in the case where
test_subtest_known_broken_ is set.
---
 test/test-lib.sh |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index 196ef49..966b2dc 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -574,12 +574,14 @@ test_failure_message_ () {
 }
 
 test_known_broken_ok_ () {
+	test_subtest_known_broken_=
 	test_fixed=$(($test_fixed+1))
 	say_color pass "%-6s" "FIXED"
 	echo " $@"
 }
 
 test_known_broken_failure_ () {
+	test_subtest_known_broken_=
 	test_broken=$(($test_broken+1))
 	test_failure_message_ "BROKEN" "$@"
 }
@@ -614,6 +616,7 @@ test_skip () {
 	fi
 	case "$to_skip" in
 	t)
+		test_subtest_known_broken_=
 		say_color skip >&3 "skipping test: $@"
 		say_color skip "%-6s" "SKIP"
 		echo " $1"
-- 
1.7.5.4

Re: [PATCH] test: reset test_subtest_known_broken_ after each success/failure.

[Dmitry Kurochkin]

On Mon, 12 Sep 2011 23:41:54 -0300, david@tethera.net wrote:
> From: David Bremner <bremner@debian.org>
> 
> This means that test_subtest_known_broken needs to be called before
> every known broken subtest, which is no different than what is
> documented for the test_begin_subtest case.
> 
> The assumption is that every test ends up calling either skipping,
> calling test_ok_ or test_failure_ and and the latter in turn delegate
> to the known_broken versions in the case where
> test_subtest_known_broken_ is set.

Looks good to me.

Regards,
  Dmitry

> ---
>  test/test-lib.sh |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 196ef49..966b2dc 100755
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -574,12 +574,14 @@ test_failure_message_ () {
>  }
>  
>  test_known_broken_ok_ () {
> +	test_subtest_known_broken_=
>  	test_fixed=$(($test_fixed+1))
>  	say_color pass "%-6s" "FIXED"
>  	echo " $@"
>  }
>  
>  test_known_broken_failure_ () {
> +	test_subtest_known_broken_=
>  	test_broken=$(($test_broken+1))
>  	test_failure_message_ "BROKEN" "$@"
>  }
> @@ -614,6 +616,7 @@ test_skip () {
>  	fi
>  	case "$to_skip" in
>  	t)
> +		test_subtest_known_broken_=
>  		say_color skip >&3 "skipping test: $@"
>  		say_color skip "%-6s" "SKIP"
>  		echo " $1"
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test: reset test_subtest_known_broken_ after each success/failure.

[David Bremner]

On Tue, 13 Sep 2011 14:19:36 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> > The assumption is that every test ends up calling either skipping,
> > calling test_ok_ or test_failure_ and and the latter in turn delegate
> > to the known_broken versions in the case where
> > test_subtest_known_broken_ is set.
> 
> Looks good to me.
> 
> Regards,
>   Dmitry

OK, pushed.

[PATCH v2 3/3] test: add emacs test for hiding a message following an HTML part

[Dmitry Kurochkin]

Human-friendly scenario:

* open a thread where a message which ends with an HTML part is
  followed by another message

* make the first message visible

* goto the beginning of the second message (first line, first colon)

* hit "RET"

Result: nothing happens except for "No URL at point" message

Expected result: the second message is shown/hidden

The root cause is that the HTML part has `keymap' text property set.
In particular, "RET" is bound to visit a URL at point.  The problem is
that `keymap' property affects the next character following the region
it is set to (see elisp manual [1]).  Hence, the first character of
the next message has a keymap inherited from the HTML part.

[1] http://www.gnu.org/software/emacs/elisp/html_node/Special-Properties.html
---
 test/emacs |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/test/emacs b/test/emacs
index f465e2b..8b627c7 100755
--- a/test/emacs
+++ b/test/emacs
@@ -342,4 +342,30 @@ test_emacs '(notmuch-show "id:f35dbb950911171438k5df6eb56k77b6c0944e2e79ae@mail.
 	    (test-visible-output)'
 test_expect_equal_file OUTPUT $EXPECTED/notmuch-show-thread-with-hidden-messages
 
+test_begin_subtest 'Hiding message following HTML part'
+test_subtest_known_broken
+id='html-message@notmuchmail.org'
+emacs_deliver_message \
+    'HTML message' \
+    '' \
+    "(message-goto-eoh)
+     (insert \"Message-ID: <$id>\n\")
+     (message-goto-body)
+     (mml-insert-part \"text/html\")
+     (insert \"<body>This is a test HTML message</body>\")"
+emacs_deliver_message \
+    'Reply to HTML message' \
+    'This is a reply to the test HTML message' \
+    "(message-goto-eoh)
+     (insert \"In-Reply-To: <$id>\n\")"
+test_emacs "(notmuch-show \"id:$id\") \
+            (notmuch-show-next-message) \
+            (command-execute (key-binding (kbd \"RET\"))) \
+            (test-visible-output)"
+test_emacs "(notmuch-show \"id:$id\") \
+            (notmuch-show-next-message) \
+            (notmuch-show-toggle-message) \
+            (test-visible-output \"EXPECTED\")"
+test_expect_equal_file OUTPUT EXPECTED
+
 test_done
-- 
1.7.5.4

Re: [PATCH v2 0/3] improved broken tests support and test for a bug

[David Bremner]

On Mon,  4 Jul 2011 08:07:18 +0400, Dmitry Kurochkin <dmitry.kurochkin@gmail.com> wrote:
> This version implements suggestions by Austin.
> 
> Regards,
>   Dmitry

I have pushed this series.