Create Abstract_Check_Runner class

[jjgrainger]

Description

The Abstract_Check_Runner class will contain common logic used to run Checks across both CLI and AJAX. The abstract class will be used to build out the following runner classes:

• Create CLI_Runner class #14
• Create AJAX_Runner class #15

Acceptance Criteria

• A new Abstract_Check_Runner class will exist in includes/Checker.
• The class will implement the Check_Runner interface.
• The class has a protected $checks property.
• The class has a protected $checks_to_run property.
• The class has an abstract is_plugin_check() method.
• The class has an abstract protected setup_checks() method.
• The class contains a prepare() method.
  • The prepare() method accepts no parameters.
  • The prepare() method is used to run any universal preparations early in the WordPress load.
    • The array of $checks_to_run array is passed to the requires_universal_preparations() method (Create Abstract_Check_Runner class #63).
    • If the method returns true the Universal_Runtime_Preparation will be used.
    • The prepare method will instantiate the Universal_Runtime_Preparation (Create a Universal_Runtime_Preparation #61) and call the prepare() method.
    • The cleanup function returned by the prepare() method is then returned.
• The class has a protected requires_universal_preparations( array $checks ) method.
  • The method accepts an array of Check instances that will be run.
  • The method loops over the Check instances and tests to see if any Checks implement the Runtime_Check interface.
  • If any Check implements the Runtime_Check interface, the method returns true, if not, the method returns false.
• The class implements a private get_shared_preparations( array $checks ) method.
  • The method accepts an array of Check instances that will be run.
  • The method assigns an empty array to a $preparations variable.
  • The method loops of the Check instants and tests for Checks that implement the With_Shared_Preparations interface.
  • If the Check implements the With_Shared_Preparations interface, the Checks get_shared_preparations method is called to return an array of preparations.
  • The preparations returned are checked for any duplicates by evaluating the preparation name and the array of constructor arguments to see if these match any existing shared preparations in the $preparations array.
  • These preparations are merged into the $preparations variable and returned by the method.
• The class implements the run() method.
  • The run method will gather the Check instances from $this->checks->get_checks() using the requested checks in the $checks_to_run property.
  • The get_shared_preparations is called passing the $checks_to_run array. The returned shared preparations are then looped over calling each preparations prepare() method. The preparations cleanup functions are then stored to a $cleanups variable.
  • The Checks are then run by passing the $checks_to_run array is then passed to $this->checks->run_checks() and the Check_Results instanced returned stored as a variable.
  • If the get_shared_preparations preparations where prepared the $cleanups functions returned are called.
  • The run method returns the $check_results instance.

[jjgrainger]

Hi @felixarntz

I've created this Abstract_Check_Runner class that would be used to hold to common functionality for both the CLI_Runner and AJAX_Runner. While working on this I've had thoughts on the wider AC's which I've listed below.

• Based on our discussions the 'orchestration' of checks is basic for the initial version of the plugin checker with the intention that I would update Check Orchestration and Execution Approach #37 with details on making this smarter in the future.
• While writing these AC's I don't see much difference in how both the CLI_Runner and AJAX_Runner would work differently and leaning towards only needing a single Check_Runner class to orchestrate and run checks that are requested.
• When running checks via AJAX in the WP Admin, I'm thinking that the state of the checks running will be managed client side and would send a request for each Check to be run and store the results response to display later.
• For CLI, we would run all checks requested to run in a single process.
• So I'm struggling to see a need to have both a CLI_Runner and an AJAX_Runner and thinking we would only need a single Check_Runner class.
• It could be argued that the AJAX_Runner would also register hooks to WordPress to handle the AJAX requests, but seeing as the CLI command is handled in its own class I imagine the admin pages and hooks for AJAX would also be handled in a separate class.

Let me know your thoughts here and I can review/update the AC's next week.

Thanks

[felixarntz]

@jjgrainger I think the logic that is outlined here overall makes sense, but some of the methods and where/how it is handled may be a bit off based on my feedback on #13. Let's finalize the ACs on that issue first, then based on those we can iterate here accordingly.

[jjgrainger]

Thanks @felixarntz I've updated the AC's based on our discussions. Please review and assign back to me.

[felixarntz]

@jjgrainger See my feedback on #13, I'd still advise to wait for having those ACs finalized since these ones here directly depend on them. See my feedback on the setup() method there.

A bit of feedback on what's already here:

• How does the $checks_to_run property get populated? How do we know which checks to run? Note that this needs to somehow depend on the CLI / AJAX specific runner classes since the detection for that will work differently.
• On the get_shared_preparations() method:
  • You may want to document that it should eliminate duplicates, e.g. if multiple checks return the same shared preparation.
  • We should make explicitly this method private so that it cannot be called from elsewhere, not even child classes. This way we ensure that we can change it later without breaking backward compatibility, which we probably need to do once we want to implement smarter "preparation orchestration".
• The term "preparation" is a few times misspelled as "preperation" - not a big deal to have typos, we just have to make sure they don't make it into the struct names in the code, so probably best to fix here already.

Small general advise: I think it's usually overkill to mention specific class properties the class should have, that becomes extremely granular in terms of ACs, and thus may result in additional iterations on a detail that we could rather figure out during code review. I think focusing on public methods and a few critical protected or private helpers is sufficient for AC level of detail.