On Mon, Mar 04, 2019 at 08:04:21AM -0400, David Bremner wrote: > Luis Ressel <aranea@aixah.de> writes: > > > --- > > test/T030-config.sh | 6 ++++-- > > test/T650-regexp-query.sh | 4 ++-- > > 2 files changed, 6 insertions(+), 4 deletions(-) > > > > In general we want more verbose commit messages for notmuch > (https://notmuchmail.org/contributing/#index5h2) > > > +test_begin_subtest "List all items (stderr output)" > > +test_expect_equal "$(notmuch_config_sanitize <OUTPUT-ERR)" "Error opening database at MAIL_DIR/.notmuch: No such file or directory" > > + > > The first change looks OK. Another option would be to cat the two files > into one in the test with a seperator. That's how the test_C based tests work. > I agree that's nicer, and just submitted an updated patch. > > test_begin_subtest "Top level --config=FILE option" > > cp "${NOTMUCH_CONFIG}" alt-config > > notmuch --config=alt-config config set user.name "Another Name" > > diff --git a/test/T650-regexp-query.sh b/test/T650-regexp-query.sh > > index 4085340f..9ba3cd64 100755 > > --- a/test/T650-regexp-query.sh > > +++ b/test/T650-regexp-query.sh > > @@ -137,10 +137,10 @@ EOF > > test_expect_equal_file EXPECTED OUTPUT > > > > test_begin_subtest "regexp error reporting" > > -notmuch search 'from:/unbalanced[/' 1>OUTPUT 2>&1 > > +notmuch search 'from:/unbalanced[/' 2>&1 | sed -e 's/^\(A Xapian[^:]*:\).*/\1/' > OUTPUT > > cat <<EOF > EXPECTED > > notmuch search: A Xapian exception occurred > > -A Xapian exception occurred parsing query: Invalid regular expression > > +A Xapian exception occurred parsing query: > > Query string was: from:/unbalanced[/ > > EOF > > This seems to lose the fact that a regexp parsing error occured. One > option would be to change the actual error message so the initial > predictable part of the error message contained some string like > "regexp". I don't really think this is a problem. notmuch's test suite is quite comprehensive, so I don't consider it essential to verify that this particular Xapian failure is caused by a regex issue as opposed to another problem. I could change the test so it accepts both the glibc and musl errors ("Invalid regular expression" and "Missing ']'", respectively), but I can't say I'm enthusiastic about that. POSIX does not specify the output of regerror(), and in fact not even which error code should be used here (musl's regcomp() currently returns REG_EBRACK, while glibc probably uses the generic REG_BADPAT); thus, the error message may change arbitrarily during a libc update, or be something else entirely for another libc implementation. If you really consider it important to convey the info that there was a regex parsing error, I can submit a patch to make compile_regex() prefix the regerror output with "regex error: " or sth like that. Regards, Luis Ressel (Resending due to trouble with my new MUA; apologies if you receive this mail twice.) _______________________________________________ notmuch mailing list notmuch@notmuchmail.org https://notmuchmail.org/mailman/listinfo/notmuch