You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Alexander Murmann <am...@apache.org> on 2019/04/15 22:22:42 UTC
Test coverage and PRs
Hi everyone,
TL;DR:
1. Let's call each other out on our PRs when test coverage is missing or
insufficient
2. Let's require test coverage for code that gets refactored as well
We already have a really great wiki page
<https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions>
on how we evaluate code contributions. Among other things it calls out test
coverage for new features as well as for bug fixes. I don't think we've
always been diligent in ensuring healthy test coverage when we've been
reviewing PRs. This seems like something we should correct as a community
to ensure Geode can provide the high quality our users expect and at the
same time give us fast feedback loops in our daily work. We need to get
better code coverage, so rejecting PRs till they have appropriate test
coverage, seems like an obvious thing we need to do.
It's also interesting to me that the wiki page calls out coverage for
features and bugs. What about refactoring code that is currently missing
coverage? To refactor with confidence we need test coverage. Therefore test
coverage should be a precondition for refactoring. I'd like to amend our
wiki to also require test coverage for refactored code. Ideally that
coverage would already be there, but we all know that's unfortunately
rarely the case. This is going to hurt in the short run, but gets us closer
where we want to be.
As always, if it's unreasonable to provide certain types of test coverage,
then let's have an explicit conversation on the PR.
Note: When I say "appropriate test coverage", I am referring to coverage of
the functionality in breadth, but also in depth via a healthy testing
pyramid <https://martinfowler.com/bliki/TestPyramid.html> that includes
unit tests, integration tests etc.
What do you all think?
Re: Test coverage and PRs
Posted by Helena Bales <hb...@pivotal.io>.
+1
On Tue, Apr 16, 2019 at 1:33 AM Juan José Ramos <jr...@pivotal.io> wrote:
> +1
>
> On Mon, Apr 15, 2019 at 11:22 PM Alexander Murmann <am...@apache.org>
> wrote:
>
> > Hi everyone,
> >
> > TL;DR:
> > 1. Let's call each other out on our PRs when test coverage is missing or
> > insufficient
> > 2. Let's require test coverage for code that gets refactored as well
> >
> > We already have a really great wiki page
> > <
> >
> https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions
> > >
> > on how we evaluate code contributions. Among other things it calls out
> test
> > coverage for new features as well as for bug fixes. I don't think we've
> > always been diligent in ensuring healthy test coverage when we've been
> > reviewing PRs. This seems like something we should correct as a community
> > to ensure Geode can provide the high quality our users expect and at the
> > same time give us fast feedback loops in our daily work. We need to get
> > better code coverage, so rejecting PRs till they have appropriate test
> > coverage, seems like an obvious thing we need to do.
> >
> > It's also interesting to me that the wiki page calls out coverage for
> > features and bugs. What about refactoring code that is currently missing
> > coverage? To refactor with confidence we need test coverage. Therefore
> test
> > coverage should be a precondition for refactoring. I'd like to amend our
> > wiki to also require test coverage for refactored code. Ideally that
> > coverage would already be there, but we all know that's unfortunately
> > rarely the case. This is going to hurt in the short run, but gets us
> closer
> > where we want to be.
> >
> > As always, if it's unreasonable to provide certain types of test
> coverage,
> > then let's have an explicit conversation on the PR.
> >
> > Note: When I say "appropriate test coverage", I am referring to coverage
> of
> > the functionality in breadth, but also in depth via a healthy testing
> > pyramid <https://martinfowler.com/bliki/TestPyramid.html> that includes
> > unit tests, integration tests etc.
> >
> > What do you all think?
> >
>
>
> --
> Juan José Ramos Cassella
> Senior Technical Support Engineer
> Email: jramos@pivotal.io
> Office#: +353 21 4238611
> Mobile#: +353 87 2074066
> After Hours Contact#: +1 877 477 2269
> Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
> How to upload artifacts:
> https://support.pivotal.io/hc/en-us/articles/204369073
> How to escalate a ticket:
> https://support.pivotal.io/hc/en-us/articles/203809556
>
> [image: support] <https://support.pivotal.io/> [image: twitter]
> <https://twitter.com/pivotal> [image: linkedin]
> <https://www.linkedin.com/company/3048967> [image: facebook]
> <https://www.facebook.com/pivotalsoftware> [image: google plus]
> <https://plus.google.com/+Pivotal> [image: youtube]
> <https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl>
>
Re: Test coverage and PRs
Posted by Juan José Ramos <jr...@pivotal.io>.
+1
On Mon, Apr 15, 2019 at 11:22 PM Alexander Murmann <am...@apache.org>
wrote:
> Hi everyone,
>
> TL;DR:
> 1. Let's call each other out on our PRs when test coverage is missing or
> insufficient
> 2. Let's require test coverage for code that gets refactored as well
>
> We already have a really great wiki page
> <
> https://cwiki.apache.org/confluence/display/GEODE/Criteria+for+Code+Submissions
> >
> on how we evaluate code contributions. Among other things it calls out test
> coverage for new features as well as for bug fixes. I don't think we've
> always been diligent in ensuring healthy test coverage when we've been
> reviewing PRs. This seems like something we should correct as a community
> to ensure Geode can provide the high quality our users expect and at the
> same time give us fast feedback loops in our daily work. We need to get
> better code coverage, so rejecting PRs till they have appropriate test
> coverage, seems like an obvious thing we need to do.
>
> It's also interesting to me that the wiki page calls out coverage for
> features and bugs. What about refactoring code that is currently missing
> coverage? To refactor with confidence we need test coverage. Therefore test
> coverage should be a precondition for refactoring. I'd like to amend our
> wiki to also require test coverage for refactored code. Ideally that
> coverage would already be there, but we all know that's unfortunately
> rarely the case. This is going to hurt in the short run, but gets us closer
> where we want to be.
>
> As always, if it's unreasonable to provide certain types of test coverage,
> then let's have an explicit conversation on the PR.
>
> Note: When I say "appropriate test coverage", I am referring to coverage of
> the functionality in breadth, but also in depth via a healthy testing
> pyramid <https://martinfowler.com/bliki/TestPyramid.html> that includes
> unit tests, integration tests etc.
>
> What do you all think?
>
--
Juan José Ramos Cassella
Senior Technical Support Engineer
Email: jramos@pivotal.io
Office#: +353 21 4238611
Mobile#: +353 87 2074066
After Hours Contact#: +1 877 477 2269
Office Hours: Mon - Thu 08:30 - 17:00 GMT. Fri 08:30 - 16:00 GMT
How to upload artifacts:
https://support.pivotal.io/hc/en-us/articles/204369073
How to escalate a ticket:
https://support.pivotal.io/hc/en-us/articles/203809556
[image: support] <https://support.pivotal.io/> [image: twitter]
<https://twitter.com/pivotal> [image: linkedin]
<https://www.linkedin.com/company/3048967> [image: facebook]
<https://www.facebook.com/pivotalsoftware> [image: google plus]
<https://plus.google.com/+Pivotal> [image: youtube]
<https://www.youtube.com/playlist?list=PLAdzTan_eSPScpj2J50ErtzR9ANSzv3kl>