You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by Julian Reschke <ju...@gmx.de> on 2017/05/11 10:49:47 UTC

Re: Enforcing minimal test coverage

On 2017-05-11 12:05, Angela Schreiber wrote:
> hi oak-devs
>
> as a follow up to http://markmail.org/message/rzbqwwx3lilshct6 i would
> like to suggest that we enforce minimal test coverage not only with
> security modules but also for other oak code that is used in production.
>
> i gave it a try and identified those modules that already have a somewhat
> sensible coverage as this exercise IMO doesn't make sense for modules that
> are poorly tested.
>
> enforcing minimal coverage would mean that the module build will fail if
> the coverage is no longer met.
>
> the figures (computed yesterday) look as follows:
>
> - oak-jcr: 0.71
> - oak-core: 0.75
 > ...

To get coverage for oak-core, one should run with

a) MongoDB running and

b) Derby RDB profile.

Did you try that?

Best regards, Julian

Re: Enforcing minimal test coverage

Posted by Matt Ryan <os...@mvryan.org>.
Hi,

Speaking generally I think this is a good idea for Oak, so I am
appreciative of the proposal.

I assume we are talking about enforcement via build automation, meaning
there would be a step in the build that would compute the coverage number
and fail the build if the coverage number is not met.  I believe we'd need
the following:

- We need to decide on a coverage amount that is the minimum acceptable.  I
would suggest something in the range of 70%-80%.

- We need to be able to achieve the coverage amount without any external
dependencies, e.g. no external services.  This is necessary so the tests
can be run as a part of build automation, or by any developer working on
the codebase, without relying on setting up a testing environment that is
more involved than simply Java and Maven.  This may require some (perhaps
considerable) effort to build test tooling or mocks in order to reach the
coverage number.

- We need a way to provide exceptions, when approved by the group, for
specific modules.  (The purist in me resists this statement, but the
pragmatist in me says I should include it.)  There may be a given module
for which 68% coverage is about the best that can reasonably be done, or at
least it is the best that can reasonably be done for now.  This might be as
simple as providing an override for the percentage on a per-module basis.
We'd need to be careful not to just use this as a way to avoid writing
tests however.


FWIW I'm trying to chip away at this a little at a time, adding unit tests
as an exercise for learning the codebase, so I'm happy to help out here if
I can.

-MR

On Thu, May 11, 2017 at 11:50 PM, Julian Reschke <ju...@gmx.de>
wrote:

> On 2017-05-11 14:03, Angela Schreiber wrote:
>
>> hi julian
>>
>> no i didn't, because i don't think it makes sense to enforce a running
>> mongoDB for being able to build oak-core. nor does it make sense to me to
>> require a coverage that can only be reached with the derby-rdb profile
>> enabled.
>>
>
> Stepping back: what's the goal of enabling the checks then? My
> understanding was that we don't want to see regressions in coverage going
> forward. That's a good goal, but wouldn't work for people developing code
> that is only executed when running Mongo or RDB.
>
> but now that you mention: this is for me another hint that the document
>> store implementations should be moved out of oak-core rather sooner thank
>> later. i recently looked at it and it seems doable with moderate effort. i
>> recently had a discussion with marcel and he proposed that those really
>> familiar with the document store code take care of that.
>> ...
>>
>
> That's an orthogonal discussion, and yes we should have it.
>
> Best regards, Julian
>

Re: Enforcing minimal test coverage

Posted by Julian Reschke <ju...@gmx.de>.
On 2017-05-11 14:03, Angela Schreiber wrote:
> hi julian
>
> no i didn't, because i don't think it makes sense to enforce a running
> mongoDB for being able to build oak-core. nor does it make sense to me to
> require a coverage that can only be reached with the derby-rdb profile
> enabled.

Stepping back: what's the goal of enabling the checks then? My 
understanding was that we don't want to see regressions in coverage 
going forward. That's a good goal, but wouldn't work for people 
developing code that is only executed when running Mongo or RDB.

> but now that you mention: this is for me another hint that the document
> store implementations should be moved out of oak-core rather sooner thank
> later. i recently looked at it and it seems doable with moderate effort. i
> recently had a discussion with marcel and he proposed that those really
> familiar with the document store code take care of that.
> ...

That's an orthogonal discussion, and yes we should have it.

Best regards, Julian

Re: Enforcing minimal test coverage

Posted by Matt Ryan <os...@mvryan.org>.
On Thu, May 11, 2017 at 6:03 AM, Angela Schreiber <anchela@adobe.com.invalid
> wrote:

> hi julian
>
> no i didn't, because i don't think it makes sense to enforce a running
> mongoDB for being able to build oak-core. nor does it make sense to me to
> require a coverage that can only be reached with the derby-rdb profile
> enabled.
>
>
>
Right, it doesn't make sense to require a running service to build.  We may
have to figure out a way to mock these somehow in order to achieve the
coverage goal.  I admit I am saying that with no real knowledge of what
that might mean in this specific case, but I have done similar things
before.

-MR

Re: Enforcing minimal test coverage

Posted by Angela Schreiber <an...@adobe.com.INVALID>.
hi julian

no i didn't, because i don't think it makes sense to enforce a running
mongoDB for being able to build oak-core. nor does it make sense to me to
require a coverage that can only be reached with the derby-rdb profile
enabled.

but now that you mention: this is for me another hint that the document
store implementations should be moved out of oak-core rather sooner thank
later. i recently looked at it and it seems doable with moderate effort. i
recently had a discussion with marcel and he proposed that those really
familiar with the document store code take care of that.

an initial look reveal the following issues:
- 2 classes have dependencies to indexing
- several classes have dependencies to plugins.observation
- usage of oak constants spread over multiple plugins. for these i would
suggest to move constants that used outside of the scope of the plugins to
a single utility in oak-core-spi

kind regards
angela


On 11/05/17 12:49, "Julian Reschke" <ju...@gmx.de> wrote:

>On 2017-05-11 12:05, Angela Schreiber wrote:
>> hi oak-devs
>>
>> as a follow up to http://markmail.org/message/rzbqwwx3lilshct6 i would
>> like to suggest that we enforce minimal test coverage not only with
>> security modules but also for other oak code that is used in production.
>>
>> i gave it a try and identified those modules that already have a
>>somewhat
>> sensible coverage as this exercise IMO doesn't make sense for modules
>>that
>> are poorly tested.
>>
>> enforcing minimal coverage would mean that the module build will fail if
>> the coverage is no longer met.
>>
>> the figures (computed yesterday) look as follows:
>>
>> - oak-jcr: 0.71
>> - oak-core: 0.75
> > ...
>
>To get coverage for oak-core, one should run with
>
>a) MongoDB running and
>
>b) Derby RDB profile.
>
>Did you try that?
>
>Best regards, Julian