![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png){kind=logo}
Opened 7 years ago
Last modified 3 months ago
#47428 new defect (bug)
Site Health: Remove duplicate code in VCS test
| Reported by: | desrosj | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | Future Release |
| Component: | Site Health | Version: | 5.2 |
| Severity: | normal | Keywords: | site-health has-patch has-unit-tests |
| Cc: | Focuses: |
Description
In WP_Site_Health_Auto_Updates::test_vcs_abspath(), almost the entire WP_Upgrader::is_vcs_checkout() function is duplicated. This test should be updated to use WP_Upgrader::is_vcs_checkout() instead. There are a few issues with this that would need to be solved, though.
In the message to the user, the directory under version control and the type of version control used are specified. This currently can't be determined without duplicating the logic. Also, in order to determine if the automatic_updates_is_vcs_checkout filter is overriding the function's result to allow updates, the filter would need to be run twice.
Props @johnbillion for the suggestion in #47388.
Change History (5)
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
7 years ago
#4
@
7 years ago
Avoiding duplication here would definitely be ideal, I'm not sure how we should approach the file/folder declarations though, as I view them as quite vital in the Site Health Check context, while it doesn't matter what is under VCS for WP_Upgrader.
Any thoughts on the best approach here?
Perhaps if WP_Upgrader::is_vcs_checkout() detects it being under version control, we can do a scan for .svn, .git, .hg and .bzr and pick up on their locations, RecursiveDirectoryIterator should be fast enough when given a rigid pattern like that to work with?
This ticket was mentioned in PR #11495 on WordPress/wordpress-develop by @iampi20.
3 months ago
#5
- Keywords has-patch has-unit-tests added
## Summary
Removes duplicated VCS checkout detection between Site Health and the automatic updater by centralizing the filesystem walk and .svn / .git / .hg / .bzr checks.
### Changes
- Adds
WP_Automatic_Updater::get_vcs_checkout_details()to return whether a VCS metadata directory was found, which ancestor path matched, and which marker (e.g..git) was used—before theautomatic_updates_is_vcs_checkoutfilter is applied. - Refactors
WP_Automatic_Updater::is_vcs_checkout()to callget_vcs_checkout_details()and then apply the filter to the raw boolean, preserving existing behavior. - Updates
WP_Site_Health_Auto_Updates::test_vcs_abspath()to useget_vcs_checkout_details( ABSPATH )for the user-facing messages and appliesautomatic_updates_is_vcs_checkoutonce with the raw discovery result (equivalent to the previous logic when a checkout was detected). - Extends PHPUnit coverage in
tests/phpunit/tests/admin/wpAutomaticUpdater.phpfor the new helper and consistency between raw discovery and the filteredis_vcs_checkout()result.
Trac ticket: https://core.trac.wordpress.org/ticket/47428
Moving Site Health tickets into their lovely new home, the Site Health component.