search

grodowski / undercover

undercover warns about methods, classes and blocks that were changed without tests, to help you easily find untested code and reduce the number of bugs. It does so by analysing data from git diffs, code structure and SimpleCov coverage reports
https://undercover-ci.com
MIT License
724 stars 28 forks source link

Undercover is slow comparing binary files #174

Closed kuahyeow closed 2 years ago

kuahyeow commented 2 years ago

Normally:

$ bundle exec undercover -c "$(git merge-base origin/master HEAD)"
🚨 WARNING: Coverage data is older than your latest changes and results might be incomplete. Re-run tests to update
undercover: ✅ No coverage is missing in latest changes
Undercover finished in 20.747s

With a binary file in the staging area:

$ git s
On branch test-merge-base
Your branch is up to date with 'origin/test-merge-base'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
    git-cheat.pdf

nothing added to commit but untracked files present (use "git add" to track)
$ git add git-cheat.pdf 
$ ls -lah git-cheat.pdf 
-rw-r--r--@ 1 tkuah  staff   7.0M 23 Jun 11:59 git-cheat.pdf
$ bundle exec undercover -c "$(git merge-base origin/master HEAD)"
🚨 WARNING: Coverage data is older than your latest changes and results might be incomplete. Re-run tests to update
undercover: ✅ No coverage is missing in latest changes
Undercover finished in 119.039s

Do you think it might be OK to add an option to exclude binary files ?

grodowski commented 2 years ago

Thanks for reporting! It could make sense to only have an allow-list for common ruby extensions for the diff 🤔 My guess is that the slowness comes from Undercover::Changeset#full_diff and it might be where that filtering is needed.

kuahyeow commented 2 years ago

Thanks for the reply @grodowski - Yes, I agree the slowness will be from full_diff possibly.

However, it looks like the file I was comparing is not a a binary, even though it's a PDF (it's some ASCII based format). So my original suggestion to exclude binary files is invalid. I'm happy to close this for now.