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 Angela Schreiber <an...@adobe.com.INVALID> on 2017/05/11 10:05:25 UTC
Enforcing minimal test coverage
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
- oak-core-spi: 0.66
- oak-lucene: 0.81
- oak-solr-core: 0.53
- oak-segment-tar: 0.65
- oak-blob: 0.70
- oak-blob-plugins: 0.51
- oak-upgrade: 0.79
- oak-commons: 0.68
the coverage for the remaining modules were either way below < 50% or i
felt that enforcing coverage doesn't make too much sense.
if you wanted to get a feeling first how this would work, you can look at
the following modules that already have it enabled:
- oak-auth-external (0.8)
- oak-auth-ldap (0.73)
- oak-authorization-cug (0.97)
wdyt?
kind regards
angela
Re: Enforcing minimal test coverage
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi Angela,
On Thu, May 11, 2017 at 4:41 PM, Angela Schreiber
<an...@adobe.com.invalid> wrote:
> ...As far as I am concerned I only run the ITs if I know (or
> suspect) a change to have a huge impact.... otherwise it just takes too
> long...
Ok! I just meant to expose our solution in case it's useful otherwise.
Our setup also works if you skip integration tests, the "unit" and
"merged" jacoco reports are then identical (which makes tons of sense
;-)
-Bertrand
Re: Enforcing minimal test coverage
Posted by Angela Schreiber <an...@adobe.com.INVALID>.
Hi Bertrand
Thanks for the pointer. As I explained in my reply to Michael it's not
about the absolute numbers but rather about getting early feedback during
development. As far as I am concerned I only run the ITs if I know (or
suspect) a change to have a huge impact.... otherwise it just takes too
long. Still I would like to get reminded if I make a change and forget to
test it.
Kind regards
Angela
On 11/05/17 16:19, "Bertrand Delacretaz" <bd...@apache.org> wrote:
>On Thu, May 11, 2017 at 2:46 PM, Michael Dürig <md...@apache.org> wrote:
>> ...I guess this only includes the coverage from running unit tests and
>>does not
>> include integration tests...
>
>FWIW, In Sling we have a setup that computes the aggregate coverage of
>unit and integration tests that run in a single Maven module.
>
>That's defined in the jacoco-report profile in the parent pom at [1].
>
>-Bertrand
>
>https://svn.apache.org/repos/asf/sling/trunk/parent/pom.xml
Re: Enforcing minimal test coverage
Posted by Bertrand Delacretaz <bd...@apache.org>.
On Thu, May 11, 2017 at 2:46 PM, Michael Dürig <md...@apache.org> wrote:
> ...I guess this only includes the coverage from running unit tests and does not
> include integration tests...
FWIW, In Sling we have a setup that computes the aggregate coverage of
unit and integration tests that run in a single Maven module.
That's defined in the jacoco-report profile in the parent pom at [1].
-Bertrand
https://svn.apache.org/repos/asf/sling/trunk/parent/pom.xml
Re: Enforcing minimal test coverage
Posted by Angela Schreiber <an...@adobe.com.INVALID>.
hi michael
exactly... it's just the default build without any extra profiles or
requirements wrt running DBs or third party tools.
as far as i am concerned this is neither about the absolute numbers nor
about reaching some imaginary test coverage goal. it's about providing us
with immediate feedback when we add new features or refactor bigger
portions of the code without providing proper testing for it.
to me that has proven to be valuable and as you might have noticed when
following JIRA over the last year, i found a considerable amount of bigger
and smaller issues just by looking at the code with a different mindset.
kind regards
angela
On 11/05/17 14:46, "Michael Dürig" <md...@apache.org> wrote:
>
>
>On 11.05.17 12:05, Angela Schreiber wrote:
>> - oak-segment-tar: 0.65
>
>I guess this only includes the coverage from running unit tests and does
>not include integration tests. For segment tar the figures are actually
>much better with the latter, which is (another) indicator that segment
>tar needs to be further decoupled to make it easier to unit test.
>
>Michael
Re: Enforcing minimal test coverage
Posted by Michael Dürig <md...@apache.org>.
On 11.05.17 12:05, Angela Schreiber wrote:
> - oak-segment-tar: 0.65
I guess this only includes the coverage from running unit tests and does
not include integration tests. For segment tar the figures are actually
much better with the latter, which is (another) indicator that segment
tar needs to be further decoupled to make it easier to unit test.
Michael
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
Re: Enforcing minimal test coverage
Posted by Julian Reschke <ju...@gmx.de>.
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