You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2014/09/28 01:25:59 UTC
Review Request 26123: Fail the build on lack of test coverage.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/
-----------------------------------------------------------
Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
Repository: aurora
Description
-------
This will fail the build if:
- global line or branch coverage is below a threshold
- a class has no test coverage
- a class flagged as known to have no coverage gains coverage
Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
Diffs
-----
build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
Diff: https://reviews.apache.org/r/26123/diff/
Testing
-------
./gradlew build -Pq
Thanks,
Bill Farner
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 28, 2014, 4:05 a.m., Joshua Cohen wrote:
> > build.gradle, line 564
> > <https://reviews.apache.org/r/26123/diff/1/?file=707768#file707768line564>
> >
> > extra slash?
Fixed.
> On Sept. 28, 2014, 4:05 a.m., Joshua Cohen wrote:
> > build.gradle, lines 565-566
> > <https://reviews.apache.org/r/26123/diff/1/?file=707768#file707768line565>
> >
> > can we pull these values out to constants so it's easy[1] to bump them up as coverage increases?
> >
> > [1] not that it's hard, but I imagine future diffs are easier if it's just:
> >
> > + MINIMUM_LINE_COVERAGE=0.87
> > - MINIMUM_LINE_COVERAGE=0.86
Done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54772
-----------------------------------------------------------
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 28, 2014, 4:05 a.m., Joshua Cohen wrote:
> > build.gradle, line 571
> > <https://reviews.apache.org/r/26123/diff/1/?file=707768#file707768line571>
> >
> > This seems like a potentially harsh penalty for good behavior if someone has to go from 0 to $MIN_COVERAGE in one go...
> >
> > I can imagine it encouraging the opposite of the desired behavior for someone adding new code to a legacy class but not adding code coverage because it means writing tests for the entire class as part of their change.
I think you might have a different idea of how this diff behaves (and perhaps the message should be updated to reflect as much - please advise).
This assertion will trip if there is _any_ test coverage added to one of these legacy classes. When you encounter this message, you should pat yourself on the back and remove the class from the legacy list. It's a provision to prevent the list itself from becoming stale.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54772
-----------------------------------------------------------
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Joshua Cohen <jc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54772
-----------------------------------------------------------
Love this!
build.gradle
<https://reviews.apache.org/r/26123/#comment95046>
extra slash?
build.gradle
<https://reviews.apache.org/r/26123/#comment95047>
can we pull these values out to constants so it's easy[1] to bump them up as coverage increases?
[1] not that it's hard, but I imagine future diffs are easier if it's just:
+ MINIMUM_LINE_COVERAGE=0.87
- MINIMUM_LINE_COVERAGE=0.86
build.gradle
<https://reviews.apache.org/r/26123/#comment95049>
This seems like a potentially harsh penalty for good behavior if someone has to go from 0 to $MIN_COVERAGE in one go...
I can imagine it encouraging the opposite of the desired behavior for someone adding new code to a legacy class but not adding code coverage because it means writing tests for the entire class as part of their change.
- Joshua Cohen
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 27, 2014, 11:38 p.m., Zameer Manji wrote:
> > Once this is commited, please make tickets for adding tests to these classes.
I'm not sure how to best do this without either creating a ton of tickets that are bound to be forgotten, or a monster ticket that is difficult to track. Any bright ideas?
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54771
-----------------------------------------------------------
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Zameer Manji <zm...@twopensource.com>.
> On Sept. 27, 2014, 4:38 p.m., Zameer Manji wrote:
> > Once this is commited, please make tickets for adding tests to these classes.
>
> Bill Farner wrote:
> I'm not sure how to best do this without either creating a ton of tickets that are bound to be forgotten, or a monster ticket that is difficult to track. Any bright ideas?
I have no ideas. In that case feel free to ignore my request for tickets.
- Zameer
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54771
-----------------------------------------------------------
On Sept. 27, 2014, 4:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 4:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Zameer Manji <zm...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54771
-----------------------------------------------------------
Once this is commited, please make tickets for adding tests to these classes.
- Zameer Manji
On Sept. 27, 2014, 4:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 4:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
> On Sept. 28, 2014, 2:57 a.m., Maxim Khutornenko wrote:
> > build.gradle, lines 565-566
> > <https://reviews.apache.org/r/26123/diff/1/?file=707768#file707768line565>
> >
> > Move thresholds to constants for better visibility?
Done.
- Bill
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54773
-----------------------------------------------------------
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54773
-----------------------------------------------------------
Ship it!
build.gradle
<https://reviews.apache.org/r/26123/#comment95048>
Move thresholds to constants for better visibility?
- Maxim Khutornenko
On Sept. 27, 2014, 11:25 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 27, 2014, 11:25 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Joshua Cohen <jc...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54908
-----------------------------------------------------------
Ship it!
Thanks for clarifying the assert message.
- Joshua Cohen
On Sept. 29, 2014, 9:10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2014, 9:10 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/
-----------------------------------------------------------
(Updated Sept. 29, 2014, 11:33 p.m.)
Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
Changes
-------
- change to using instruction instead of line coverage, since this is more prominently displayed in the HTML report
- fixed coverage computation to match HTML report
- pulled function call out of assertion to make a failed assertion show the resulting double value
Repository: aurora
Description
-------
This will fail the build if:
- global line or branch coverage is below a threshold
- a class has no test coverage
- a class flagged as known to have no coverage gains coverage
Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
Diffs (updated)
-----
build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
Diff: https://reviews.apache.org/r/26123/diff/
Testing
-------
./gradlew build -Pq
Thanks,
Bill Farner
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Kevin Sweeney <ke...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/#review54902
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Sweeney
On Sept. 29, 2014, 2:10 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26123/
> -----------------------------------------------------------
>
> (Updated Sept. 29, 2014, 2:10 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> This will fail the build if:
> - global line or branch coverage is below a threshold
> - a class has no test coverage
> - a class flagged as known to have no coverage gains coverage
>
> Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
>
>
> Diffs
> -----
>
> build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
>
> Diff: https://reviews.apache.org/r/26123/diff/
>
>
> Testing
> -------
>
> ./gradlew build -Pq
>
>
> Thanks,
>
> Bill Farner
>
>
Re: Review Request 26123: Fail the build on lack of test coverage.
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26123/
-----------------------------------------------------------
(Updated Sept. 29, 2014, 9:10 p.m.)
Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko.
Repository: aurora
Description
-------
This will fail the build if:
- global line or branch coverage is below a threshold
- a class has no test coverage
- a class flagged as known to have no coverage gains coverage
Hopefully we can all contribute to whittle the legacy non-covered list down to zero.
Diffs (updated)
-----
build.gradle eabf65c13749ca98929e6b845cbc5f0d248003d6
Diff: https://reviews.apache.org/r/26123/diff/
Testing
-------
./gradlew build -Pq
Thanks,
Bill Farner