Re: [PATCH v2 2/5] test: Add tests for 'config' command

Subject: Re: [PATCH v2 2/5] test: Add tests for 'config' command

Date: Sat, 31 Mar 2012 08:45:25 +0100

To: Peter Wang, notmuch@notmuchmail.org

Cc:

From: Mark Walters


This whole series looks good to me with one minor query.

> Start a new test script.
> ---
>  test/config       |   20 ++++++++++++++++++++
>  test/notmuch-test |    1 +
>  2 files changed, 21 insertions(+), 0 deletions(-)
>  create mode 100755 test/config
>
> diff --git a/test/config b/test/config
> new file mode 100755
> index 0000000..d3e574c
> --- /dev/null
> +++ b/test/config
> @@ -0,0 +1,20 @@
> +#!/usr/bin/env bash
> +
> +test_description='"notmuch config"'
> +. test-lib.sh
> +
> +test_begin_subtest "Get string value"
> +test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
> +
> +test_begin_subtest "Get list value"
> +test_expect_equal "$(notmuch config get new.tags)" "\
> +unread
> +inbox"
> +
> +test_expect_success "Set string value" \
> +  'notmuch config set foo.bar baz'
> +
> +test_expect_success "Set list value" \
> +  'notmuch config set foo.list xxx "yyy yyy" "zzz zzz"'

It seems off to call is success without checking that the value has
actually been set. Of course it is checked in the notmuch config list
test introduced in the next commit but I think if it would be better to
check with notmuch config get here too (i.e. check that reading back the
value gives what you want). Otherwise a failure in `setting' will show up
as a test failure in `listing'.

Best wishes

Mark

> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index f03b594..e08ec72 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -19,6 +19,7 @@ cd $(dirname "$0")
>  TESTS="
>    basic
>    help-test
> +  config
>    new
>    count
>    search
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Thread: