add covered_locs output to coverage #3231
Conversation
|
Maybe @samwgoldman or @aackerman could a look? |
|
@clarkbw I'm not on the flow team, I just happened to have touched this code last. It looks fine to me anyways. 👍 |
Ok, thanks @aackerman! |
|
@avikchaudhuri has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
Woo! |
|
Anything I can do to help here? |
|
What is the state of this? |
|
Any movement on this? |
|
Any word on this PR? Seems like it was pulled into Phabricator, would be great if someone with access to that could respond or at least give an update if any further work needs to be done? |
|
Rebased and updated this PR to resolve the conflict that was created when f6ae791 was merged a couple weeks go. 😫 😢 😭 |
|
Thanks @clarkbw! This PR would be awesome to get in, appreciate you keeping it up to date so that its ready to go when Facebook gets around to it. |
|
It’s an unrelated intermittent failure. Happy to rebase again but I don’t think there’s a point unless this might actually get merged. |
|
@clarkbw might as well remove that barrier. |
|
Anything remaining that's holding this back? I'd really like to start using it. |
|
This would be really nice to have better coverage utilities to block PR etc if coverage goes down. I believe this PR is the least we can accept to start moving forward! Diff is REALLY small! |
|
+1 to the above comments asking for this small change to be merged. The way I see it:
Thanks! |
|
cc @avikchaudhuri and @samwgoldman this would really help with improving flow line coverage reports. also see: rpl/flow-coverage-report#67 |
|
Merging this, sorry for it falling off the radar. |
|
Woo! Thanks, @avikchaudhuri |
Summary: Hello 👋 I've been working with the codecov team on the flow coverage report so I can upload my flow coverage report to http://codecov.io/ and use the service to analyze coverage over time. One thing I was told was missing from the flow coverage report (in json format) was a `covered_locs` output. There is `uncovered_locs` but no inverse for `covered_locs`, only the sum `covered_count` which makes it difficult for them to create the line-by-line report. Because the flow report only shows lines not covered Codecov never knows when lines are covered so it always results in 0% coverage. 🐼 This change simply adds `covered_locs` as an additional field to the json output. I can't foresee that this could be a breaking change for anyone ingesting the json output. It does create slightly larger output files as all covered lines now need to be described as well. I've only just given the latest coverage output to Codecov, when I hear back I'll update this if there are any additional issues but I wa Closes #3231 Reviewed By: samwgoldman, mrkev Differential Revision: D4861141 Pulled By: avikchaudhuri fbshipit-source-id: f55498956fe2ae9c385c40effb5be1717ed06279
Hello 👋
I've been working with the @codecov team on the flow coverage report so I can upload my flow coverage report to http://codecov.io/ and use the service to analyze coverage over time.
One thing I was told was missing from the flow coverage report (in json format) was a
covered_locsoutput. There isuncovered_locsbut no inverse forcovered_locs, only the sumcovered_countwhich makes it difficult for them to create the line-by-line report. Because the flow report only shows lines not covered Codecov never knows when lines are covered so it always results in 0% coverage. 🐼This change simply adds
covered_locsas an additional field to the json output. I can't foresee that this could be a breaking change for anyone ingesting the json output. It does create slightly larger output files as all covered lines now need to be described as well.I've only just given the latest coverage output to Codecov, when I hear back I'll update this if there are any additional issues but I wanted to get this conversation started in the mean time.