[PATCH 0/3] test fixes, portability

[PATCH 0/3] test fixes, portability

[Jani Nikula]

I had a glance at a 'make test' report on OS X [1], and tried to fix a
few things from there. While at it, noticed that count test is horribly
broken.

BR,
Jani.

[1] https://spod.gy/p/u9j3zxg96vu6k3wy

Jani Nikula (3):
  test: fix count test
  test: wrap 'wc -l' results in arithmetic evaluation to strip
    whitespace
  test: use perl instead of sed -r for portability

 test/count       |   25 +++++++++++++------------
 test/test-lib.sh |    2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] test: fix count test

[Jani Nikula]

The quoting for ${SEARCH} is broken when it's supposed to be '*', and
it seems tricky to get it right. Just drop the variable and use '*'
directly. Before this, none of the messages ever matched, and the test
was comparing zeros.
---
 test/count |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/test/count b/test/count
index 300b171..8e587ff 100755
--- a/test/count
+++ b/test/count
@@ -4,37 +4,34 @@ test_description='"notmuch count" for messages and threads'
 
 add_email_corpus
 
-SEARCH="\"*\""
-
 test_begin_subtest "message count is the default for notmuch count"
 test_expect_equal \
-    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
-    "`notmuch count ${SEARCH}`"
+    "`notmuch search --output=messages '*' | wc -l`" \
+    "`notmuch count '*'`"
 
 test_begin_subtest "message count with --output=messages"
 test_expect_equal \
-    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
-    "`notmuch count --output=messages ${SEARCH}`"
+    "`notmuch search --output=messages '*' | wc -l`" \
+    "`notmuch count --output=messages '*'`"
 
 test_begin_subtest "thread count with --output=threads"
 test_expect_equal \
-    "`notmuch search --output=threads ${SEARCH} | wc -l`" \
-    "`notmuch count --output=threads ${SEARCH}`"
+    "`notmuch search --output=threads '*' | wc -l`" \
+    "`notmuch count --output=threads '*'`"
 
 test_begin_subtest "thread count is the default for notmuch search"
 test_expect_equal \
-    "`notmuch search ${SEARCH} | wc -l`" \
-    "`notmuch count --output=threads ${SEARCH}`"
+    "`notmuch search '*' | wc -l`" \
+    "`notmuch count --output=threads '*'`"
 
-SEARCH="from:cworth and not from:cworth"
 test_begin_subtest "count with no matching messages"
 test_expect_equal \
     "0" \
-    "`notmuch count --output=messages ${SEARCH}`"
+    "`notmuch count --output=messages from:cworth and not from:cworth`"
 
 test_begin_subtest "count with no matching threads"
 test_expect_equal \
     "0" \
-    "`notmuch count --output=threads ${SEARCH}`"
+    "`notmuch count --output=threads from:cworth and not from:cworth`"
 
 test_done
-- 
1.7.10.4

Re: [PATCH 1/3] test: fix count test

[Michal Nazarewicz]

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

On Tue, Dec 04 2012, Jani Nikula wrote:
> The quoting for ${SEARCH} is broken when it's supposed to be '*', and

Why is it broken?  It does not appear to be broken to me and in fact the
test passes.

> it seems tricky to get it right. Just drop the variable and use '*'
> directly. Before this, none of the messages ever matched, and the test
> was comparing zeros.
> ---
>  test/count |   23 ++++++++++-------------
>  1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/test/count b/test/count
> index 300b171..8e587ff 100755
> --- a/test/count
> +++ b/test/count
> @@ -4,37 +4,34 @@ test_description='"notmuch count" for messages and threads'
>  
>  add_email_corpus
>  
> -SEARCH="\"*\""
> -
>  test_begin_subtest "message count is the default for notmuch count"
>  test_expect_equal \
> -    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
> -    "`notmuch count ${SEARCH}`"
> +    "`notmuch search --output=messages '*' | wc -l`" \
> +    "`notmuch count '*'`"
>  
>  test_begin_subtest "message count with --output=messages"
>  test_expect_equal \
> -    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
> -    "`notmuch count --output=messages ${SEARCH}`"
> +    "`notmuch search --output=messages '*' | wc -l`" \
> +    "`notmuch count --output=messages '*'`"
>  
>  test_begin_subtest "thread count with --output=threads"
>  test_expect_equal \
> -    "`notmuch search --output=threads ${SEARCH} | wc -l`" \
> -    "`notmuch count --output=threads ${SEARCH}`"
> +    "`notmuch search --output=threads '*' | wc -l`" \
> +    "`notmuch count --output=threads '*'`"
>  
>  test_begin_subtest "thread count is the default for notmuch search"
>  test_expect_equal \
> -    "`notmuch search ${SEARCH} | wc -l`" \
> -    "`notmuch count --output=threads ${SEARCH}`"
> +    "`notmuch search '*' | wc -l`" \
> +    "`notmuch count --output=threads '*'`"
>  
> -SEARCH="from:cworth and not from:cworth"
>  test_begin_subtest "count with no matching messages"
>  test_expect_equal \
>      "0" \
> -    "`notmuch count --output=messages ${SEARCH}`"
> +    "`notmuch count --output=messages from:cworth and not from:cworth`"
>  
>  test_begin_subtest "count with no matching threads"
>  test_expect_equal \
>      "0" \
> -    "`notmuch count --output=threads ${SEARCH}`"
> +    "`notmuch count --output=threads from:cworth and not from:cworth`"
>  
>  test_done

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



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

Re: [PATCH 1/3] test: fix count test

[Jani Nikula]

On Wed, 05 Dec 2012, Michal Nazarewicz <mina86@mina86.com> wrote:
> On Tue, Dec 04 2012, Jani Nikula wrote:
>> The quoting for ${SEARCH} is broken when it's supposed to be '*', and
>
> Why is it broken?  It does not appear to be broken to me and in fact the
> test passes.

This is exactly why nobody noticed before. It looks all right and the
test passes...

>> it seems tricky to get it right. Just drop the variable and use '*'
>> directly. Before this, none of the messages ever matched, and the test
>> was comparing zeros.

...but the query does not find anything, and the test compares zero
results to zero results. Please try this patch, which should pass if
everything were all right:

diff --git a/test/count b/test/count
index 300b171..ecae40e 100755
--- a/test/count
+++ b/test/count
@@ -6,6 +6,11 @@ add_email_corpus
 
 SEARCH="\"*\""
 
+test_begin_subtest "check the query"
+test_expect_equal \
+    "`notmuch count ${SEARCH}`" \
+    "`notmuch count '*'`"
+
 test_begin_subtest "message count is the default for notmuch count"
 test_expect_equal \
     "`notmuch search --output=messages ${SEARCH} | wc -l`" \

-- 

At least for me this produces:

 FAIL   check the query
 --- count.1.expected   2012-12-05 08:01:09.004751327 +0000
 +++ count.1.output     2012-12-05 08:01:09.004751327 +0000
 @@ -1 +1 @@
 -52
 +0

And because it seems to be really hard to get the quoting right, and
keep it that way, I think it's more robust just to s/${SEARCH}/'*'/.


BR,
Jani.


>> ---
>>  test/count |   23 ++++++++++-------------
>>  1 file changed, 10 insertions(+), 13 deletions(-)
>>
>> diff --git a/test/count b/test/count
>> index 300b171..8e587ff 100755
>> --- a/test/count
>> +++ b/test/count
>> @@ -4,37 +4,34 @@ test_description='"notmuch count" for messages and threads'
>>  
>>  add_email_corpus
>>  
>> -SEARCH="\"*\""
>> -
>>  test_begin_subtest "message count is the default for notmuch count"
>>  test_expect_equal \
>> -    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
>> -    "`notmuch count ${SEARCH}`"
>> +    "`notmuch search --output=messages '*' | wc -l`" \
>> +    "`notmuch count '*'`"
>>  
>>  test_begin_subtest "message count with --output=messages"
>>  test_expect_equal \
>> -    "`notmuch search --output=messages ${SEARCH} | wc -l`" \
>> -    "`notmuch count --output=messages ${SEARCH}`"
>> +    "`notmuch search --output=messages '*' | wc -l`" \
>> +    "`notmuch count --output=messages '*'`"
>>  
>>  test_begin_subtest "thread count with --output=threads"
>>  test_expect_equal \
>> -    "`notmuch search --output=threads ${SEARCH} | wc -l`" \
>> -    "`notmuch count --output=threads ${SEARCH}`"
>> +    "`notmuch search --output=threads '*' | wc -l`" \
>> +    "`notmuch count --output=threads '*'`"
>>  
>>  test_begin_subtest "thread count is the default for notmuch search"
>>  test_expect_equal \
>> -    "`notmuch search ${SEARCH} | wc -l`" \
>> -    "`notmuch count --output=threads ${SEARCH}`"
>> +    "`notmuch search '*' | wc -l`" \
>> +    "`notmuch count --output=threads '*'`"
>>  
>> -SEARCH="from:cworth and not from:cworth"
>>  test_begin_subtest "count with no matching messages"
>>  test_expect_equal \
>>      "0" \
>> -    "`notmuch count --output=messages ${SEARCH}`"
>> +    "`notmuch count --output=messages from:cworth and not from:cworth`"
>>  
>>  test_begin_subtest "count with no matching threads"
>>  test_expect_equal \
>>      "0" \
>> -    "`notmuch count --output=threads ${SEARCH}`"
>> +    "`notmuch count --output=threads from:cworth and not from:cworth`"
>>  
>>  test_done
>
> -- 
> Best regards,                                         _     _
> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

Re: [PATCH 1/3] test: fix count test

[Michal Nazarewicz]

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

Hmm, I thought I've already replied to this email, but it still sticks
as unread.  In case I did, sorry for duplicate.

On Wed, Dec 05 2012, Jani Nikula wrote:
> Please try this patch, which should pass if everything were all right:
>
> diff --git a/test/count b/test/count
> index 300b171..ecae40e 100755
> --- a/test/count
> +++ b/test/count
> @@ -6,6 +6,11 @@ add_email_corpus
>  
>  SEARCH="\"*\""
>  
> +test_begin_subtest "check the query"
> +test_expect_equal \
> +    "`notmuch count ${SEARCH}`" \
> +    "`notmuch count '*'`"
> +
>  test_begin_subtest "message count is the default for notmuch count"
>  test_expect_equal \
>      "`notmuch search --output=messages ${SEARCH} | wc -l`" \

Yeah, you're right, sorry for the commotion.

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



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

Re: [PATCH 1/3] test: fix count test

[Jani Nikula]

On Wed, 05 Dec 2012, Michal Nazarewicz <mina86@mina86.com> wrote:
> Hmm, I thought I've already replied to this email, but it still sticks
> as unread.  In case I did, sorry for duplicate.
>
> On Wed, Dec 05 2012, Jani Nikula wrote:
>> Please try this patch, which should pass if everything were all right:
>>
>> diff --git a/test/count b/test/count
>> index 300b171..ecae40e 100755
>> --- a/test/count
>> +++ b/test/count
>> @@ -6,6 +6,11 @@ add_email_corpus
>>  
>>  SEARCH="\"*\""
>>  
>> +test_begin_subtest "check the query"
>> +test_expect_equal \
>> +    "`notmuch count ${SEARCH}`" \
>> +    "`notmuch count '*'`"
>> +
>>  test_begin_subtest "message count is the default for notmuch count"
>>  test_expect_equal \
>>      "`notmuch search --output=messages ${SEARCH} | wc -l`" \
>
> Yeah, you're right, sorry for the commotion.

No worries. I wrote the test originally, and I thought it was all right
too...

Jani.

Re: [PATCH 1/3] test: fix count test

[Michal Nazarewicz]

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

On Wed, Dec 05 2012, Jani Nikula <jani@nikula.org> wrote:
> diff --git a/test/count b/test/count
> index 300b171..ecae40e 100755
> --- a/test/count
> +++ b/test/count
> @@ -6,6 +6,11 @@ add_email_corpus
>  
>  SEARCH="\"*\""
>  
> +test_begin_subtest "check the query"
> +test_expect_equal \
> +    "`notmuch count ${SEARCH}`" \
> +    "`notmuch count '*'`"
> +

Maybe it would actually be beneficial to add something along the lines of:

test_begin_subtest "check the query"
test_expect_not_equal \
    "`notmuch count '*'`" \
    0

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



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

[PATCH 2/3] test: wrap 'wc -l' results in arithmetic evaluation to strip whitespace

[Jani Nikula]

This is for portability, as 'wc -l' emits whitespace on some BSD
variants. Suggested by Tomi Ollila <tomi.ollila@iki.fi>.

---

Updated version of id:1338361324-57289-8-git-send-email-pioto@pioto.org
---
 test/count |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/test/count b/test/count
index 8e587ff..879b114 100755
--- a/test/count
+++ b/test/count
@@ -4,24 +4,28 @@ test_description='"notmuch count" for messages and threads'
 
 add_email_corpus
 
+# Note: The 'wc -l' results below are wrapped in arithmetic evaluation
+# $((...)) to strip whitespace. This is for portability, as 'wc -l'
+# emits whitespace on some BSD variants.
+
 test_begin_subtest "message count is the default for notmuch count"
 test_expect_equal \
-    "`notmuch search --output=messages '*' | wc -l`" \
+    "$((`notmuch search --output=messages '*' | wc -l`))" \
     "`notmuch count '*'`"
 
 test_begin_subtest "message count with --output=messages"
 test_expect_equal \
-    "`notmuch search --output=messages '*' | wc -l`" \
+    "$((`notmuch search --output=messages '*' | wc -l`))" \
     "`notmuch count --output=messages '*'`"
 
 test_begin_subtest "thread count with --output=threads"
 test_expect_equal \
-    "`notmuch search --output=threads '*' | wc -l`" \
+    "$((`notmuch search --output=threads '*' | wc -l`))" \
     "`notmuch count --output=threads '*'`"
 
 test_begin_subtest "thread count is the default for notmuch search"
 test_expect_equal \
-    "`notmuch search '*' | wc -l`" \
+    "$((`notmuch search '*' | wc -l`))" \
     "`notmuch count --output=threads '*'`"
 
 test_begin_subtest "count with no matching messages"
-- 
1.7.10.4

[PATCH 3/3] test: use perl instead of sed -r for portability

[Jani Nikula]

Our OS X users report -r is not a supported option for sed. Use perl
instead.
---
 test/test-lib.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f169785..31ed107 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -576,7 +576,7 @@ NOTMUCH_NEW ()
 
 notmuch_search_sanitize ()
 {
-    sed -r -e 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
+    perl -pe 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
 }
 
 NOTMUCH_SHOW_FILENAME_SQUELCH='s,filename:.*/mail,filename:/XXX/mail,'
-- 
1.7.10.4

Re: [PATCH 3/3] test: use perl instead of sed -r for portability

[Michal Nazarewicz]

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

On Tue, Dec 04 2012, Jani Nikula wrote:
> Our OS X users report -r is not a supported option for sed. Use perl
> instead.
> ---
>  test/test-lib.sh |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index f169785..31ed107 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -576,7 +576,7 @@ NOTMUCH_NEW ()
>  
>  notmuch_search_sanitize ()
>  {
> -    sed -r -e 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
> +    perl -pe 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'

Alternatively, this could just be convert into a basic regexp:

sed -e 's/\("\?thread"\?: \?"\?\)................\("\?\)/\1XXX\2/'

which I think is even more portable, no?

>  }
>  
>  NOTMUCH_SHOW_FILENAME_SQUELCH='s,filename:.*/mail,filename:/XXX/mail,'

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

[-- Attachment #2.1: Type: text/plain, Size: 0 bytes --]



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

Re: [PATCH 3/3] test: use perl instead of sed -r for portability

[Jani Nikula]

On Wed, 05 Dec 2012, Michal Nazarewicz <mina86@mina86.com> wrote:
> On Tue, Dec 04 2012, Jani Nikula wrote:
>> Our OS X users report -r is not a supported option for sed. Use perl
>> instead.
>> ---
>>  test/test-lib.sh |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/test/test-lib.sh b/test/test-lib.sh
>> index f169785..31ed107 100644
>> --- a/test/test-lib.sh
>> +++ b/test/test-lib.sh
>> @@ -576,7 +576,7 @@ NOTMUCH_NEW ()
>>  
>>  notmuch_search_sanitize ()
>>  {
>> -    sed -r -e 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
>> +    perl -pe 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
>
> Alternatively, this could just be convert into a basic regexp:
>
> sed -e 's/\("\?thread"\?: \?"\?\)................\("\?\)/\1XXX\2/'
>
> which I think is even more portable, no?

I don't know.

Jani.

>
>>  }
>>  
>>  NOTMUCH_SHOW_FILENAME_SQUELCH='s,filename:.*/mail,filename:/XXX/mail,'
>
> -- 
> Best regards,                                         _     _
> .o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
> ..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
> ooo +----<email/xmpp: mpn@google.com>--------------ooO--(_)--Ooo--

Re: [PATCH 0/3] test fixes, portability

[Tomi Ollila]

On Tue, Dec 04 2012, Jani Nikula <jani@nikula.org> wrote:

> I had a glance at a 'make test' report on OS X [1], and tried to fix a
> few things from there. While at it, noticed that count test is horribly
> broken.

LGTM

>
> BR,
> Jani.

Tomi

>
> [1] https://spod.gy/p/u9j3zxg96vu6k3wy
>
> Jani Nikula (3):
>   test: fix count test
>   test: wrap 'wc -l' results in arithmetic evaluation to strip
>     whitespace
>   test: use perl instead of sed -r for portability
>
>  test/count       |   25 +++++++++++++------------
>  test/test-lib.sh |    2 +-
>  2 files changed, 14 insertions(+), 13 deletions(-)
>
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/3] test fixes, portability

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> I had a glance at a 'make test' report on OS X [1], and tried to fix a
> few things from there. While at it, noticed that count test is horribly
> broken.

pushed,

d