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

add new thumbnail flag #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

framirez90
Copy link

I added a new feature to only be able download the first photo of certain album as as it shown on the device.

@menkej
Copy link
Collaborator

menkej commented May 28, 2021

Thanks for your pull request! What is your usecase for this?

Could you please also add testcases for it?

@framirez90
Copy link
Author

Hi @menkej

I want to retrieve only the photo as it shown on album cover on IOS device.

I added a new TC for it.

@menkej menkej self-requested a review June 1, 2021 17:32
Copy link
Collaborator

@menkej menkej left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please have another look at the test case and also add your change to the CHANGELOG. Thanks!

runner = CliRunner(env={
"CLIENT_ID": "DE309E26-942E-11E8-92F5-14109FE0B321"
})
result = runner.invoke(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are running the script but don't have any assertions/tests. I suggest checking for the pictures you are expecting to be downloaded. Additionally you should do a negative test, p.e. run without your new parameter (so that more pictures are downloaded) and check that the second picture that would come is not been downloaded when run with --thumbnail.

shutil.rmtree(base_dir)
os.makedirs(base_dir)

with vcr.use_cassette("tests/vcr_cassettes/listing_photos.yml"):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best would be to add a new cassette for this test case. Creating a new cassette is pretty easy. Just use a new filename and run the test again. Make sure to remove personal data from the cassette before you check in.

@@ -52,6 +52,9 @@ Options:
Download live photos)
--force-size Only download the requested size (default:
download original if size is not available)
--thumbnail Only download the first photo as thumbnail
of album as it is shown in the device
(-a, --album option is required)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is -a required?

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