Test fixes #37
Test fixes #37
Conversation
tests/test.py
Outdated
| def test_fileish_types(self): | ||
| """Test the constructor does the right thing when given different types""" | ||
| with FitFile(testfile('Settings.FIT')): | ||
| with FitFile(testfile('Settings.fit')): |
There was a problem hiding this comment.
This was my bad. The uppercase here was intentional to trigger an issue where .FIT in filename was confused with .FIT in the file header. I thought I had added a Settings.FIT file alongside Settings.fit, but I guess it didn't get added to git properly (probably my filesystem's fault).
Would you be able to rename the file to Settings.FIT and change all the other instances of Settings.fit in the tests to Settings.FIT? Alternatively just add another file with an 8-character filename and an uppercase extension (nametest.FIT?) and sub that in here. Should add a comment too since I didn't really explain the test very well.
There was a problem hiding this comment.
added a nametest.FIT and improved documentation.
tests/test.py
Outdated
| pass | ||
| with FitFile(io.BytesIO(open(testfile("Settings.fit"), 'rb').read())): | ||
| pass | ||
| f = open(testfile("Settings.fit"), 'rb') |
There was a problem hiding this comment.
Good catch, would prefer still using a context manager for the open/closing instead of an f.close(). Something like:
with open(testfile("Settings.fit"), 'rb') as f:
FitFile(f)
with open(testfile("Settings.fit"), 'rb') as f:
FitFile(f.read())
with open(testfile("Settings.fit"), 'rb') as f:
FitFile(io.BytesIO(f.read()))There was a problem hiding this comment.
of course, you're right. fixed.
There was a naming problem on case-insensitive filesystems, which is why the tests refered to both 'Settings.FIT' and 'Settings.FIT', with the latter not inside git, and tests failing on all case-sensitive filesystems. Fix this by duplicating 'Settings.fit' to 'nametest.FIT', and using it inside `test_fileish_types(self)`. Also improve test documentation a bit, while adding a link to the underlying issue.
|
addressed all comments and rebased on |
|
Perfect, thanks! |
2 participants
No description provided.