[PATCH 0/7] Travis and test suite improvements

[PATCH 0/7] Travis and test suite improvements

[Vladimir Panteleev]

Test more things better on Travis CI, and fix things in the test suite
that did not run properly on Travis CI.

I've enabled Travis on my GitHub fork, so you can see what the results
would look like here:
https://travis-ci.org/CyberShadow/notmuch

Vladimir Panteleev (7):
  .travis.yml: Invoke `make download-test-databases' before running
    tests
  .travis.yml: Add gdb and gpgsm dependencies
  .travis.yml: Replace manual zlib installation with "dist: trusty"
  test: Fix T050-new.sh on some Travis CI machines
  .travis.yml: Switch to "sudo: false" for faster builds
  test: Fix T350-crypto.sh on Emacs snapshots
  .travis.yml: Test against several Emacs versions

 .travis.yml         | 37 ++++++++++++++++++++++++++-----------
 test/T050-new.sh    |  2 +-
 test/T350-crypto.sh |  1 +
 3 files changed, 28 insertions(+), 12 deletions(-)

-- 
2.13.3

[PATCH 1/7] .travis.yml: Invoke `make download-test-databases' before running tests

[Vladimir Panteleev]

Download the test message database used for the T530-upgrade.sh test.

If the additional load on the web server is undesired, Travis can be
instructed to cache the file.
---
 .travis.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.travis.yml b/.travis.yml
index 6bd397ef..d1aa68b3 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -14,6 +14,7 @@ before_install:
 
 script:
   - ./configure
+  - make download-test-databases
   - make test
 
 # notifications:
-- 
2.13.3

[PATCH 2/7] .travis.yml: Add gdb and gpgsm dependencies

[Vladimir Panteleev]

This allows running more tests which were previously skipped due to
missing dependencies.
---
 .travis.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.travis.yml b/.travis.yml
index d1aa68b3..505c4e22 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,7 +1,7 @@
 language: c
 before_install:
   - sudo apt-get update -qq
-  - sudo apt-get install dtach libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx
+  - sudo apt-get install dtach libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx gdb gpgsm
 
   # Notmuch requires zlib 1.2.5.2, unfortunately travis runs on Ubuntu 12.04LTS which
   # ships with zlib 1.2.3.3. We need to update to zlib 1.2.5.2 to be able to build.
-- 
2.13.3

[PATCH 3/7] .travis.yml: Replace manual zlib installation with "dist: trusty"

[Vladimir Panteleev]

Travis now offers Ubuntu Trusty (14.04 LTS) VMs as test runners, which
is gradually becoming the default. We can opt in to using Trusty now
so that we no longer need to manually update zlib to a newer version.
---
 .travis.yml | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index 505c4e22..e297e516 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,17 +1,12 @@
 language: c
+
+dist: trusty
+sudo: required
+
 before_install:
   - sudo apt-get update -qq
   - sudo apt-get install dtach libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx gdb gpgsm
 
-  # Notmuch requires zlib 1.2.5.2, unfortunately travis runs on Ubuntu 12.04LTS which
-  # ships with zlib 1.2.3.3. We need to update to zlib 1.2.5.2 to be able to build.
-  # TODO: Watch https://github.com/travis-ci/travis-ci/issues/2046 and remove
-  #       this hack once travis-ci switches to Ubuntu 14.04
-  - wget 'https://github.com/notmuch/travis-files/raw/master/zlib1g-dev_1.2.8.dfsg-1ubuntu1_amd64.deb'
-  - wget 'https://github.com/notmuch/travis-files/raw/master/zlib1g_1.2.8.dfsg-1ubuntu1_amd64.deb'
-  - sudo dpkg -i zlib1g-dev_1.2.8.dfsg-1ubuntu1_amd64.deb zlib1g_1.2.8.dfsg-1ubuntu1_amd64.deb
-  - sudo apt-get install -f
-
 script:
   - ./configure
   - make download-test-databases
-- 
2.13.3

[PATCH 4/7] test: Fix T050-new.sh on some Travis CI machines

[Vladimir Panteleev]

On some system configurations, setting a breakpoint on the "add_file"
function then issuing "continue" in gdb causes the debugger to
seemingly jump over the add_file invocation. This results in a test
failure, as the "Handle files vanishing between scandir and add_file"
subtest expects add_file to be called and fail due to the vanishing
file. The compiler optimization level also plays a role - the problem
can be reproduced with CFLAGS having -O2 but not -Og.

This problem was observed manifesting as a test failure on Travis CI
configured with "dist: trusty" and "sudo: false". It was not
reproducible on a local Docker image of Travis' runtime environment,
so Travis' virtualization infrastructure likely plays a role as well.

* T050-new.sh: Breakpoint notmuch_database_add_message instead of
  add_file to the same effect, and avoid bad gdb behaviour on Travis
  CI.
---
 test/T050-new.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index ffa303ef..8b7ac4e1 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -309,7 +309,7 @@ cat <<EOF > notmuch-new-vanish.gdb
 set breakpoint pending on
 set logging file notmuch-new-vanish-gdb.log
 set logging on
-break add_file
+break notmuch_database_add_message
 commands
 shell rm -f ${MAIL_DIR}/vanish
 continue
-- 
2.13.3

[PATCH 5/7] .travis.yml: Switch to "sudo: false" for faster builds

[Vladimir Panteleev]

Builds not requiring sudo access run in a container, which will have
better performance and less overhead on the Travis infrastructure.

Use the apt addon to install dependencies instead of explicit apt-get
commands.
---
 .travis.yml | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index e297e516..8b71568d 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,11 +1,18 @@
 language: c
 
 dist: trusty
-sudo: required
+sudo: false
 
-before_install:
-  - sudo apt-get update -qq
-  - sudo apt-get install dtach libxapian-dev libgmime-2.6-dev libtalloc-dev python-sphinx gdb gpgsm
+addons:
+  apt:
+    packages:
+    - dtach
+    - libxapian-dev
+    - libgmime-2.6-dev
+    - libtalloc-dev
+    - python-sphinx
+    - gdb
+    - gpgsm
 
 script:
   - ./configure
-- 
2.13.3

[PATCH 6/7] test: Fix T350-crypto.sh on Emacs snapshots

[Vladimir Panteleev]

The "reply to encrypted message" subtest of T350-crypto.sh was failing
with Emacs snapshots because it assumed the "In-Reply-To" and
"References" headers it attempts to filter out would never be
line-wrapped. This apparently changed in the latest Emacs versions,
which causes the test to fail when ran against them.

* T350-crypto.sh: Unwrap headers before filtering them.
---
 test/T350-crypto.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 7dab39a2..69d9b160 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -394,6 +394,7 @@ test_expect_equal_json \
 
 test_begin_subtest "reply to encrypted message"
 output=$(notmuch reply --decrypt subject:"test encrypted message 002" \
+    | tr '\n' '|' | sed 's/|[[:space:]][[:space:]]*/ /g' | tr '|' '\n' \
     | grep -v -e '^In-Reply-To:' -e '^References:')
 expected='From: Notmuch Test Suite <test_suite@notmuchmail.org>
 Subject: Re: test encrypted message 002
-- 
2.13.3

[PATCH] test/crypto: remove headers more robustly

[David Bremner]

In [1], Vladimir Panteleev observed that the In-Reply-To and
References headers could be wrapped in the 'default' output format of
notmuch-reply, depending on the version of Emacs creating the
message. In my own experiments notmuch-reply sometimes wraps headers
with only one message-id if that message-id is long enough. However it
happens, this causes the previous approach using grep to fail.

Since I found the proposed unwrapping shell fragment in [1] a bit hard
to follow, I decided to write a little python script instead.

[1] id:20170817175145.3204-7-notmuch@thecybershadow.net
---
 test/T350-crypto.sh |  2 +-
 test/test-lib.sh    | 11 +++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
index 7dab39a2..fd950952 100755
--- a/test/T350-crypto.sh
+++ b/test/T350-crypto.sh
@@ -394,7 +394,7 @@ test_expect_equal_json \
 
 test_begin_subtest "reply to encrypted message"
 output=$(notmuch reply --decrypt subject:"test encrypted message 002" \
-    | grep -v -e '^In-Reply-To:' -e '^References:')
+    | notmuch_reply_sanitize_refs)
 expected='From: Notmuch Test Suite <test_suite@notmuchmail.org>
 Subject: Re: test encrypted message 002
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index d2b2a47f..4716f649 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -507,6 +507,17 @@ NOTMUCH_DUMP_TAGS ()
     notmuch dump --include=tags "${@}" | sed '/^#/d' | sort
 }
 
+notmuch_reply_sanitize_refs ()
+{
+    $NOTMUCH_PYTHON -c "
+import email,sys
+msg=email.message_from_file(sys.stdin)
+del msg['in-reply-to']
+del msg['references']
+print(msg.as_string(False))
+"
+}
+
 notmuch_search_sanitize ()
 {
     perl -pe 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
-- 
2.14.1

Re: [PATCH] test/crypto: remove headers more robustly

[Tomi Ollila]

On Fri, Sep 01 2017, David Bremner wrote:

> In [1], Vladimir Panteleev observed that the In-Reply-To and
> References headers could be wrapped in the 'default' output format of
> notmuch-reply, depending on the version of Emacs creating the
> message. In my own experiments notmuch-reply sometimes wraps headers
> with only one message-id if that message-id is long enough. However it
> happens, this causes the previous approach using grep to fail.

we could (also) make emacs think it has wider than 80 characters to fit on
one line...

> Since I found the proposed unwrapping shell fragment in [1] a bit hard
> to follow, I decided to write a little python script instead.

There was nothing hard in that shell construct ;), but I also thought some
alternative solutions (one in awk and one in perl)

How 'bout

drop_email_headers ()
{
    $NOTMUCH_PYTHON -c 'import email, sys
msg = email.message_from_file(sys.stdin)
for hdr in sys.argv: msg.pop(hdr, None)
print(msg.as_string(False))'
}

and then ... | drop_email_headers In-Reply-To References




>
> [1] id:20170817175145.3204-7-notmuch@thecybershadow.net
> ---
>  test/T350-crypto.sh |  2 +-
>  test/test-lib.sh    | 11 +++++++++++
>  2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/test/T350-crypto.sh b/test/T350-crypto.sh
> index 7dab39a2..fd950952 100755
> --- a/test/T350-crypto.sh
> +++ b/test/T350-crypto.sh
> @@ -394,7 +394,7 @@ test_expect_equal_json \
>  
>  test_begin_subtest "reply to encrypted message"
>  output=$(notmuch reply --decrypt subject:"test encrypted message 002" \
> -    | grep -v -e '^In-Reply-To:' -e '^References:')
> +    | notmuch_reply_sanitize_refs)
>  expected='From: Notmuch Test Suite <test_suite@notmuchmail.org>
>  Subject: Re: test encrypted message 002
>  
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index d2b2a47f..4716f649 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -507,6 +507,17 @@ NOTMUCH_DUMP_TAGS ()
>      notmuch dump --include=tags "${@}" | sed '/^#/d' | sort
>  }
>  
> +notmuch_reply_sanitize_refs ()
> +{
> +    $NOTMUCH_PYTHON -c "
> +import email,sys
> +msg=email.message_from_file(sys.stdin)
> +del msg['in-reply-to']
> +del msg['references']
> +print(msg.as_string(False))
> +"
> +}
> +
>  notmuch_search_sanitize ()
>  {
>      perl -pe 's/("?thread"?: ?)("?)................("?)/\1\2XXX\3/'
> -- 
> 2.14.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] test/crypto: remove headers more robustly

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Fri, Sep 01 2017, David Bremner wrote:
>
>> In [1], Vladimir Panteleev observed that the In-Reply-To and
>> References headers could be wrapped in the 'default' output format of
>> notmuch-reply, depending on the version of Emacs creating the
>> message. In my own experiments notmuch-reply sometimes wraps headers
>> with only one message-id if that message-id is long enough. However it
>> happens, this causes the previous approach using grep to fail.
>
> we could (also) make emacs think it has wider than 80 characters to fit on
> one line...
>
>> Since I found the proposed unwrapping shell fragment in [1] a bit hard
>> to follow, I decided to write a little python script instead.
>
> There was nothing hard in that shell construct ;), but I also thought some
> alternative solutions (one in awk and one in perl)
>
> How 'bout
>
> drop_email_headers ()
> {
>     $NOTMUCH_PYTHON -c 'import email, sys
> msg = email.message_from_file(sys.stdin)
> for hdr in sys.argv: msg.pop(hdr, None)
> print(msg.as_string(False))'
> }
>
> and then ... | drop_email_headers In-Reply-To References

I pushed something like this. 'pop' doesn't seem to exist? Also I needed
to pass $* in to the python script. Maybe that should be "${@}", but
there really should not be spaces in header names.

d

[PATCH 7/7] .travis.yml: Test against several Emacs versions

[Vladimir Panteleev]

Use an env matrix and Flycheck's emacs-travis project to install and
test against multiple Emacs versions (currently 24.3, 24.5, 25.1, and
the latest development snapshot).
---
 .travis.yml | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index 8b71568d..5ee6a6cc 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -3,6 +3,12 @@ language: c
 dist: trusty
 sudo: false
 
+env:
+  - EMACS_VERSION=24.3
+  - EMACS_VERSION=24.5
+  - EMACS_VERSION=25.1
+  - EMACS_VERSION=snapshot
+
 addons:
   apt:
     packages:
@@ -15,6 +21,12 @@ addons:
     - gpgsm
 
 script:
+  # Configure and install Emacs (according to $EMACS_VERSION)
+  - export PATH="$HOME/bin:$PATH"
+  - wget 'https://raw.githubusercontent.com/flycheck/emacs-travis/master/emacs-travis.mk'
+  - make -f emacs-travis.mk install_emacs
+  - emacs --version
+
   - ./configure
   - make download-test-databases
   - make test
-- 
2.13.3

Re: [PATCH 7/7] .travis.yml: Test against several Emacs versions

[David Bremner]

Vladimir Panteleev <notmuch@thecybershadow.net> writes:


> +env:
> +  - EMACS_VERSION=24.3
> +  - EMACS_VERSION=24.5
> +  - EMACS_VERSION=25.1
> +  - EMACS_VERSION=snapshot
> +

I take it travis doesn't object to building 4 versions of emacs from
scratch? How long does that take?

>  script:
> +  # Configure and install Emacs (according to $EMACS_VERSION)
> +  - export PATH="$HOME/bin:$PATH"
> +  - wget 'https://raw.githubusercontent.com/flycheck/emacs-travis/master/emacs-travis.mk'
> +  - make -f emacs-travis.mk install_emacs
> +  - emacs --version
> +

I'm not very happy about downloading and running code from a third party
during our test runs. I guess we could embed a copy, althought that has
it's downsides as well.

Re: [PATCH 7/7] .travis.yml: Test against several Emacs versions

[Vladimir Panteleev]

On 2017-09-02 00:41, David Bremner wrote:
> I take it travis doesn't object to building 4 versions of emacs from
> scratch?

I don't know why they would. There are many open-source projects with 
much higher computational demands using Travis' free service. There are 
also many projects doing the exact thing proposed in this patch:

https://github.com/search?utf8=%E2%9C%93&q=travis+EMACS_VERSION+language%3AYAML&type=Code

> How long does that take?

About 2-4 minutes for the tagged releases and 6 minutes for the 
snapshot. You can see the times yourself in the build logs for the 
Travis results of my fork:

https://travis-ci.org/CyberShadow/notmuch

The figure you're looking for is the execution time of the "make -f 
emacs-travis.mk install_emacs" command, displayed on the right-hand side 
in the log.

> I'm not very happy about downloading and running code from a third party
> during our test runs.

Why do you think so?

Consider:

- You are already relying on a third-party here (Travis). They run a lot 
of code specific to their testing platform on their systems.

- The code (emacs-travis.mk) is specific to Travis. It is not part of 
the notmuch test suite, just the Travis configuration which runs it.

- Using emacs-travis.mk in this manner is how many Emacs packages run 
their tests:

https://github.com/search?p=1&q=emacs-travis.mk+language%3AYAML&type=Code&utf8=%E2%9C%93

-- 
Best regards,
  Vladimir

Re: [PATCH 0/7] Travis and test suite improvements

[David Bremner]

Vladimir Panteleev <notmuch@thecybershadow.net> writes:

> Test more things better on Travis CI, and fix things in the test suite
> that did not run properly on Travis CI.
>

I've pushed the first 5 in the series. I haven't had a chance to review
the last two yet.

d

Re: [PATCH 0/7] Travis and test suite improvements

[David Bremner]

David Bremner <david@tethera.net> writes:

> Vladimir Panteleev <notmuch@thecybershadow.net> writes:
>
>> Test more things better on Travis CI, and fix things in the test suite
>> that did not run properly on Travis CI.
>>
>
> I've pushed the first 5 in the series. I haven't had a chance to review
> the last two yet.
>
> d

Ah right now I remember. I had to amend 4/7 due to the renaming of
notmuch_database_add_message to notmuch_database_index_file. Although
the old name still exists as a wrapper, it seems to cause problems for
gdb to set the break on the wrapper function. Hopefully this still fixes
the problem on travis.

d