[Architecture] Balancing the need for improved test coverage with the need for continued forward progress...

Colin Clark colinbdclark at gmail.com
Wed May 11 17:03:17 UTC 2016


In general, I think having a view into test coverage will be very useful for anyone reviewing pull requests. I'd like to get a better feel for the value and reliability of a tool like Istanbul before we propose any formalized governance around test coverage, but I'm very supportive of the idea of introducing it into our workflow. Why don't we look to automate this, try it out, and then revisit the impact in practice in a few months to decide if it should be a formal part of the code review criteria?

I'd like to hear from Avtar and others working on the Quality Infrastructure for P4A in terms of how we can automate this for developers, and ultimately to expose the resulting metrics via the graphs and summaries in the QI's dashboard.

Colin

> On May 11, 2016, at 4:20 AM, Tony Atkins <tony at raisingthefloor.org> wrote:
> 
> Hi, All:
> 
> Antranig and I have often talked about using Istanbul <https://github.com/gotwarlost/istanbul> more widely to improve the test coverage of our projects.  It's no secret that I am a big proponent of test coverage.  Although it cannot protect you from all bugs, it certainly reduces the number of dark corners where they can hide.
> 
> I have lately been running Istanbul on each of my projects before submitting a PR, and working towards 100% coverage wherever possible.  I don't expect everyone else to commit to that as a starting point.  That would turn the next minor fix in a each package into a real chore, especially for a our bigger projects.  Instead, I wanted to start a discussion at tonight's meeting about enforcing "good enough" test coverage goals in future PRs.
> 
> As we don't have any standards at the moment, I'd propose starting with Istanbul's defaults <https://github.com/gotwarlost/istanbul/blob/ad8ab1d7a5284407efb83d0d7a8b32eb6fbd93c4/misc/config/istanbul-config.json#L38> for the code coverage levels:
> For lines of javascript, "red" is less than 50% coverage, "yellow" is 50-80%, and "green" is 80% coverage or better.
> For branches, "red" is less than 25% coverage, and "yellow" is less than 60% coverage.
> We could start by saying that for a PR to be approved, that there should be tests for at least 80% of the code that was changed as the focus of the PR.  Let's say you're updating a component and adding an option to reverse the neutron flow.  Let's also say you're a nice person and fix a few linting errors in the project while you're there.  You should definitely add tests for the new functionality, but you shouldn't be required to add tests for files where you basically fixed typos.
> 
> If we wanted to push for coverage a bit more aggressively, we could agree that there should be at least 80% coverage in each file that is changed as the focus of the PR.  This would push some but not all of the technical debt onto new contributors, but would help us more quickly add coverage.  To continue the above scenario, the contributor would be responsible for improving the tests for the file in which the component they are updating lives, but not for adding tests for the files where they fixed linting errors.
> 
> Of course, all of this is part of the discussion between the reviewer and submitter, and like JSHint, istanbul is just a tool to help inform that discussion.
> 
> I look forward to hearing your thoughts on the list and in the meeting tonight.
> 
> Cheers,
> 
> 
> Tony
> 
> 
> _______________________________________________
> Architecture mailing list
> Architecture at lists.gpii.net
> http://lists.gpii.net/mailman/listinfo/architecture

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.gpii.net/pipermail/architecture/attachments/20160511/c9483670/attachment.html>


More information about the Architecture mailing list