Create AJAX_Runner#79
Conversation
jjgrainger
added
[Type] Enhancement
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@jjgrainger Overall this looks solid, left a few small points of feedback.
|
Thanks @felixarntz I've made the updates based on your feedback. |
felixarntz
left a comment
felixarntz
left a comment
There was a problem hiding this comment.
@jjgrainger Basically LGTM, just a few nit-picks. I can address them really quickly to unblock.
|
@joemcgill Can you give this a quick sanity check and 2nd approval (just to unblock because Mukesh is traveling this week)? |
joemcgill
left a comment
joemcgill
left a comment
There was a problem hiding this comment.
A couple questions, but this all looks good to me.
| // Get the plugin name from the AJAX request. | ||
| $plugin_file = Plugin_Request_Utility::get_plugin_basename_from_input( $_REQUEST['plugin'] ); | ||
|
|
||
| $this->checks = new Checks( WP_PLUGIN_DIR . '/' . $plugin_file ); |
There was a problem hiding this comment.
This assumes that the plugin is always being loaded from inside the plugins directory rather than as an mu-plugin or required via code, as is often the case during unit testing setups, or some platform configurations. Is that an intentional limitation at this point?
There was a problem hiding this comment.
Thanks @joemcgill thats a great point and something we should probably look into in the future. For this initial milestone I believe the intention is to only check plugins found within the main plugin directory.
| use WordPress\Plugin_Check\Checker\AJAX_Runner; | ||
| use WordPress\Plugin_Check\Checker\Check_Result; | ||
|
|
||
| class AJAX_Runner_Tests extends WP_UnitTestCase { |
There was a problem hiding this comment.
Just an observation, but since all of these are filtering the wp_doing_ajax value, is there any value in moving this shared code to a fixture that runs before each test? I also wonder if there's any value in having a test case that confirms the expected behavior if wp_doing_ajax is false?
There was a problem hiding this comment.
Thanks @joemcgill I've added an additional test to check behaviour when wp_doing_ajax is false. We can potentially refactor the tests here in future to reduce duplicate code here.
3 participants
Adds the
AJAX_Runnerclass.Closes #15