From 556e3a9e8a729960c540cecf1a539c0b48d50702 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 12 Nov 2014 15:47:32 +0800 Subject: [PATCH 1/2] MDL-48197 testing: Always set up a mail sink for mail redirection Rather than setting noemailever in the bootstrap, this sets up a default phpmailer message sink which will catch all messages. Tests which require the phpmailer sink will continue to work as before without change, though the noemailever config setting no longer needs to be altered. For tests wanting to test with noemailever set with the previous behaviour, they will need to load the message sink and then close it. --- lib/phpunit/bootstrap.php | 2 -- lib/phpunit/classes/util.php | 15 ++++++++++----- lib/upgrade.txt | 3 +++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/phpunit/bootstrap.php b/lib/phpunit/bootstrap.php index 4d573df0b00..add63122157 100644 --- a/lib/phpunit/bootstrap.php +++ b/lib/phpunit/bootstrap.php @@ -209,8 +209,6 @@ error_reporting($CFG->debug); ini_set('display_errors', '1'); ini_set('log_errors', '1'); -$CFG->noemailever = true; // better not mail anybody from tests, override temporarily if necessary - // some ugly hacks $CFG->themerev = 1; $CFG->jsrev = 1; diff --git a/lib/phpunit/classes/util.php b/lib/phpunit/classes/util.php index c5e408b3ae7..1c1928e0ff7 100644 --- a/lib/phpunit/classes/util.php +++ b/lib/phpunit/classes/util.php @@ -106,12 +106,15 @@ class phpunit_util extends testing_util { // Stop any message redirection. phpunit_util::stop_message_redirection(); - // Stop any message redirection. - phpunit_util::stop_phpmailer_redirection(); - // Stop any message redirection. phpunit_util::stop_event_redirection(); + // Start a new email redirection. + // This will clear any existing phpmailer redirection. + // We redirect all phpmailer output to this message sink which is + // called instead of phpmailer actually sending the message. + phpunit_util::start_phpmailer_redirection(); + // We used to call gc_collect_cycles here to ensure desctructors were called between tests. // This accounted for 25% of the total time running phpunit - so we removed it. @@ -678,9 +681,11 @@ class phpunit_util extends testing_util { */ public static function start_phpmailer_redirection() { if (self::$phpmailersink) { - self::stop_phpmailer_redirection(); + // If an existing mailer sink is active, just clear it. + self::$phpmailersink->clear(); + } else { + self::$phpmailersink = new phpunit_phpmailer_sink(); } - self::$phpmailersink = new phpunit_phpmailer_sink(); return self::$phpmailersink; } diff --git a/lib/upgrade.txt b/lib/upgrade.txt index 79e608d706b..7ea8021503a 100644 --- a/lib/upgrade.txt +++ b/lib/upgrade.txt @@ -4,6 +4,9 @@ information provided here is intended especially for developers. === 2.9 === * \core\event\course_viewed 'other' argument renamed from coursesectionid to coursesectionnumber as it contains the section number. +* PHPUnit: PHPMailer Sink is now started for all tests and is setup within the phpunit wrapper for advanced tests. + Catching debugging messages when sending mail will no longer work. Use $sink = $this->redirectEmails(); and then check + the message in the sink instead. === 2.8 === From fc05c1b8e53db921aaf330caca1dee76f0b8d0e6 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Wed, 12 Nov 2014 15:51:18 +0800 Subject: [PATCH 2/2] MDL-48197 testing: Normalise uses of noemailever and phpmailer sink --- auth/ldap/tests/plugin_test.php | 3 ++- badges/tests/badgeslib_test.php | 2 -- course/tests/courserequest_test.php | 4 ---- lib/phpunit/tests/advanced_test.php | 2 ++ lib/tests/messagelib_test.php | 19 +++++++++++-------- lib/tests/moodlelib_test.php | 8 +++++--- message/output/email/message_output_email.php | 3 --- mod/forum/tests/mail_test.php | 3 --- mod/forum/tests/maildigest_test.php | 3 --- 9 files changed, 20 insertions(+), 27 deletions(-) diff --git a/auth/ldap/tests/plugin_test.php b/auth/ldap/tests/plugin_test.php index 7f0855e0085..ebb6f31df25 100644 --- a/auth/ldap/tests/plugin_test.php +++ b/auth/ldap/tests/plugin_test.php @@ -386,8 +386,9 @@ class auth_ldap_plugin_testcase extends advanced_testcase { $auth = get_auth_plugin('ldap'); $sink = $this->redirectEvents(); + $mailsink = $this->redirectEmails(); $auth->user_signup((object)$user, false); - $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting'); + $this->assertEquals(1, $mailsink->count()); $events = $sink->get_events(); $sink->close(); diff --git a/badges/tests/badgeslib_test.php b/badges/tests/badgeslib_test.php index aa074b47d8b..ce13ed8dcf4 100644 --- a/badges/tests/badgeslib_test.php +++ b/badges/tests/badgeslib_test.php @@ -41,8 +41,6 @@ class core_badges_badgeslib_testcase extends advanced_testcase { global $DB, $CFG; $this->resetAfterTest(true); - unset_config('noemailever'); - $CFG->enablecompletion = true; $user = $this->getDataGenerator()->create_user(); diff --git a/course/tests/courserequest_test.php b/course/tests/courserequest_test.php index 3b85c334ae6..1e96d21e29c 100644 --- a/course/tests/courserequest_test.php +++ b/course/tests/courserequest_test.php @@ -81,8 +81,6 @@ class core_course_courserequest_testcase extends advanced_testcase { $this->resetAfterTest(true); $this->preventResetByRollback(); - unset_config('noemailever'); - $defaultcategory = $DB->get_field_select('course_categories', "MIN(id)", "parent=0"); set_config('enablecourserequests', 1); set_config('requestcategoryselection', 0); @@ -138,8 +136,6 @@ class core_course_courserequest_testcase extends advanced_testcase { $this->resetAfterTest(true); $this->preventResetByRollback(); - unset_config('noemailever'); - $this->setAdminUser(); set_config('enablecourserequests', 1); set_config('requestcategoryselection', 0); diff --git a/lib/phpunit/tests/advanced_test.php b/lib/phpunit/tests/advanced_test.php index 82c3b28a0af..1caf138c122 100644 --- a/lib/phpunit/tests/advanced_test.php +++ b/lib/phpunit/tests/advanced_test.php @@ -432,6 +432,8 @@ class core_phpunit_advanced_testcase extends advanced_testcase { $message2->notification = 0; // There should be debugging message without redirection. + $mailsink = $this->redirectEmails(); + $mailsink->close(); message_send($message1); $this->assertDebuggingCalled(null, null, 'message_send() must print debug message that messaging is disabled in phpunit tests.'); diff --git a/lib/tests/messagelib_test.php b/lib/tests/messagelib_test.php index 2d076bea8c6..9d86d0c8619 100644 --- a/lib/tests/messagelib_test.php +++ b/lib/tests/messagelib_test.php @@ -30,7 +30,6 @@ class core_messagelib_testcase extends advanced_testcase { public function test_message_provider_disabled() { $this->resetAfterTest(); $this->preventResetByRollback(); - unset_config('noemailever'); // Disable instantmessage provider. $disableprovidersetting = 'moodle_instantmessage_disable'; @@ -382,8 +381,6 @@ class core_messagelib_testcase extends advanced_testcase { $this->preventResetByRollback(); $this->resetAfterTest(); - unset_config('noemailever'); - $user1 = $this->getDataGenerator()->create_user(); $user2 = $this->getDataGenerator()->create_user(); @@ -717,7 +714,6 @@ class core_messagelib_testcase extends advanced_testcase { $this->resetAfterTest(); $this->preventResetByRollback(); - set_config('noemailever', 1); $user1 = $this->getDataGenerator()->create_user(); $user2 = $this->getDataGenerator()->create_user(); @@ -734,24 +730,30 @@ class core_messagelib_testcase extends advanced_testcase { $message->smallmessage = 'small message'; $message->notification = '0'; + $mailsink = $this->redirectEmails(); + + // Sending outside of a transaction is fine. message_send($message); - $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting'); + $this->assertEquals(1, $mailsink->count()); $transaction1 = $DB->start_delegated_transaction(); + $mailsink->clear(); message_send($message); - $this->assertDebuggingNotCalled(); + $this->assertEquals(0, $mailsink->count()); $transaction2 = $DB->start_delegated_transaction(); + $mailsink->clear(); message_send($message); - $this->assertDebuggingNotCalled(); + $this->assertEquals(0, $mailsink->count()); try { $transaction2->rollback(new Exception('x')); $this->fail('Expecting exception'); } catch (Exception $e) {} $this->assertDebuggingNotCalled(); + $this->assertEquals(0, $mailsink->count()); $this->assertTrue($DB->is_transaction_started()); @@ -760,11 +762,12 @@ class core_messagelib_testcase extends advanced_testcase { $this->fail('Expecting exception'); } catch (Exception $e) {} $this->assertDebuggingNotCalled(); + $this->assertEquals(0, $mailsink->count()); $this->assertFalse($DB->is_transaction_started()); message_send($message); - $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting'); + $this->assertEquals(1, $mailsink->count()); } public function test_forced_rollback() { diff --git a/lib/tests/moodlelib_test.php b/lib/tests/moodlelib_test.php index 004cc071ad5..206a6c9378f 100644 --- a/lib/tests/moodlelib_test.php +++ b/lib/tests/moodlelib_test.php @@ -2607,6 +2607,11 @@ class core_moodlelib_testcase extends advanced_testcase { $subject2 = 'subject 2'; $messagetext2 = 'message text 2'; + // Close the default email sink. + $sink = $this->redirectEmails(); + $sink->close(); + + $CFG->noemailever = true; $this->assertNotEmpty($CFG->noemailever); email_to_user($user1, $user2, $subject, $messagetext); $this->assertDebuggingCalled('Not sending email due to $CFG->noemailever config setting'); @@ -2660,9 +2665,6 @@ class core_moodlelib_testcase extends advanced_testcase { $user = $this->getDataGenerator()->create_user(); - // Set config to allow email_to_user() to be called. - $CFG->noemailever = false; - // Update user password. $sink = $this->redirectEvents(); $sink2 = $this->redirectEmails(); // Make sure we are redirecting emails. diff --git a/message/output/email/message_output_email.php b/message/output/email/message_output_email.php index 247aa5fa028..1970606c09d 100644 --- a/message/output/email/message_output_email.php +++ b/message/output/email/message_output_email.php @@ -39,9 +39,6 @@ class message_output_email extends message_output { function send_message($eventdata) { global $CFG; - // Ignore $CFG->noemailever here because we want to test this code, - // the message sending fails later in email_to_user(). - // skip any messaging suspended and deleted users if ($eventdata->userto->auth === 'nologin' or $eventdata->userto->suspended or $eventdata->userto->deleted) { return true; diff --git a/mod/forum/tests/mail_test.php b/mod/forum/tests/mail_test.php index db6eb94cd64..2f6d81461c1 100644 --- a/mod/forum/tests/mail_test.php +++ b/mod/forum/tests/mail_test.php @@ -60,9 +60,6 @@ class mod_forum_mail_testcase extends advanced_testcase { // ensure that messages are sent out. $CFG->maxeditingtime = -1; - // Ensure that we don't prevent e-mail as this will cause unit test failures. - $CFG->noemailever = false; - $this->helper = $helper; } diff --git a/mod/forum/tests/maildigest_test.php b/mod/forum/tests/maildigest_test.php index 19725bad229..d52eee4a951 100644 --- a/mod/forum/tests/maildigest_test.php +++ b/mod/forum/tests/maildigest_test.php @@ -72,9 +72,6 @@ class mod_forum_maildigest_testcase extends advanced_testcase { // messages are sent out. $CFG->maxeditingtime = 1; - // Ensure that we don't prevent e-mail as this will cause unit test failures. - $CFG->noemailever = false; - // We must clear the subscription caches. This has to be done both before each test, and after in case of other // tests using these functions. \mod_forum\subscriptions::reset_forum_cache();