Only delete files that were successfully downloaded #625
Conversation
|
@jfsimoneau do you have an idea what tests can a good indicator that new functionality is fixing the issue and working correctly? That would be very helpful for maintaining the app going forward. |
|
@AndreyNikiforov I admit I did not take the time to see how we could simulate a failed download in the unit tests. I will take a look later today. |
|
I added a test with a failed download. This should fix #614. |
tests/test_download_photos.py
Outdated
| f"INFO Downloading {os.path.join(base_dir, os.path.normpath('2018/07/31/IMG_7409.JPG'))}", | ||
| self._caplog.text, | ||
| ) | ||
| self.assertNotIn( |
There was a problem hiding this comment.
Is there a way to confirm skipping deletion call without/in addition to relying on logs? It would be beneficial to separate logic check from logging check, so we can change one without breaking tests for the other.
Note the most test still combine log and state/file/api checks and I hope that can be changes over time too.
There was a problem hiding this comment.
I wanted to mock the internal delete_photo() function to see if it was called, but it seems it's not accessible. The best way I found to make that tests is to craft a cassette to simulate a bad request. Because of that bad request, the delete is never called so the cassette is complete.
AndreyNikiforov
merged commit 3ed662d
into
icloud-photos-downloader:master
No description provided.