From: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>
To: david@tethera.net, notmuch@notmuchmail.org
Subject: Re: [PATCH] test: add nontrivial test for restore --accumulate.
In-Reply-To: <1353069079-25270-1-git-send-email-david@tethera.net>
References: <1353069079-25270-1-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.14+45~g6ea9330 (http://notmuchmail.org) Emacs/24.1.1
	(x86_64-pc-linux-gnu)
Date: Fri, 16 Nov 2012 20:56:37 -0500
Message-ID: <8762552dre.fsf@betacantrips.com>
MIME-Version: 1.0
Content-Type: text/plain
Cc: David Bremner <bremner@debian.org>
Precedence: list

david@tethera.net writes:

> From: David Bremner <bremner@debian.org>
>
> It seems we have never tested the case that restore --accumulate
> actually adds tags. I noticed this when I started optimizing and no
> tests failed.
>
> I also had to modify the next test. Perhaps a seperate patch could
> make these tests more independent of the previous ones.
> ---
>  test/dump-restore |   14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/test/dump-restore b/test/dump-restore
> index f25f7cf..ca7a730 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -29,18 +29,20 @@ test_expect_success 'Accumulate original tags' \
>    notmuch dump > dump.actual &&
>    test_cmp dump-ABC_DEF.expected dump.actual'
>
> -test_expect_success 'Restoring original tags' \
> -  'notmuch restore --input=dump.expected &&
> -  notmuch dump > dump.actual &&
> -  test_cmp dump.expected dump.actual'
> -

I guess you're removing this test because it just shows that restore can
remove tags, and we already see that in earlier tests?

>  test_expect_success 'Restore with nothing to do' \
>    'notmuch restore < dump.expected &&
>    notmuch dump > dump.actual &&
>    test_cmp dump.expected dump.actual'

Maybe change the name of this test, as now it certainly does something?

> +test_expect_success 'Accumulate with changes' \
> +  'notmuch restore --input=dump.expected &&
> +   notmuch restore --accumulate --input=dump-ABC_DEF.expected &&
> +  notmuch dump >  OUTPUT.$test_count &&
> +  test_cmp dump-ABC_DEF.expected OUTPUT.$test_count'

Alignment? I think each line should start with two spaces.

> +
>  test_expect_success 'Restore with nothing to do, II' \
> -  'notmuch restore --accumulate --input=dump.expected &&
> +  'notmuch restore --input=dump.expected &&
> +  notmuch restore --accumulate --input=dump.expected &&
>    notmuch dump > dump.actual &&
>    test_cmp dump.expected dump.actual'

Maybe change the name? "Accumulate with nothing to do", for instance?

Ethan