Refactor hash generation for testability #368
Conversation
tox.ini
Outdated
| @@ -6,6 +6,7 @@ deps = | |||
| coverage | |||
| pretend | |||
| pytest | |||
| py27,pypy: pyblake2 | |||
There was a problem hiding this comment.
hashlib on py2 doesn't have a blake2b hash but pyblake2 is used for backwards compatibility there so let's install it for our testing purposes.
| blake_update(content) | ||
| hasher = HashManager(filename) | ||
| hasher.hash() | ||
| hexdigest = hasher.hexdigest() |
There was a problem hiding this comment.
We could also return a namedtuple here instead of a dictionary. I didn't think it mattered much either way right now
There was a problem hiding this comment.
I definitely prefer namedtuples over dicts for cases like this.
| return self._blake_hasher.hexdigest() | ||
| return None | ||
|
|
||
| def hash(self): |
There was a problem hiding this comment.
This could get called automagically, or we could add a function that creates the object, calls this method, and then returns the hexdigest, but I wasn't sure that was necessary either
|
cool, thanks @sigmavirus24 |
| blake_update(content) | ||
| hasher = HashManager(filename) | ||
| hasher.hash() | ||
| hexdigest = hasher.hexdigest() |
There was a problem hiding this comment.
I definitely prefer namedtuples over dicts for cases like this.
After reviewing #367 I realized that this code could be more easily tested if it was broken out from the PackageFile object. Since it was such a tiny refactor, I went ahead, refactored it and added the necessary tests to ensure this just works.
sigmavirus24
force-pushed
the
hashing-and-testing
branch
from
82a50aa
to
cbcb856
Compare
Codecov Report
@@ Coverage Diff @@
## master #368 +/- ##
==========================================
+ Coverage 73.33% 74.55% +1.22%
==========================================
Files 13 13
Lines 645 672 +27
Branches 99 101 +2
==========================================
+ Hits 473 501 +28
+ Misses 143 142 -1
Partials 29 29
Continue to review full report at Codecov.
|
After reviewing #367 I realized that this code could be more easily
tested if it was broken out from the PackageFile object. Since it was
such a tiny refactor, I went ahead, refactored it and added the
necessary tests to ensure this just works.
cc @dralley