Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test compatibility for Windows #102

Merged
merged 6 commits into from
Mar 18, 2021

Conversation

unapproachable
Copy link
Contributor

Fix for #101

Okay, this was far more complicated that I initially imagined. I had to borrow from a couple of different ideas from old versions of the code. Opted to branch the execution of exiftool under win32 as a quoted string instead of using shlex.quote because it does more to mangle the names on Windows (see below) than it does to fix them. Non-win32 functionality should remain as before. Also renamed the test image to something acceptable on win32 platform as a valid filename.

C:\>python
Python 3.8.6 (tags/v3.8.6:db45529, Sep 23 2020, 15:52:53) [MSC v.1927 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import shlex
>>> s = shlex.quote("exif's.jpg")
>>> str(s)
'\'exif\'"\'"\'s.jpg\''

Also added a second test to verify the more common scenario of users adding spaces and punctuation to their image filenames.

--Added additional test image
--Corrected failing trailing slash test with Windows specific test

Test run on my Windows machine:

P:\Projects\phockup>pytest
=============================================== test session starts ================================================
platform win32 -- Python 3.8.6, pytest-6.2.2, py-1.10.0, pluggy-0.13.1
rootdir: P:\Projects\phockup
plugins: hypothesis-6.3.4, mock-3.5.1
collected 49 items

tests\test_date.py ................                                                                           [ 32%]
tests\test_exif.py ....                                                                                       [ 40%]
tests\test_phockup.py .............................                                                           [100%]

=============================================== 49 passed in 15.35s ================================================

@ivandokov ivandokov merged commit cfb79d5 into ivandokov:master Mar 18, 2021
@unapproachable unapproachable deleted the feature/test-fix branch March 22, 2021 12:52
unapproachable added a commit to unapproachable/phockup that referenced this pull request Mar 25, 2021
* Migrated to a more compatible filename for testing exiftool

* --Migrated Windows escaping to quoted input rather than shlex.quote escaping.
--Added space and punctuation test
--Added additional test image

* --Updated trailing slash test to use Windows specific slashes on win32

* --Updated trailing slash test to use Windows specific slashes on win32

* --revert unintended commit
ivandokov added a commit that referenced this pull request Mar 30, 2021
)

* Removed for compatibility with Windows

* Implemented --maxdepth to limit recursion on input directory
Arbitrary limit of 255 set for command line option
#92

* Test compatibility for Windows (#102)

* Migrated to a more compatible filename for testing exiftool

* --Migrated Windows escaping to quoted input rather than shlex.quote escaping.
--Added space and punctuation test
--Added additional test image

* --Updated trailing slash test to use Windows specific slashes on win32

* --Updated trailing slash test to use Windows specific slashes on win32

* --revert unintended commit

* Added quiet mode to not show output. (#103)

* Arbitrary text change to help to test commits

* Updated readme.md with maxdepth details
Updated defaults to support --maxdepth=0 to allow processsing only the input directory without any subdirectory traversal.

Co-authored-by: roykrikke <[email protected]>
Co-authored-by: Ivan Dokov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants