New/changed patchbot coverage plugin?

[Simon King]

Hi!

At #13370, I just noticed that the patchbot isn't even running the
doctest, because plugins.coverage complains. I think it is a bad idea to
not run the tests just because plugins.commit_messages or
plugins.coverage or plugins.trailing_whitespace fails.

In addition, I don't understand why the plugin is complaining: All
functions introduced by my patch have a test, and I even add a test that
refers to the ticket (showin that some problem is fixed). So, what
criteria are to meet for making the plugin happy?

Best regards,
Simon

[Robert Bradshaw]

On Fri, Aug 31, 2012 at 10:04 AM, Simon King <simon...@uni-jena.de> wrote:
> Hi!

Hi. Yeah, I've been making a lot of improvements over the last couple
of days. Glad to see someone noticed :).

> At #13370, I just noticed that the patchbot isn't even running the
> doctest, because plugins.coverage complains. I think it is a bad idea to
> not run the tests just because plugins.commit_messages or
> plugins.coverage or plugins.trailing_whitespace fails.

Yes, very true. This particular patchbot is running in a "plugin only"
mode for testing and quick coverage; the normal behavior has not
changed.

> In addition, I don't understand why the plugin is complaining: All
> functions introduced by my patch have a test, and I even add a test that
> refers to the ticket (showin that some problem is fixed). So, what
> criteria are to meet for making the plugin happy?

Looking at http://patchbot.sagemath.org/log/13370/debian/lenny/sid/x86_64/2.6.24-28-server/sage.math.washington.edu/2012-08-31%2008:05:43%20-0700?plugin=plugins.coverage&diff=/log/0/debian/lenny/sid/x86_64/2.6.24-28-server/sage.math.washington.edu/2012-08-31%2003%3A35%3A10%20-0700&ticket=13370&base=5.3.rc0
, I see

Decreased doctests misc/cachefunc.pyx from 56 / 63 = 88% to 60 / 68 = 88%

which indicates that there were formerly 7 undoctested functions in
this file and now there are 8. Now it's quite possible that sage
-coverageall has a bug in it, which is beyond the scope of the
patchbot.

Also, unhappy plugins shouldn't be a blocker, rather they should be
something the reviewer should at least be aware of (which may of
course indicate a blocker, if there are for example new undoctested
functions).

- Robert

[Simon King]

Hi Robert,

On 2012-08-31, Robert Bradshaw <robe...@math.washington.edu> wrote:
> On Fri, Aug 31, 2012 at 10:04 AM, Simon King <simon...@uni-jena.de> wrote:
>> In addition, I don't understand why the plugin is complaining: All
>> functions introduced by my patch have a test, and I even add a test that
>> refers to the ticket (showin that some problem is fixed). So, what
>> criteria are to meet for making the plugin happy?
>

> Looking at http://patchbot.sagemath.org/log/13370/debian/lenny/sid/x86_64/...

> , I see
>
> Decreased doctests misc/cachefunc.pyx from 56 / 63 = 88% to 60 / 68 = 88%
>
> which indicates that there were formerly 7 undoctested functions in
> this file and now there are 8.

The point is that my patch doesn't touch misc/cachefunc.pyx.

Cheers,
Simon

[Robert Bradshaw]

I see that you have 5 dependencies; perhaps one of them does? (It
computes the diff against pristine sage-main, dependencies and all.)
It doesn't try to build/run/test the union of your dependencies minus
the new patches (though that's arguably a sane feature request).

- Robert

[Simon King]

Hi Robert,

On 2012-08-31, Robert Bradshaw <robe...@math.washington.edu> wrote:
>> The point is that my patch doesn't touch misc/cachefunc.pyx.
>
> I see that you have 5 dependencies; perhaps one of them does?

The plugin compares sage-5.3.rc0 with sage-5.3.rc0+#13370. Two of the five
dependencies of the ticket are already part of sage-5.3.rc0. With the
dependencies of the dependencies, we have to consider #715, #11521,
#12215 and #12313.

#715 doesn't touch misc/cachefunc.pyx. Neither does #11521 or #12313.

#12215 introduces a new class in nisc/cachefunc. Both the class itself and its
methods `__init__`, `__call__`, `is_in_cache` and `set_cache` are
tested. I could imagine, though, that the plugin does not recognise all
of the tests: The class is called WeakCachedFunction, but it gives rise
to a decorator called weak_cached_function.

Cheers,
Simon

[Robert Bradshaw]

#12215 looks like the culprit:

http://patchbot.sagemath.org/log/12215/Fedora/17/x86_64/3.5.2-3.fc17.x86_64/volker-desktop.stp.dias.ie/2012-08-27%2012:34:42%20+0100?plugin=plugins.coverage&diff=/log/0/Fedora/17/x86_64/3.5.2-3.fc17.x86_64/volker-desktop.stp.dias.ie/2012-08-25%2018%3A55%3A21%20%2B0100&ticket=12215&base=5.3.rc0

which boils down to a broken sage -coverageall . File a ticket! :-)

- Robert