From 5b1955c07391c059811fe1174660f18876c6436b Mon Sep 17 00:00:00 2001 From: David Carrillo Date: Mon, 16 Mar 2026 13:16:32 +0100 Subject: [PATCH] MDL-88226 phpunit: Fix fragile tests using adhoc task manager - Reset task manager state when resetting all data - mod_quiz: Fix fragile adhoc task manager tests - mod_assign: Fix fragile adhoc task manager tests --- lib/phpunit/classes/util.php | 1 + .../feedback/editpdf/tests/feedback_test.php | 9 +- mod/assign/tests/notification_helper_test.php | 92 ++++++++++++++----- mod/quiz/tests/notification_helper_test.php | 35 ++++--- 4 files changed, 97 insertions(+), 40 deletions(-) diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index 48b9ab8536a..858337e01c7 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -228,6 +228,7 @@ class phpunit_util extends testing_util { core_filetypes::reset_caches(); \core_search\manager::clear_static(); core_user::reset_caches(); + \core\task\manager::reset_state(); \core\output\icon_system::reset_caches(); if (class_exists('core_media_manager', false)) { core_media_manager::reset_caches(); diff --git a/mod/assign/feedback/editpdf/tests/feedback_test.php b/mod/assign/feedback/editpdf/tests/feedback_test.php index 29993a214f3..f38635832a9 100644 --- a/mod/assign/feedback/editpdf/tests/feedback_test.php +++ b/mod/assign/feedback/editpdf/tests/feedback_test.php @@ -16,6 +16,7 @@ namespace assignfeedback_editpdf; +use assignfeedback_editpdf\task\convert_submission; use mod_assign_test_generator; defined('MOODLE_INTERNAL') || die(); @@ -360,7 +361,7 @@ final class feedback_test extends \advanced_testcase { $this->add_file_submission($student, $assign); // Run the conversion task. - $task = \core\task\manager::get_next_adhoc_task(time()); + $task = \core\task\manager::get_next_adhoc_task(time(), true, convert_submission::class); ob_start(); $task->execute(); \core\task\manager::adhoc_task_complete($task); @@ -369,14 +370,14 @@ final class feedback_test extends \advanced_testcase { // Confirm, that submission has been converted and the task queue is now empty. $this->assertStringContainsString('Converting submission for user id ' . $student->id, $output); $this->assertStringContainsString('The document has been successfully converted', $output); - $this->assertNull(\core\task\manager::get_next_adhoc_task(time())); + $this->assertNull(\core\task\manager::get_next_adhoc_task(time(), true, convert_submission::class)); // Trigger a re-queue by 'updating' a submission. $submission = $assign->get_user_submission($student->id, true); $plugin = $assign->get_submission_plugin_by_type('file'); $plugin->save($submission, (new \stdClass)); - $task = \core\task\manager::get_next_adhoc_task(time()); + $task = \core\task\manager::get_next_adhoc_task(time(), true, convert_submission::class); // Verify that queued a conversion task. $this->assertNotNull($task); @@ -388,7 +389,7 @@ final class feedback_test extends \advanced_testcase { // Confirm, that submission has been converted and the task queue is now empty. $this->assertStringContainsString('Converting submission for user id ' . $student->id, $output); $this->assertStringContainsString('The document has been successfully converted', $output); - $this->assertNull(\core\task\manager::get_next_adhoc_task(time())); + $this->assertNull(\core\task\manager::get_next_adhoc_task(time(), true, convert_submission::class)); } /** diff --git a/mod/assign/tests/notification_helper_test.php b/mod/assign/tests/notification_helper_test.php index fceee9097ac..14722b62669 100644 --- a/mod/assign/tests/notification_helper_test.php +++ b/mod/assign/tests/notification_helper_test.php @@ -17,7 +17,11 @@ namespace mod_assign; use core\task\task_trait; -use mod_assign\task\queue_assignment_due_digest_notification_tasks_for_users; +use mod_assign\task\queue_assignment_due_soon_notification_tasks_for_users; +use mod_assign\task\queue_assignment_overdue_notification_tasks_for_users; +use mod_assign\task\send_assignment_due_digest_notification_to_user; +use mod_assign\task\send_assignment_due_soon_notification_to_user; +use mod_assign\task\send_assignment_overdue_notification_to_user; /** * Test class for the assignment notification_helper. @@ -40,18 +44,26 @@ final class notification_helper_test extends \advanced_testcase { $task->execute(); $clock = \core\di::get(\core\clock::class); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + queue_assignment_due_soon_notification_tasks_for_users::class + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_assign\task\queue_assignment_due_soon_notification_tasks_for_users::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_due_soon_notification_to_user::class + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_assign\task\send_assignment_due_soon_notification_to_user::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } } @@ -356,27 +368,40 @@ final class notification_helper_test extends \advanced_testcase { $task->execute(); $clock->bump(5); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); - $this->assertInstanceOf(\mod_assign\task\queue_assignment_due_soon_notification_tasks_for_users::class, $adhoctask); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + queue_assignment_due_soon_notification_tasks_for_users::class, + ); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); // Delete the assignment. $DB->delete_records('assign', ['id' => $assignment->id]); // Try to run the ad-hoc task to send the notifications. $clock->bump(5); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); - $this->assertInstanceOf(\mod_assign\task\send_assignment_due_soon_notification_to_user::class, $adhoctask); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_due_soon_notification_to_user::class, + ); ob_start(); $adhoctask->execute(); $output = ob_get_clean(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); // The ad-hoc task should be deleted. - $this->assertNull(\core\task\manager::get_next_adhoc_task($clock->time())); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_due_soon_notification_to_user::class, + ); + $this->assertNull($adhoctask); $this->assertStringContainsString( needle: "No notification send as the assignment $assignment->id can no longer be found in the database.", haystack: $output @@ -391,18 +416,26 @@ final class notification_helper_test extends \advanced_testcase { $task->execute(); $clock = \core\di::get(\core\clock::class); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + queue_assignment_overdue_notification_tasks_for_users::class, + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_assign\task\queue_assignment_overdue_notification_tasks_for_users::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_overdue_notification_to_user::class, + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_assign\task\send_assignment_overdue_notification_to_user::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } } @@ -741,27 +774,40 @@ final class notification_helper_test extends \advanced_testcase { $task->execute(); $clock->bump(5); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); - $this->assertInstanceOf(\mod_assign\task\queue_assignment_overdue_notification_tasks_for_users::class, $adhoctask); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + queue_assignment_overdue_notification_tasks_for_users::class, + ); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); // Delete the assignment. $DB->delete_records('assign', ['id' => $assignment->id]); // Try to run the ad-hoc task to send the notifications. $clock->bump(5); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); - $this->assertInstanceOf(\mod_assign\task\send_assignment_overdue_notification_to_user::class, $adhoctask); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_overdue_notification_to_user::class, + ); ob_start(); $adhoctask->execute(); $output = ob_get_clean(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); // The ad-hoc task should be deleted. - $this->assertNull(\core\task\manager::get_next_adhoc_task($clock->time())); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_overdue_notification_to_user::class, + ); + $this->assertNull($adhoctask); $this->assertStringContainsString( needle: "No notification send as the assignment $assignment->id can no longer be found in the database.", haystack: $output @@ -776,11 +822,15 @@ final class notification_helper_test extends \advanced_testcase { $task->execute(); $clock = \core\di::get(\core\clock::class); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_assignment_due_digest_notification_to_user::class, + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_assign\task\send_assignment_due_digest_notification_to_user::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } } diff --git a/mod/quiz/tests/notification_helper_test.php b/mod/quiz/tests/notification_helper_test.php index d1e932bdce0..a5b6c7439ad 100644 --- a/mod/quiz/tests/notification_helper_test.php +++ b/mod/quiz/tests/notification_helper_test.php @@ -16,6 +16,10 @@ namespace mod_quiz; +use mod_quiz\task\queue_all_quiz_open_notification_tasks; +use mod_quiz\task\queue_quiz_open_notification_tasks_for_users; +use mod_quiz\task\send_quiz_open_soon_notification_to_user; + /** * Test class for the quiz notification helper. * @@ -30,22 +34,30 @@ final class notification_helper_test extends \advanced_testcase { * Run all the tasks related to the notifications. */ protected function run_notification_helper_tasks(): void { - $task = \core\task\manager::get_scheduled_task(\mod_quiz\task\queue_all_quiz_open_notification_tasks::class); + $task = \core\task\manager::get_scheduled_task(queue_all_quiz_open_notification_tasks::class); $task->execute(); $clock = \core\di::get(\core\clock::class); - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + queue_quiz_open_notification_tasks_for_users::class + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_quiz\task\queue_quiz_open_notification_tasks_for_users::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } - $adhoctask = \core\task\manager::get_next_adhoc_task($clock->time()); + $adhoctask = \core\task\manager::get_next_adhoc_task( + $clock->time(), + true, + send_quiz_open_soon_notification_to_user::class + ); if ($adhoctask) { - $this->assertInstanceOf(\mod_quiz\task\send_quiz_open_soon_notification_to_user::class, $adhoctask); $adhoctask->execute(); \core\task\manager::adhoc_task_complete($adhoctask); + \core\task\manager::reset_state(); } } @@ -214,23 +226,16 @@ final class notification_helper_test extends \advanced_testcase { ]); $clock->bump(5); - // Get the users within the date range. - $quizzes = $helper::get_quizzes_within_date_range(); - foreach ($quizzes as $q) { - $users = $helper::get_users_within_quiz($q->id); - } - $quizzes->close(); - // Run the tasks. $this->run_notification_helper_tasks(); // Get the notifications that should have been created during the adhoc task. - $this->assertCount(1, $sink->get_messages()); + $messages = $sink->get_messages_by_component('mod_quiz'); + $this->assertCount(1, $messages); // Check the subject matches. - $messages = $sink->get_messages_by_component('mod_quiz'); $message = reset($messages); - $stringparams = ['timeopen' => userdate($users[$user1->id]->timeopen), 'quizname' => $quiz->name]; + $stringparams = ['timeopen' => userdate($timeopen), 'quizname' => $quiz->name]; $expectedsubject = get_string('quizopendatesoonsubject', 'mod_quiz', $stringparams); $this->assertEquals($expectedsubject, $message->subject);