lilypond-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jonas Hahnfeld
Subject: Re: [RFC] Automatic 'make check' in CI
Date: Thu, 19 Nov 2020 23:00:01 +0100
User-agent: Evolution 3.38.1

Am Donnerstag, den 19.11.2020, 21:44 +0100 schrieb Michael Käppler:
> Am 18.11.2020 um 21:25 schrieb Jonas Hahnfeld:
> > Hi all,
> Hi Jonas,
> > 
> > I'd like to present a first workable version of 'make check' for use in
> > our CI pipelines. I've pushed the necessary commits to my personal fork
> > and created two merge requests to demonstrate the results:
> Great!
> > 
> > 1. Run 'make check' for merge requests (no difference)
> > URL: https://gitlab.com/hahnjo/lilypond/-/merge_requests/5
> > Job: https://gitlab.com/hahnjo/lilypond/-/jobs/858498690
> > test-results:
> > https://hahnjo.gitlab.io/-/lilypond/-/jobs/858498690/artifacts/test-results/index.html
> I stumbled across the git error in
> https://gitlab.com/hahnjo/lilypond/-/jobs/858498690#L85
> but this is from print-gittxt.sh, right?

Exactly, this is issue #6061 also mentioned below.

> > 
> > 2. Introduce difference in bar-lines.ly
> > URL: https://gitlab.com/hahnjo/lilypond/-/merge_requests/6
> > Job: https://gitlab.com/hahnjo/lilypond/-/jobs/852618720
> > test-results:
> > https://hahnjo.gitlab.io/-/lilypond/-/jobs/852618720/artifacts/test-results/index.html
> > 
> > This first workable implementation contains the minimum functionality:
> > It runs 'make test-baseline' for every push to the master branch and
> > replaces 'make test' in the pipelines for MRs with 'make check'. The
> > test-results are uploaded as artifacts and can be either downloaded as
> > zip archive or viewed directly (see above).
> > 
> > There are a few known issues that I'm aware of:
> >   - I needed to delete input/regression/option-help.ly because it logs
> > the options currently in use by lilypond, including the job-count which
> > varies when using our own runners with more than one core.
> Sounds like this should be fixed in get_help_string(), but I'm fine with
> removing
> the regtest, too. I can not imagine how this 'test' would be useful.
> >   - The test-results are pretty hard to find: Even if you know where to
> > look, it takes at least three clicks to download the zip archive plus
> > extracting and opening index.html; viewing the results directly takes
> > another three clicks from the job page (via 'Browse'). To ease the
> > second option, I've included a link in the latest jobs of !5 above. The
> > link is clickable which is a new feature of GitLab that I asked to be
> > enabled for my repo, so it won't work directly for lilypond/lilypond.
> >   - The gittree information does not contain the right merge base. This
> > is related to issue #6061 and I've submitted a change to GitLab to
> > expose the needed information in CI, but it wasn't merged yet.
> >   - The modified job for MRs always downloads the latest test-baseline
> > from master and not the one corresponding to the merge base. Same
> > reason as the previous point, I hope we can get the needed information
> > soon-ish. Until then, we need to rebase to adapt the MR commits to the
> > available test-baseline.
> But this is also the way James is testing currently, isn't it?

No, it's not:

> He does a `test-baseline` against latest master

Yes, but ...

> and a `make check` on the feature branch,

the feature branch gets applied on whatever latest master is (I
personally merged the branch, but the end result is the same). This
comes from the time when we really submitted patches / diffs via
Rietveld. As discussed in the other messages with Dan, I think this
would be the right way to test things, but the currently available
options make this unattractive.

> which may be lagging behing `master`...right?
> But I agree that for reproducibility it would be nice to take the
> baseline from the merge base.
> When would you do the `make test-baseline`, then?
> As part of the pipeline on the feature branch? Or would we keep a
> repository of, say, the
> last 20 baselines on `master` and pick the right one according to the
> merge base sha?

Generating test-baseline in the MR pipeline would be very hard to
implement, so I'd leave this to pipelines on the master branch. In my
book, this should also use less resources because, by definition, there
are less merges to master than pipelines on MRs. Downloading the right
artifact should be easy once we know the commit -> pipeline -> job.
GitLab takes care of removing the artifacts after 7 days, so we don't
have to worry about this.

> >   - Sometimes the test-results contain spurious diffs, for example here:
> > https://hahnjo.gitlab.io/-/lilypond/-/jobs/858441670/artifacts/test-results/index.html
> > I can reproduce this locally with --enable-checking, but haven't
> > investigated further yet (there were a couple of other problems that I
> > needed to solve in order to get things working...). If somebody has an
> > idea for a fix, that would be great but I think these can be safely
> > ignored for now.
> > 
> > There are a few more elaborate things that I'd like to work on in the
> > future. For example, GitLab can show a list of 'failing' tests which
> > can tell us at first glance if we need to look into the test-results.
> > I've prototyped this integration in the second MR, but it's very
> > misleading because the file extensions are missing and GitLab prints "0
> > failed out of null" when there are no tests. The obvious solution is to
> > mark all existing tests as success, but this requires a bit more
> > thought to integrate into output-distance.py (or somewhere else).
> > 
> > Despite these shortcomings, I think it would make sense to enable this
> > first implementation in lilypond/lilypond. What do you think?
> Does definitely make sense, however, I would value James' opinion on
> that, too.

🙂 I already received helpful feedback off-list before sharing with the
mailing list.

Jonas

> 
> Thanks for your work!
> Michael
> 

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


reply via email to

[Prev in Thread] Current Thread [Next in Thread]