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 unit tests #35

Merged
merged 46 commits into from Mar 27, 2021
Merged

Add unit tests #35

merged 46 commits into from Mar 27, 2021

Conversation

GjjvdBurg
Copy link
Owner

This PR aims to add some unit tests to this program. It's been quite some time since I've actually worked on this and I want to make sure it's robust before we start adding in the suggestions in #31 and #32.

@sebastiaan-lampo
Copy link
Contributor

Gertjan,

given this excellent idea of adding more tests I've held off on adding anything more PRs or editing. Let me know how you'd like me to proceed.

I have almost completed my goal of having HPS work straight from photo management software to website on my own fork. In the process I have had to revisit #29 because I ran into memory issues again. This is something that unittest already pointed out to me when I developed #36. In short I replaced img = Image.open(self.original_path) by with Image.open(self.original_path) as img which ensured the files were being closed again.

As a milestone, HPS sucessfully generated a hugo website with over 1200 photos in nested directories. Largest single album was 120 photos.

I'll be happy to submit/amend integrated or separate PRs against any branch as per your directions. If you'd like to discuss on email or whatsapp you can find me at sebastiaan.lampo + google's email domain. We kunnen de conversatie ook in het Nederlands voeren.

@GjjvdBurg
Copy link
Owner Author

Hi @sebastiaan-lampo,

Thanks for your message. I've spent some time on the unit test PR this weekend, but haven't finished it yet. I hope to integrate your other work in due course but working on this project is not a major priority for me at the moment, as you'll hopefully understand. It's great to hear though that you've managed to use HPS successfully on such a large gallery!

I'll take your comment on the memory issues into account, thanks for that suggestion.

To be continued!

@sebastiaan-lampo
Copy link
Contributor

Totally understand, no rush. In the meantime I'll progress on my fork and make it as robust as possible and be ready to format the PRs however is easiest for you.

@GjjvdBurg
Copy link
Owner Author

Sounds like a great plan, thanks for understanding!

@GjjvdBurg GjjvdBurg marked this pull request as ready for review March 23, 2021 00:09
@GjjvdBurg
Copy link
Owner Author

Hi @sebastiaan-lampo, I think I'm done adding unit tests for now. Do you have any comments on this that I should be aware of before merging? Thanks!

@GjjvdBurg GjjvdBurg merged commit 16707cc into master Mar 27, 2021
@sebastiaan-lampo
Copy link
Contributor

Sorry Gertjan, life took over with a need to find a new home in the next two months. I'll have a look at the new tests and see what others are needed for my additional features.

@GjjvdBurg
Copy link
Owner Author

No worries Sebastiaan, there's no rush. I think most of my changes are minor, so it shouldn't be too much trouble to rebase/merge master into your branches. If there's anything that's unclear, please let me know.

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.

None yet

2 participants