Html reporter can have trouble with binary files about diff_cover HOT 19 CLOSED

@Bachmann1234 do you have any idea how we might fix this? It's blocking some work for us, since it's borking the build (ref openedx/edx-platform#6921 )

from diff_cover.

@sarina oh jeez I had not fully grasp this was blocking and have not given this much time yet. Ill dig into this tonight.

from diff_cover.

@Bachmann1234 I might be able to give it some time in a few days, if you can't. Just wondering if you have a thought at where to start... heh.

from diff_cover.

Well ill give you my theory then.

So this explodes here: https://github.com/Bachmann1234/diff-cover/blob/master/diff_cover/snippets.py#L155

And it explodes trying to decode utf-8 output

What this tells me as that detect_encoding is detecting utf-8 but the encoding is something else.

There might be a way of checking the metadata of the file to see what encoding it was saved in... but more likely we still have to guess.

Since I don't think we can actually know the encoding of the file for sure the next best thing would be to throw UnicodeDammit from http://www.crummy.com/software/BeautifulSoup/bs4/doc/ at it. It tends to do pretty well. If you end up doing this just make sure to add chardet to the requirements as well as BS4 will not include it by default and this will lead to weaker encoding sniffing.

from diff_cover.

Of course... if it is a binary file any encoding would be wrong and we should not include it in the report

from diff_cover.

Yeah - I think that's the solution I really want, to just not try to run diff-cover on a binary file... 😞

from diff_cover.

@sarina yep, to reproduce this I would look at the bcmap files in pdf.js. thats my hunch

from diff_cover.

Oh and one more thing. I checked out that branch just now and ran git stat on it to see the files that changed as github was chocking under the load.

https://gist.github.com/Bachmann1234/cb957e80010b0e063a05

No python files. Unless i'm missing something the real question may be how pep8 found a violation to include in a snippet in the first place

from diff_cover.

@sarina im having trouble reproducing this
If you get stuck can you get me these pep8 reports?

/home/jenkins/workspace/edx-platform-all-tests-pr/SHARD/1/TEST_SUITE/quality/label_exp/jenkins-worker/reports/cms/pep8.report

 /home/jenkins/workspace/edx-platform-all-tests-pr/SHARD/1/TEST_SUITE/quality/label_exp/jenkins-worker/reports/lms/pep8.report 

/home/jenkins/workspace/edx-platform-all-tests-pr/SHARD/1/TEST_SUITE/quality/label_exp/jenkins-worker/reports/common/pep8.report

from diff_cover.

@Bachmann1234 @sarina For what it's worth, when I was reproducing this on the affected branch a couple of weeks ago, diff-quality was reporting 100% coverage...no violations for pep8 at the time. Even so, rendering the html report was problematic.

For the build pasted above, the .report files are empty for pep8. That is normal in a pull request that is keeping our pep8 violations at zero.

from diff_cover.

Hmm, this is a pickle

So I tried to recreate this by just using the diff in the PR (grabed platform and the remote. Went to that branch and ran diff)

This was the report i got out.

https://gist.github.com/Bachmann1234/b7c67b6d7fb38e57880e

from diff_cover.

Yeah that looks pretty a-ok. Sorry this is so hard to reproduce :( Maybe someone will check in a .exe and then we'll have another example :\

from diff_cover.

@sarina @benpatterson so I was thinking. I could make a branch that simply wraps that line with a try catch to find out exactly what file is causing it to choke on your build server.

Also, it would unblock that PR since it would run

from diff_cover.

We've decided to not let the quality issue actually block the PR. but would
be good at some point to fix this.

On Sat, Feb 21, 2015 at 4:30 PM, Matt Bachmann [email protected]
wrote:

@sarina https://github.com/sarina @benpatterson
https://github.com/benpatterson so I was thinking. I could make a
branch that simply wraps that line with a try catch to find out exactly
what file is causing it to choke on your build server.

Also, it would unblock that PR since it would run

—
Reply to this email directly or view it on GitHub
#4 (comment)
.

from diff_cover.

@sarina alrighty. Ill make another attempt to reproduce this sometime this week.

Probably after that PR is squashed and merged. Then I dont have to mess with remotes

from diff_cover.

Its been a while, has this issue ever come up again?

from diff_cover.

No but I suspect it's because we infrequently commit binary files.
On Sep 11, 2015 7:42 PM, "Matt Bachmann" [email protected] wrote:

Its been a while, has this issue ever come up again?

—
Reply to this email directly or view it on GitHub
#4 (comment)
.

from diff_cover.

Same as @sarina . I haven't seen it, but haven't been seeing too many binaries come by. I've attempted to reproduce it a few times by creating tars, zips and whatnot, but no luck. It was consistently happening when I reported it--on multiple systems, etc. But...no dice otherwise.

from diff_cover.

Odd, ok. Closing this now . If you ever find another example let me know.

from diff_cover.