[wp-trac] [WordPress Trac] #47223: Site Health Check: "last missed cron" test too aggressive

[WordPress Trac]

#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