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

Don't update translations asynchronously during tests #683

Closed

Conversation

ocean90
Copy link
Member

@ocean90 ocean90 commented Nov 3, 2020

The tests for WP_REST_Plugins_Controller::create_item() in WP_REST_Plugins_Controller_Test are doing real api.w.org requests which will also return data about outdated translations. If the tests are run again, Language_Pack_Upgrader::sync_upgrade() will now update the translations based on the previous data.

Trac ticket: https://core.trac.wordpress.org/ticket/51670


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@desrosj
Copy link
Contributor

desrosj commented Nov 3, 2020

This looks good to me, and I also think I know what caused this to start happening all of a sudden.

In the commit in question, I also expanded the list of excluded files when packaging the ZIP file during the setup job to include .git*, and packagehash.txt. If packagehash.txt was present on the actual test jobs, it would prevent the npx install-changed command from running the install command. I added the .git* exclusion to prevent the .git folder from also being included.

The code right before apply_filters( 'async_update_translation' ) runs the is_vcs_checkout() check and returns early if the site appears to be under version control. When the .git folder was excluded from the ZIP file, the follow up jobs in the workflow that run the tests would no longer assume that the install is under version control, and the language packs would be updated.

It's not clear to me whether running the tests with the assumption that the install is under version control is preferred, though. If it is, we should probably override the is_vcs_checkout() function with the automatic_updates_is_vcs_checkout filter in the test suite itself to always guarantee the site will be viewed as under VCS.

@TimothyBJacobs
Copy link
Member

TimothyBJacobs commented Nov 3, 2020

I thought it was blocked because that is only hooked in the admin? https://core.trac.wordpress.org/ticket/50732#comment:17

Ignore me, I'm an idiot.

@johnbillion johnbillion closed this Nov 4, 2020
@ocean90 ocean90 deleted the fix/translation-updates-51670 branch November 22, 2022 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants