Re: [PATCH 2/2] lib/message-property: catch xapian exceptions

Subject: Re: [PATCH 2/2] lib/message-property: catch xapian exceptions

Date: Mon, 27 Mar 2023 08:30:23 -0300

To: Kevin Boulain, notmuch@notmuchmail.org

Cc:

From: David Bremner


Kevin Boulain <kevin@boula.in> writes:

> Since libnotmuch exposes a C interface there's no way for clients to
> diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
> index 944e1810..f7cabe4d 100755
> --- a/test/T610-message-property.sh
> +++ b/test/T610-message-property.sh
> @@ -363,7 +363,6 @@ EOF

Overall this looks good, but I think the tests are a little cryptic as written.

>  test_expect_equal_file /dev/null OUTPUT
>  
>  test_begin_subtest "edit property on removed message without uncaught exception"
> -test_subtest_known_broken
>  cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
>  EXPECT0(notmuch_database_remove_message (db, notmuch_message_get_filename (message)));
>  EXPECT0(notmuch_message_remove_property (message, "example", "example"));
> @@ -371,11 +370,11 @@ EOF
>  cat <<'EOF' >EXPECTED
>  == stdout ==
>  == stderr ==
> +line 30: 3
>  EOF
>  test_expect_equal_file EXPECTED OUTPUT

In general I think it's better if the output of a test does not change
when marked non-broken. More importantly, in this case it means that the
call to notmuch_message_remove_property is not actually returning 0 any
more. So that output is actually the EXPECT0 assertion failing. I agree
it should print something more helpful (and I understand these
assumptions are not documented). So you should probably test that the
status is not success, and ideally print the message. You can see some
examples in T560-lib-error.sh, in particular in the created file c_tail
which handles checking the error code and printing the message.
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: