[wp-trac] [WordPress Trac] #47223: Site Health Check: "last missed cron" test too aggressive
WordPress Trac
noreply at wordpress.org
Tue Aug 6 00:51:42 UTC 2019
#47223: Site Health Check: "last missed cron" test too aggressive
-------------------------------------------------+-------------------------
Reporter: DavidAnderson | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: Future
| Release
Component: Site Health | Version: 5.2
Severity: normal | Resolution:
Keywords: site-health needs-testing has-patch | Focuses:
-------------------------------------------------+-------------------------
Comment (by afragen):
@peterwilsoncc
> My rough thoughts/questions are:
> * Are $timeout_late_cron and $timeout_missed_cron required as
properties or can they be defined in the functions
It seems that `$timeout_late_cron` could be defined in the function
`has_late_cron()` but `$timeout_missed_cron` is used in `has_late_cron()`
and `has_missed_cron()`.
> * Unit tests would be dandy, keep in mind DISABLE_WP_CRON === true in
the test suite, affecting the timing rules
Yes they would be. Alas, I've never actually written one 😔
> * I'm not going to add/change constants, are filters required for the
timing rules?
I don't really think constants or filters are necessary for this module
specific to the Site Health page. If the admin is sufficiently capable
it's simple enough to just disable the test and prevent it from showing by
using the `add_filter( 'site_status_tests' ... )` and filter out
`scheduled_events` from the array.
Also,
* Must we test for `true === DISABLE_WP_CRON`? I thought the value was a
boolean if defined. If so the patch should be updated in the constructor.
* Isn't the WPCS `elseif` and not `else if`?
* Couldn't the conditionals in `get_test_scheduled_events()` be changed
from
{{{
if ( is_wp_error( $this->has_missed_cron() ) ) {
...
} else {
if( $this->has_missed_cron() ) {
...
} else if ( $this->has_late_cron() {
...
}
}}}
to
{{{
if ( is_wp_error( $this->has_missed_cron() ) ) {
...
} elseif( $this->has_missed_cron() ) {
...
} elseif ( $this->has_late_cron() {
...
}
}}}
I've added a diff with my thoughts. Please ensure my logic with the
restructured conditional.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/47223#comment:23>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list