Skip to content

Fix tests to account for current core behavior #169

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.

Sign up for GitHub

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

Jump to bottom
Closed
wojtekn wants to merge 1 commit into from
Closed

Fix tests to account for current core behavior #169

wojtekn wants to merge 1 commit into from

Conversation

@wojtekn
Copy link
Contributor

@wojtekn wojtekn commented Dec 15, 2022

In this PR, I propose to fix the failing tests. It looks like the change was introduced in 3b25705 .

I think that at that time, there was a bug in the WP core that resulted in saving 150x150 thumbnails for 150x150 images. We modified tests in the commit shown above to ensure they check the current core behavior, even if it was considered buggy.

However, I'm unsure about the exact timing of those events, as tests for the older WP version were marked as skipped in Aug 2019 (#112), tests were modified for the newer WP version in 2021 (3b25705), but the bug in the core was fixed in Sep 2019 (https://core.trac.wordpress.org/ticket/32437)?

@wojtekn wojtekn requested a review from a team as a code owner December 15, 2022 09:27
@wojtekn wojtekn mentioned this pull request Dec 15, 2022
Closed
2 tasks
@danielbachhuber
Copy link
Member

danielbachhuber commented Dec 16, 2022

Interestingly, this passes for me locally...

@wojtekn
Copy link
Contributor Author

wojtekn commented Dec 19, 2022

@danielbachhuber could you please check tests for the main as well? For me, tests for "Regenerate all images default behavior" scenario fail locally on main, and succeed with the fix from this branch applied. On our build server, it works for main, but not for this PR.

It seems it's something environment-related, but I couldn't find the root cause yet.

@danielbachhuber
Copy link
Member

danielbachhuber commented Dec 19, 2022

could you please check tests for the main as well? For me, tests for "Regenerate all images default behavior" scenario fail locally on main, and succeed with the fix from this branch applied. On our build server, it works for main, but not for this PR.

It seems it's something environment-related, but I couldn't find the root cause yet.

@wojtekn Yep, main is broken for me locally too. I agree that it's something environment-related, and am not sure what it might be.

@danielbachhuber
Copy link
Member

danielbachhuber commented Dec 21, 2022

It looks like the change was introduced in 3b25705 .

For reference, the associated PR is #140

@danielbachhuber danielbachhuber mentioned this pull request Dec 21, 2022
Closed
@danielbachhuber
Copy link
Member

danielbachhuber commented Dec 21, 2022

@wojtekn I'm still flummoxed as to why white-150-square-150x150.jpg is created in the GitHub Actions environment, and isn't created locally 😞

@schlessera @swissspidy Any ideas on what might be going on?

I added a couple of debug scenarios with #173 to see if we can get more visibility.

@danielbachhuber
Copy link
Member

danielbachhuber commented Dec 21, 2022

@wojtekn Found it! 😁 https://core.trac.wordpress.org/ticket/57370

@danielbachhuber danielbachhuber mentioned this pull request Jan 16, 2023
Closed
@danielbachhuber
Copy link
Member

danielbachhuber commented Jan 16, 2023

@wojtekn Closing this in favor of #174

Thanks again for your work on it!

@wojtekn wojtekn deleted the fix/media-regenerate-tests-to-account-for-150-thumbnail branch January 16, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@wojtekn wojtekn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wojtekn @danielbachhuber