Re: [RFC] Automatic 'make check' in CI

[Jonas Hahnfeld]

Am Samstag, den 21.11.2020, 18:50 +0100 schrieb Jonas Hahnfeld:
> Am Samstag, den 21.11.2020, 09:46 +0100 schrieb Jonas Hahnfeld:
> > Am Freitag, den 20.11.2020, 17:55 -0500 schrieb Dan Eble:
> > > On Nov 20, 2020, at 12:22, Jonas Hahnfeld <hahnjo@hahnjo.de> wrote:
> > > > > > Despite these shortcomings, I think it would make sense to enable
> > > > > > this first implementation in lilypond/lilypond. What do you think?
> > > > > > 
> > > > > 
> > > > > yes, +1 . I am especially happy that this will likely have no extra
> > > > > overhead.
> > > > 
> > > > No extra overhead per pipeline in MRs, but you get an extra pipeline
> > > > for every merge to master. We'll see if this poses a serious problem…
> > > 
> > > Given the positive feedback this change has already received, can we
> > > accelerate its progress?  I'm eager to have it in place because I've
> > > got a branch with new some regression tests ready.
> > 
> > Sure, I can merge !517 later today and then work on the second MR to
> > actually enable 'make check' in the pipelines.
> 
> Something's not working correctly here:
> https://hahnjo.gitlab.io/-/lilypond/-/jobs/864846942/artifacts/test-results/index.html
> The test-baseline was generated by frog (using parallelism), check
> executed on a shared runner. I thought I had tested using a test-
> baseline with different number of cores...

What I probably checked is various values for CPU_COUNT, including just
one core, and all works correctly AFAICT. What gives differences is not
setting CPU_COUNT / job-count at all.

I successfully traced this back to compiling, for example, footnote-
volta-spanner.ly and skyline-point-extent.ly (in that order!) with an
added '\include "lilypond-book-preamble.ly"' (attached). If I run
 $ ./out/bin/lilypond footnote-volta-spanner.ly skyline-point-extent.ly
the skyline-point-extent.pdf has some added whitespace that vanishes
when passing -djob-count=1 or 2 (starting with 3, it won't fork because
there are less input files than jobs).

footnote-volta-spanner.ly was added by
commit c1f44faf958aafe7802f432a5ae0f4c74a8c0146
Author: Han-Wen Nienhuys <hanwenn@gmail.com>
Date:   Tue Jun 16 08:59:35 2020 +0200

    For footnotes on spanners, copy bounds from underlying spanner.
    
    This is likely more correct for spanners that run from non-musical
    columns (eg. volta brackets) or are created retroactively
    (eg. automatic beams).
    
    Demonstrate this by adding a regression test that puts a footnote on a
    VoltaBracket.
    
    Without adjustments of Footnote_engraver, the test produces the following:
    
      programming error: Spanner `FootnoteSpanner' is not fully contained in 
parent spanner.  Ignoring orphaned part
      continuing, cross fingers
      programming error: bounds of this piece aren't breakable.
      continuing, cross fingers

(https://gitlab.com/lilypond/lilypond/-/merge_requests/138). During
review, David had some concerns about Spanners that were dismissed on
the basis that there was no test case that showed wrong results (is
that really our standard for claiming that a change works correctly?!).

I tried reverting that commit and all other differences also went away.
Does this mean the change is indeed flawed?

Any ideas, comments?
Jonas

footnote-volta-spanner.ly
Description: Text Data

skyline-point-extent.ly
Description: Text Data

signature.asc
Description: This is a digitally signed message part