* [PATCH] Compact tests and error handling
@ 2013-10-15 1:13 Ben Gamari
2013-10-15 1:13 ` [PATCH 1/3] test: Add compact test Ben Gamari
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Ben Gamari @ 2013-10-15 1:13 UTC (permalink / raw)
To: notmuch
Here are a few patches adding a test case and some more user feedback on
rename failure during compaction.
Cheers,
- Ben
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] test: Add compact test 2013-10-15 1:13 [PATCH] Compact tests and error handling Ben Gamari @ 2013-10-15 1:13 ` Ben Gamari 2013-10-20 12:18 ` David Bremner 2013-10-15 1:13 ` [PATCH 2/3] compact: Give user more feedback on failure renaming Ben Gamari ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Ben Gamari @ 2013-10-15 1:13 UTC (permalink / raw) To: notmuch Signed-off-by: Ben Gamari <bgamari.foss@gmail.com> --- test/compact | 35 +++++++++++++++++++++++++++++++++++ test/notmuch-test | 1 + 2 files changed, 36 insertions(+) create mode 100755 test/compact diff --git a/test/compact b/test/compact new file mode 100755 index 0000000..54e85ab --- /dev/null +++ b/test/compact @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +test_description='"notmuch compact"' +. ./test-lib.sh + +add_message '[subject]=One' +add_message '[subject]=Two' +add_message '[subject]=Three' + +notmuch tag +tag1 \* +notmuch tag +tag2 subject:Two +notmuch tag -tag1 +tag3 subject:Three + +test_begin_subtest "Compacting" +notmuch compact +test_expect_success "compact" "notmuch compact" + +notmuch search \* +output=$(notmuch search \* | notmuch_search_sanitize) +test_expect_equal "$output" "\ +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread) +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread) +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)" + +test_begin_subtest "Restoring backup" +rm -Rf ${TEST_TMPDIR}/mail/xapian +mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian + +notmuch search \* +output=$(notmuch search \* | notmuch_search_sanitize) +test_expect_equal "$output" "\ +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread) +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread) +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)" + +test_done diff --git a/test/notmuch-test b/test/notmuch-test index aa28bb0..ec94baf 100755 --- a/test/notmuch-test +++ b/test/notmuch-test @@ -19,6 +19,7 @@ cd $(dirname "$0") TESTS=" basic help-test + compact config setup new -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] test: Add compact test 2013-10-15 1:13 ` [PATCH 1/3] test: Add compact test Ben Gamari @ 2013-10-20 12:18 ` David Bremner 0 siblings, 0 replies; 8+ messages in thread From: David Bremner @ 2013-10-20 12:18 UTC (permalink / raw) To: Ben Gamari, notmuch Hi Ben; Thanks for writing these tests. I have some proposed changes. I didn't understand why you called notmuch search twice, and for whatever reason test_begin_subtest doesn't pair up with test_expect_success diff --git a/test/compact b/test/compact index 54e85ab..5bb5cea 100755 --- a/test/compact +++ b/test/compact @@ -10,11 +10,9 @@ notmuch tag +tag1 \* notmuch tag +tag2 subject:Two notmuch tag -tag1 +tag3 subject:Three -test_begin_subtest "Compacting" -notmuch compact -test_expect_success "compact" "notmuch compact" +test_expect_success "Running compact" "notmuch compact" -notmuch search \* +test_begin_subtest "Compact preserves database" output=$(notmuch search \* | notmuch_search_sanitize) test_expect_equal "$output" "\ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread) @@ -25,7 +23,6 @@ test_begin_subtest "Restoring backup" rm -Rf ${TEST_TMPDIR}/mail/xapian mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian -notmuch search \* output=$(notmuch search \* | notmuch_search_sanitize) test_expect_equal "$output" "\ thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] compact: Give user more feedback on failure renaming 2013-10-15 1:13 [PATCH] Compact tests and error handling Ben Gamari 2013-10-15 1:13 ` [PATCH 1/3] test: Add compact test Ben Gamari @ 2013-10-15 1:13 ` Ben Gamari 2013-10-15 1:13 ` [PATCH 3/3] compact: Provide user with more error feedback Ben Gamari 2013-10-26 12:26 ` [PATCH] Compact tests and error handling David Bremner 3 siblings, 0 replies; 8+ messages in thread From: Ben Gamari @ 2013-10-15 1:13 UTC (permalink / raw) To: notmuch Provide the user with instructions after we fail to move the old un-compacted database out of the way. Signed-off-by: Ben Gamari <bgamari.foss@gmail.com> --- lib/database.cc | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/database.cc b/lib/database.cc index 06f1c0a..57c2292 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -922,7 +922,14 @@ notmuch_database_compact (const char* path, if (old_xapian_path != NULL) { if (rename(xapian_path, old_xapian_path)) { - fprintf (stderr, "Error moving old database out of the way\n"); + fprintf (stderr, "Error moving old database out of the way: %s\n", + strerror(errno)); + fprintf (stderr, "\n"); + fprintf (stderr, "Old database: %s\n", xapian_path); + fprintf (stderr, "Compacted database: %s\n", compact_xapian_path); + fprintf (stderr, "\n"); + fprintf (stderr, "At this point it's probably best to remove the compacted database,\n"); + fprintf (stderr, "find the cause of this error, and try compacting again.\n"); ret = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] compact: Provide user with more error feedback 2013-10-15 1:13 [PATCH] Compact tests and error handling Ben Gamari 2013-10-15 1:13 ` [PATCH 1/3] test: Add compact test Ben Gamari 2013-10-15 1:13 ` [PATCH 2/3] compact: Give user more feedback on failure renaming Ben Gamari @ 2013-10-15 1:13 ` Ben Gamari 2013-10-20 18:13 ` Tomi Ollila 2013-10-26 12:26 ` [PATCH] Compact tests and error handling David Bremner 3 siblings, 1 reply; 8+ messages in thread From: Ben Gamari @ 2013-10-15 1:13 UTC (permalink / raw) To: notmuch Provide instructions on what to do when we couldn't move the compacted database into place. Signed-off-by: Ben Gamari <bgamari.foss@gmail.com> --- lib/database.cc | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/database.cc b/lib/database.cc index 57c2292..6f9fed1 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -938,7 +938,23 @@ notmuch_database_compact (const char* path, } if (rename(compact_xapian_path, xapian_path)) { - fprintf (stderr, "Error moving compacted database\n"); + fprintf (stderr, "Error moving compacted database into place: %s\n", strerror(errno)); + fprintf (stderr, "\n"); + fprintf (stderr, "Encountered error %s while moving the compacted database,\n", + strerror(errno)); + fprintf (stderr, "\n"); + fprintf (stderr, " %s\n", compact_xapian_path); + fprintf (stderr, "\n"); + fprintf (stderr, "to\n"); + fprintf (stderr, "\n"); + fprintf (stderr, " %s\n", xapian_path); + fprintf (stderr, "\n"); + fprintf (stderr, "Please identify the reason for this and move the compacted database into place manually.\n"); + if (backup_path != NULL) { + fprintf (stderr, "Otherwise, you can revert to the backup database located at,\n"); + fprintf (stderr, "\n"); + fprintf (stderr, " %s\n", backup_path); + } ret = NOTMUCH_STATUS_FILE_ERROR; goto DONE; } -- 1.8.1.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] compact: Provide user with more error feedback 2013-10-15 1:13 ` [PATCH 3/3] compact: Provide user with more error feedback Ben Gamari @ 2013-10-20 18:13 ` Tomi Ollila 0 siblings, 0 replies; 8+ messages in thread From: Tomi Ollila @ 2013-10-20 18:13 UTC (permalink / raw) To: Ben Gamari, notmuch On Tue, Oct 15 2013, Ben Gamari <bgamari.foss@gmail.com> wrote: > Provide instructions on what to do when we couldn't move the compacted > database into place. > > Signed-off-by: Ben Gamari <bgamari.foss@gmail.com> > --- > lib/database.cc | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/lib/database.cc b/lib/database.cc > index 57c2292..6f9fed1 100644 > --- a/lib/database.cc > +++ b/lib/database.cc > @@ -938,7 +938,23 @@ notmuch_database_compact (const char* path, > } > > if (rename(compact_xapian_path, xapian_path)) { > - fprintf (stderr, "Error moving compacted database\n"); > + fprintf (stderr, "Error moving compacted database into place: %s\n", strerror(errno)); > + fprintf (stderr, "\n"); > + fprintf (stderr, "Encountered error %s while moving the compacted database,\n", > + strerror(errno)); At this point 'errno' may have changed, so the above error string might not be the same. Maybe removing the '%s ' part altogether as inserting the part looks a bit strange... > + fprintf (stderr, "\n"); > + fprintf (stderr, " %s\n", compact_xapian_path); > + fprintf (stderr, "\n"); > + fprintf (stderr, "to\n"); > + fprintf (stderr, "\n"); > + fprintf (stderr, " %s\n", xapian_path); > + fprintf (stderr, "\n"); > + fprintf (stderr, "Please identify the reason for this and move the compacted database into place manually.\n"); > + if (backup_path != NULL) { > + fprintf (stderr, "Otherwise, you can revert to the backup database located at,\n"); the commas (,) here and a few lines above looks a bit strange to me. > + fprintf (stderr, "\n"); > + fprintf (stderr, " %s\n", backup_path); Maybe saying something how to do this revert would be enlightening to the user... (i.e. mentioning that backup_path needs to be moved as xapian_path). > + } > ret = NOTMUCH_STATUS_FILE_ERROR; > goto DONE; > } > -- Tomi ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Compact tests and error handling 2013-10-15 1:13 [PATCH] Compact tests and error handling Ben Gamari ` (2 preceding siblings ...) 2013-10-15 1:13 ` [PATCH 3/3] compact: Provide user with more error feedback Ben Gamari @ 2013-10-26 12:26 ` David Bremner 2013-10-27 20:29 ` Ben Gamari 3 siblings, 1 reply; 8+ messages in thread From: David Bremner @ 2013-10-26 12:26 UTC (permalink / raw) To: Ben Gamari, notmuch Ben Gamari <bgamari.foss@gmail.com> writes: > Here are a few patches adding a test case and some more user feedback on > rename failure during compaction. > Hi Ben; I'd like to include this series in the next release, freezing on Nov. 8. Will you have a chance to revise it before then? d ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] Compact tests and error handling 2013-10-26 12:26 ` [PATCH] Compact tests and error handling David Bremner @ 2013-10-27 20:29 ` Ben Gamari 0 siblings, 0 replies; 8+ messages in thread From: Ben Gamari @ 2013-10-27 20:29 UTC (permalink / raw) To: David Bremner, notmuch [-- Attachment #1: Type: text/plain, Size: 467 bytes --] David Bremner <david@tethera.net> writes: > Ben Gamari <bgamari.foss@gmail.com> writes: > >> Here are a few patches adding a test case and some more user feedback on >> rename failure during compaction. >> > > Hi Ben; > > I'd like to include this series in the next release, freezing on > Nov. 8. Will you have a chance to revise it before then? > Yes, I actually have the set sitting around waiting to be sent out. I'll take care of this right now. Cheers, - Ben [-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2013-10-27 20:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-10-15 1:13 [PATCH] Compact tests and error handling Ben Gamari 2013-10-15 1:13 ` [PATCH 1/3] test: Add compact test Ben Gamari 2013-10-20 12:18 ` David Bremner 2013-10-15 1:13 ` [PATCH 2/3] compact: Give user more feedback on failure renaming Ben Gamari 2013-10-15 1:13 ` [PATCH 3/3] compact: Provide user with more error feedback Ben Gamari 2013-10-20 18:13 ` Tomi Ollila 2013-10-26 12:26 ` [PATCH] Compact tests and error handling David Bremner 2013-10-27 20:29 ` Ben Gamari
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).