Re: checking 2240

[Julien Rioux]

On 22/01/2012 2:50 PM, Graham Percival wrote:
> On Sun, Jan 22, 2012 at 08:43:26PM +0100, Janek Warchoł wrote:
> > One quick question: Patchy checks patches one at a time, doesn't it?
> > I.e. applies a patch (doesn't commit), tests, unapplies and moves to
> > another patch?
>
> ...
>
> why are you asking this question?  Is the source code really
> *that* hard to read?  It's 18 lines!
>
> https://github.com/gperciva/lilypond-extra/blob/master/patches/compile_lilypond_test.py#L282
>
>          for patch in patches:
>              issue_id = patch[0]
>              patch_filename = patch[1]
>              title = patch[2]
>              print "Trying %i with %s" % (issue_id, patch_filename)
>              try:
>                  autoCompile.configure()
>                  autoCompile.patch(patch_filename)
>                  autoCompile.build(quick_make=True,
> issue_id=issue_id)
>                  autoCompile.regtest_check(issue_id)

Python exception handling is simple but it can trip you up.
When any of patch, build, or regtest_check fails here, we exit the try: 
block and end up...

>                  autoCompile.copy_regtests(issue_id)
>                  autoCompile.make_regtest_show_script(issue_id,
> title)
>                  # reverse stuff
>                  autoCompile.patch(patch_filename, reverse=True)
>                  autoCompile.regtest_clean(issue_id)
>                  autoCompile.clean(issue_id)
>              except Exception as err:

...here in the except: block, bypassing the whole git patch --reverse 
and subsequent clean up for the failed patch. There is no clean up 
below, as you can see:

>                  print "Problem with issue %i" % issue_id
>                  print err
>
> - Graham
>
> _______________________________________________
> lilypond-devel mailing list
> address@hidden
> https://lists.gnu.org/mailman/listinfo/lilypond-devel

So there is a serious flaw in the patch-testing internals of this 
script. When every patch succeeds, everything is fine, but when one 
patch fails, everything snowballs.

Another issue is that git master is not updated to the latest 
origin/master before starting the test runs. This might be just a matter 
of documentation, but I would actually have this happen in the script 
itself since the name of the game is automatic testing.

Thanks to Janek for test running patchy, which led to both findings. 
He's been looking into fixing these issues, but he needs to get exams 
out of the way now, and could get back to hacking this up on Friday.

Regards,
Julien