Re: [PATCH] test: Ignore subtly different behaviour of the musl libc

[Luis Ressel <aranea@aixah.de>]

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.)