Re: Fix 1477: Add (ly:expect-warning msg args) to suppress expected warnings (issue 5037046)

[Reinhold Kainhofer]

Am Wednesday, 28. September 2011, 11:54:38 schrieb address@hidden:
> On Sep 28, 2011, at 11:45 AM, Reinhold Kainhofer wrote:
> > Am Wednesday, 28. September 2011, 09:07:12 schrieben Sie:
> >> LGTM
> >> 
> >> It'd be great to see the regtests run with -dwarning-as-error now that
> >> you've done all this work.  Is that possible to incorporate into this
> >> patch?
> > 
> > Actually, I don't think that's a good idea, for several reasons:
> > 
> > 1) The warning-as-error is currently only implemented in the Input class,
> > not for warnings that are triggered by directly calling (ly:warning...).
> > So we won't catch all warnings anyway.
> 
> Is there a way to make it such that they are capture?.

I'm working on it.

> > 2) Setting -dwarning-as-error means that any warning (e.g. a failed
> > barcheck...) will cause a complete failure of the whole regtest build. I
> > think this is way too volatile to be useful. A build should never fail,
> > except for real problems.
> 
> I agree, but perhaps warnings like bar, bar number, and octave checks can
> also have a throw/catch mechanism like the one you're implementing here.

Sure, if the warning is supposed to be there, it will be suppressed and the 
file won't fail. But I'm talking about those little oversights that happen 
when doing last-second changes before committing.

I don't think that an inadvertent warning should hold up all other developers 
from testing their own patches.

> True, but the whole reason these problems show up in the master branch in
> the first place is because people (including me) accidentally push stuff
> that generates warnings.  If the regtest build failed because of
> -dwarning-as-error, then the warning-generating code would not be pushed
> (assuming the person is not a malevolent pusher and/or lazy).  That's the
> big advantage I see of using -dwarning-as-error for regtests.

Well, I forgot my last reason:

3) We currently have several warnings in the regtest that should not be there. 
To see this, simply run "lilypond -l BASIC  *.ly" in input/regression.

With this patch applied, the only displayed warnings should be those from 
safe.ly and warn-expected-warning-missing.ly.

However, there are many more. Those are not expected, so suppressing them is 
like painting over the rust without actually fixing it. Unless all those are 
fixed, we simply cannot enable warning-as-error.

Attached is the current list of unexpected warning messages, coming from 23 
different regtests... Some of those are already reported as bugs, about others 
I'm not sure (i.e. I don't know what is really expected, in particular with 
the vertical spacing).

Cheers,
Reinhold

-- 
------------------------------------------------------------------
Reinhold Kainhofer, address@hidden, http://reinhold.kainhofer.com/
 * Financial & Actuarial Math., Vienna Univ. of Technology, Austria
 * http://www.fam.tuwien.ac.at/, DVR: 0005886
 * LilyPond, Music typesetting, http://www.lilypond.org

regtests-unexpected-warnings.txt
Description: Text document