geode-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Helena Bales <hba...@pivotal.io>
Subject Re: Test coverage and PRs
Date Tue, 16 Apr 2019 16:57:50 GMT
+1

On Tue, Apr 16, 2019 at 1:33 AM Juan José Ramos <jramos@pivotal.io> wrote:

> +1
>
> On Mon, Apr 15, 2019 at 11:22 PM Alexander Murmann <amurmann@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>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message