From cf18b840a8d9a207201bea79bb7358707249b8ca Mon Sep 17 00:00:00 2001 From: Caleb Stauffer Date: Fri, 4 Oct 2024 12:57:35 -0400 Subject: [PATCH 1/4] exclude sniffs --- phpcs.xml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/phpcs.xml b/phpcs.xml index 0e3a1ee6c..d37f87c98 100644 --- a/phpcs.xml +++ b/phpcs.xml @@ -43,4 +43,12 @@ lib/* tests/* + + + tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php + + + + tests/* + From f97745b2584dcacba17b0d7b2fad15c26bc53cd1 Mon Sep 17 00:00:00 2001 From: Caleb Stauffer Date: Fri, 4 Oct 2024 12:57:43 -0400 Subject: [PATCH 2/4] eliminate code smells --- .../ActionScheduler_wpCommentLogger_Test.php | 165 ++++++++++-------- 1 file changed, 91 insertions(+), 74 deletions(-) diff --git a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php index d2f3fccc1..cf7cd8d5f 100644 --- a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php +++ b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php @@ -5,6 +5,8 @@ * @package test_cases\logging */ class ActionScheduler_wpCommentLogger_Test extends ActionScheduler_UnitTestCase { + + /** @var bool */ private $use_comment_logger; public function test_default_logger() { @@ -19,10 +21,10 @@ public function test_default_logger() { public function test_add_log_entry() { $action_id = as_schedule_single_action( time(), 'a hook' ); - $logger = ActionScheduler::logger(); - $message = 'Logging that something happened'; - $log_id = $logger->log( $action_id, $message ); - $entry = $logger->get_entry( $log_id ); + $logger = ActionScheduler::logger(); + $message = 'Logging that something happened'; + $log_id = $logger->log( $action_id, $message ); + $entry = $logger->get_entry( $log_id ); $this->assertEquals( $action_id, $entry->get_action_id() ); $this->assertEquals( $message, $entry->get_message() ); @@ -39,41 +41,51 @@ public function test_add_log_datetime() { $this->assertEquals( $action_id, $entry->get_action_id() ); $this->assertEquals( $message, $entry->get_message() ); - $date = new ActionScheduler_DateTime( 'now', new DateTimeZone( 'UTC' ) ); - $log_id = $logger->log( $action_id, $message, $date ); - $entry = $logger->get_entry( $log_id ); + $date = new ActionScheduler_DateTime( 'now', new DateTimeZone( 'UTC' ) ); + $log_id = $logger->log( $action_id, $message, $date ); + $entry = $logger->get_entry( $log_id ); $this->assertEquals( $action_id, $entry->get_action_id() ); $this->assertEquals( $message, $entry->get_message() ); } public function test_erroneous_entry_id() { - $comment = wp_insert_comment(array( - 'comment_post_ID' => 1, - 'comment_author' => 'test', - 'comment_content' => 'this is not a log entry', - )); + $comment = wp_insert_comment( + array( + 'comment_post_ID' => 1, + 'comment_author' => 'test', + 'comment_content' => 'this is not a log entry', + ) + ); + $logger = ActionScheduler::logger(); - $entry = $logger->get_entry( $comment ); + $entry = $logger->get_entry( $comment ); + $this->assertEquals( '', $entry->get_action_id() ); $this->assertEquals( '', $entry->get_message() ); } public function test_storage_comments() { $action_id = as_schedule_single_action( time(), 'a hook' ); - $logger = ActionScheduler::logger(); - $logs = $logger->get_logs( $action_id ); - $expected = new ActionScheduler_LogEntry( $action_id, 'action created' ); - $this->assertTrue( in_array( $this->log_entry_to_array( $expected ) , $this->log_entry_to_array( $logs ) ) ); + $logger = ActionScheduler::logger(); + $logs = $logger->get_logs( $action_id ); + $expected = new ActionScheduler_LogEntry( $action_id, 'action created' ); + $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ), true ) ); } protected function log_entry_to_array( $logs ) { if ( $logs instanceof ActionScheduler_LogEntry ) { - return array( 'action_id' => $logs->get_action_id(), 'message' => $logs->get_message() ); + return array( + 'action_id' => $logs->get_action_id(), + 'message' => $logs->get_message(), + ); } - foreach ( $logs as $id => $log) { - $logs[ $id ] = array( 'action_id' => $log->get_action_id(), 'message' => $log->get_message() ); + foreach ( $logs as $id => $log ) { + $logs[ $id ] = array( + 'action_id' => $log->get_action_id(), + 'message' => $log->get_message(), + ); } return $logs; @@ -81,67 +93,70 @@ protected function log_entry_to_array( $logs ) { public function test_execution_comments() { $action_id = as_schedule_single_action( time(), ActionScheduler_Callbacks::HOOK_WITH_CALLBACK ); - $logger = ActionScheduler::logger(); - $started = new ActionScheduler_LogEntry( $action_id, 'action started via Unit Tests' ); - $finished = new ActionScheduler_LogEntry( $action_id, 'action complete via Unit Tests' ); + $logger = ActionScheduler::logger(); + $started = new ActionScheduler_LogEntry( $action_id, 'action started via Unit Tests' ); + $finished = new ActionScheduler_LogEntry( $action_id, 'action complete via Unit Tests' ); $runner = ActionScheduler_Mocker::get_queue_runner(); $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); - $this->assertTrue( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ), true ) ); } public function test_failed_execution_comments() { - $hook = md5(rand()); - add_action( $hook, array( $this, '_a_hook_callback_that_throws_an_exception' ) ); + $hook = md5( wp_rand() ); + add_action( $hook, array( $this, 'a_hook_callback_that_throws_an_exception' ) ); + $action_id = as_schedule_single_action( time(), $hook ); - $logger = ActionScheduler::logger(); - $started = new ActionScheduler_LogEntry( $action_id, 'action started via Unit Tests' ); - $finished = new ActionScheduler_LogEntry( $action_id, 'action complete via Unit Tests' ); - $failed = new ActionScheduler_LogEntry( $action_id, 'action failed via Unit Tests: Execution failed' ); + $logger = ActionScheduler::logger(); + $started = new ActionScheduler_LogEntry( $action_id, 'action started via Unit Tests' ); + $finished = new ActionScheduler_LogEntry( $action_id, 'action complete via Unit Tests' ); + $failed = new ActionScheduler_LogEntry( $action_id, 'action failed via Unit Tests: Execution failed' ); $runner = ActionScheduler_Mocker::get_queue_runner(); $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); - $this->assertFalse( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); - $this->assertTrue( in_array( $this->log_entry_to_array( $failed ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertFalse( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $failed ), $this->log_entry_to_array( $logs ), true ) ); } public function test_failed_schedule_next_instance_comments() { - $action_id = rand(); + $action_id = wp_rand(); $logger = ActionScheduler::logger(); $log_entry = new ActionScheduler_LogEntry( $action_id, 'There was a failure scheduling the next instance of this action: Execution failed' ); try { - $this->_a_hook_callback_that_throws_an_exception(); + $this->a_hook_callback_that_throws_an_exception(); } catch ( Exception $e ) { do_action( 'action_scheduler_failed_to_schedule_next_instance', $action_id, $e, new ActionScheduler_Action( ActionScheduler_Callbacks::HOOK_WITH_CALLBACK ) ); } $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $log_entry ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $log_entry ), $this->log_entry_to_array( $logs ), true ) ); } public function test_fatal_error_comments() { - $hook = md5(rand()); + $hook = md5( wp_rand() ); $action_id = as_schedule_single_action( time(), $hook ); - $logger = ActionScheduler::logger(); - do_action( 'action_scheduler_unexpected_shutdown', $action_id, array( - 'type' => E_ERROR, + $logger = ActionScheduler::logger(); + $args = array( + 'type' => E_ERROR, 'message' => 'Test error', - 'file' => __FILE__, - 'line' => __LINE__, - )); + 'file' => __FILE__, + 'line' => __LINE__, + ); - $logs = $logger->get_logs( $action_id ); - $found_log = FALSE; + do_action( 'action_scheduler_unexpected_shutdown', $action_id, $args ); + + $logs = $logger->get_logs( $action_id ); + $found_log = false; foreach ( $logs as $l ) { if ( strpos( $l->get_message(), 'unexpected shutdown' ) === 0 ) { - $found_log = TRUE; + $found_log = true; } } $this->assertTrue( $found_log, 'Unexpected shutdown log not found' ); @@ -150,14 +165,15 @@ public function test_fatal_error_comments() { public function test_canceled_action_comments() { $action_id = as_schedule_single_action( time(), 'a hook' ); as_unschedule_action( 'a hook' ); - $logger = ActionScheduler::logger(); - $logs = $logger->get_logs( $action_id ); + + $logger = ActionScheduler::logger(); + $logs = $logger->get_logs( $action_id ); $expected = new ActionScheduler_LogEntry( $action_id, 'action canceled' ); - $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ), true ) ); } - public function _a_hook_callback_that_throws_an_exception() { - throw new RuntimeException('Execution failed'); + public function a_hook_callback_that_throws_an_exception() { + throw new RuntimeException( 'Execution failed' ); } public function test_filtering_of_get_comments() { @@ -166,43 +182,44 @@ public function test_filtering_of_get_comments() { return; } - $post_id = $this->factory->post->create_object(array( - 'post_title' => __FUNCTION__, - )); - $comment_id = $this->factory->comment->create_object(array( - 'comment_post_ID' => $post_id, - 'comment_author' => __CLASS__, - 'comment_content' => __FUNCTION__, - )); + $post_id = $this->factory->post->create_object( array( 'post_title' => __FUNCTION__ ) ); + $comment_id = $this->factory->comment->create_object( + array( + 'comment_post_ID' => $post_id, + 'comment_author' => __CLASS__, + 'comment_content' => __FUNCTION__, + ) + ); - // Verify that we're getting the expected comment before we add logging comments + // Verify that we're getting the expected comment before we add logging comments. $comments = get_comments(); $this->assertCount( 1, $comments ); $this->assertEquals( $comment_id, $comments[0]->comment_ID ); - $action_id = as_schedule_single_action( time(), 'a hook' ); - $logger = ActionScheduler::logger(); - $message = 'Logging that something happened'; - $log_id = $logger->log( $action_id, $message ); - + $logger = ActionScheduler::logger(); + $message = 'Logging that something happened'; + $log_id = $logger->log( $action_id, $message ); - // Verify that logging comments are excluded from general comment queries + // Verify that logging comments are excluded from general comment queries. $comments = get_comments(); $this->assertCount( 1, $comments ); $this->assertEquals( $comment_id, $comments[0]->comment_ID ); - // Verify that logging comments are returned when asking for them specifically - $comments = get_comments(array( - 'type' => ActionScheduler_wpCommentLogger::TYPE, - )); - // Expecting two: one when the action is created, another when we added our custom log + // Verify that logging comments are returned when asking for them specifically. + $comments = get_comments( + array( + 'type' => ActionScheduler_wpCommentLogger::TYPE, + ) + ); + + // Expecting two: one when the action is created, another when we added our custom log. $this->assertCount( 2, $comments ); - $this->assertContains( $log_id, wp_list_pluck($comments, 'comment_ID')); + $this->assertContains( $log_id, wp_list_pluck( $comments, 'comment_ID' ) ); } private function using_comment_logger() { - if ( null === $this->use_comment_logger ) { + if ( is_null( $this->use_comment_logger ) ) { $this->use_comment_logger = ! ActionScheduler_DataController::dependencies_met(); } From 5c63142d7f390ae37e654c4750b3262dcdd23c58 Mon Sep 17 00:00:00 2001 From: Caleb Stauffer Date: Fri, 4 Oct 2024 14:37:11 -0400 Subject: [PATCH 3/4] use loose comparison, and disable sniff --- .../ActionScheduler_wpCommentLogger_Test.php | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php index cf7cd8d5f..e6704d200 100644 --- a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php +++ b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php @@ -1,4 +1,5 @@ get_logs( $action_id ); $expected = new ActionScheduler_LogEntry( $action_id, 'action created' ); - $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ) ) ); } protected function log_entry_to_array( $logs ) { @@ -101,8 +102,8 @@ public function test_execution_comments() { $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ), true ) ); - $this->assertTrue( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); } public function test_failed_execution_comments() { @@ -119,9 +120,9 @@ public function test_failed_execution_comments() { $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ), true ) ); - $this->assertFalse( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ), true ) ); - $this->assertTrue( in_array( $this->log_entry_to_array( $failed ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); + $this->assertFalse( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $failed ), $this->log_entry_to_array( $logs ) ) ); } public function test_failed_schedule_next_instance_comments() { @@ -136,7 +137,7 @@ public function test_failed_schedule_next_instance_comments() { } $logs = $logger->get_logs( $action_id ); - $this->assertTrue( in_array( $this->log_entry_to_array( $log_entry ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $log_entry ), $this->log_entry_to_array( $logs ) ) ); } public function test_fatal_error_comments() { @@ -169,7 +170,7 @@ public function test_canceled_action_comments() { $logger = ActionScheduler::logger(); $logs = $logger->get_logs( $action_id ); $expected = new ActionScheduler_LogEntry( $action_id, 'action canceled' ); - $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ), true ) ); + $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ) ) ); } public function a_hook_callback_that_throws_an_exception() { From ea715adf3301715f5939bf483c42b652fdb4252a Mon Sep 17 00:00:00 2001 From: Caleb Stauffer Date: Wed, 30 Oct 2024 18:04:23 -0400 Subject: [PATCH 4/4] ignore by line or block rather than by file --- .../ActionScheduler_wpCommentLogger_Test.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php index e6704d200..b5d41db0a 100644 --- a/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php +++ b/tests/phpunit/logging/ActionScheduler_wpCommentLogger_Test.php @@ -1,5 +1,4 @@ get_logs( $action_id ); $expected = new ActionScheduler_LogEntry( $action_id, 'action created' ); + + // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ) ) ); } @@ -102,8 +103,11 @@ public function test_execution_comments() { $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); + + // phpcs:disable WordPress.PHP.StrictInArray.MissingTrueStrict $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); $this->assertTrue( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); + // phpcs:enable } public function test_failed_execution_comments() { @@ -120,9 +124,12 @@ public function test_failed_execution_comments() { $runner->run( 'Unit Tests' ); $logs = $logger->get_logs( $action_id ); + + // phpcs:disable WordPress.PHP.StrictInArray.MissingTrueStrict $this->assertTrue( in_array( $this->log_entry_to_array( $started ), $this->log_entry_to_array( $logs ) ) ); $this->assertFalse( in_array( $this->log_entry_to_array( $finished ), $this->log_entry_to_array( $logs ) ) ); $this->assertTrue( in_array( $this->log_entry_to_array( $failed ), $this->log_entry_to_array( $logs ) ) ); + // phpcs:enable } public function test_failed_schedule_next_instance_comments() { @@ -137,6 +144,8 @@ public function test_failed_schedule_next_instance_comments() { } $logs = $logger->get_logs( $action_id ); + + // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict $this->assertTrue( in_array( $this->log_entry_to_array( $log_entry ), $this->log_entry_to_array( $logs ) ) ); } @@ -170,6 +179,8 @@ public function test_canceled_action_comments() { $logger = ActionScheduler::logger(); $logs = $logger->get_logs( $action_id ); $expected = new ActionScheduler_LogEntry( $action_id, 'action canceled' ); + + // phpcs:ignore WordPress.PHP.StrictInArray.MissingTrueStrict $this->assertTrue( in_array( $this->log_entry_to_array( $expected ), $this->log_entry_to_array( $logs ) ) ); }