Re: Review Request 26123: Fail the build on lack of test coverage.
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.
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.
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.
--- 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
Re: 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/#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.
--- 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.
--- 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
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.
--- 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.
--- 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.
--- 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
