Use of ly:expect-warning in regtests

[Michael Käppler-2]

Hi all,
while fixing the issue Dan brought up (ottavation markups and MIDI
output) and looking
through the existing regtest ottavation-markups.ly, I noticed that it
does output a warning (as intended).
Other regtests, however, use ly:expect-warning to suppress these
warnings and output a new warning instead, if
the expected warning is missing.

Some "statistics":
Regtests, that _do_ output a warning

grep 'warning: ' lilypond/build/input/regression/out-test-baseline/*.log
| wc -l
52
(There are some warnings for missing expected warnings included, however)

Regtests, that use ly:expect-warning

git grep '(ly:expect-warning' input/regression/* | wc -l
118

Some use ly:expect-warning in combination with ly:set-option
'warning-as-error.
Mostly they set it to #f:

git grep "(ly:set-option 'warning-as-error #f" input/regression/* | wc -l
39

But some set it to #t:
git grep "(ly:set-option 'warning-as-error #t" input/regression/* | wc -l
9

Normally, 'warning-as-error is set to #f, anyway. Why is it necessary to
manually set it to #f?

It would be great if somebody could enlighten me here and tell me the
underlying policy,
what is considered good practise for the regtests. (if there is any)

Thanks in advance,
Michael

[Dan Eble]

On Oct 2, 2020, at 08:24, Michael Käppler <[hidden email]> wrote:
>
> Other regtests, however, use ly:expect-warning to suppress these
> warnings and output a new warning instead, if
> the expected warning is missing.
...
> Some use ly:expect-warning in combination with ly:set-option
> 'warning-as-error.
...
> But some set it to #t:
> git grep "(ly:set-option 'warning-as-error #t" input/regression/* | wc -l
> 9

That is how to test an expected warning.

> Mostly they set it to #f:
>
> git grep "(ly:set-option 'warning-as-error #f" input/regression/* | wc -l
> 39

I don't know the history behind that, but it doesn't seem useful.  It could be a mistake.
—
Dan

[Michael Käppler-2]

Am 02.10.2020 um 14:38 schrieb Dan Eble:
>> Mostly they set it to #f:
>>
>> git grep "(ly:set-option 'warning-as-error #f" input/regression/* | wc -l
>> 39
> I don't know the history behind that, but it doesn't seem useful.  It could be a mistake.
I think I got it.
At first there were neither 'warning-as-error nor ly:expect-warning.

In 2009, 'warning-as-error was introduced in
531042cb79ca69ad0599c77da9e5bfe4edbbde05

IIUC the idea was later to enable 'warning-as-error for all regtests to spot
potential failures earlier.
Since some regtests emitted warnings intentionally, there was a issue
created
for setting 'warning-as-error to #f for these regtests, so that they
would not break
compilation.

https://gitlab.com/lilypond/lilypond/-/issues/814

In 2011, ly:expect-warning was introduced with commit
1ecdd56060e34a00b2be6b38029b286a601ea6f8
by Reinhold Kainhofer.

Reinhold also adapted the regtests in such a way, that all
expected warnings should be suppressed and remaining warnings
unveil bugs.
However, he left the old (ly:set-option 'warning-as-error #f) code in.
(Don't know why)

tl;dr: I would suggest the following policy:
- do not output expected warnings in regtests, instead use ly:expect-warning
- set 'warning-as-error to #t for these regtests

For all existing regtests: remove (ly:set-option 'warning-as-error #f)

In the long run:
- check the regtests for warnings and figure out, whether
they are intended
  - if they are intended, add ly:expect-warning
- run the regtests with 'warning-as-error to #t, if all snipped are fixed

It seems that this topic has been around from time to time.
Maybe James can tell us, why running the regtests with 'warning-as-error #t
was never realized.

Cheers,
Michael

[James Lowe-9]

Hello

On 02/10/2020 14:34, Michael Käppler wrote:
> Maybe James can tell us, why running the regtests with
> 'warning-as-error #t
> was never realized.

I cannot. I was never part of those decisions.

Sorry.

James