From 0d373e7d31003378946a83c47722f61266693688 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 18 Oct 2026 14:06:15 +0000 Subject: [PATCH] Add regression test for repeated cron runs; drop unused return value - tests/notification/duplicates_test.php: the same update handed over by the cron job repeatedly yields exactly one notification per user, a newer version a new one - tests/notification/base.php: provide a fresh type instance per request like shared: false - notification/base.php: parent::create_insert_array() has no return value since phpBB 3.2 Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01CFCEXHKUor1WeKS7KibQwV --- notification/base.php | 2 +- tests/notification/base.php | 9 ++- tests/notification/duplicates_test.php | 90 ++++++++++++++++++++++ tests/notification/fixtures/duplicates.xml | 33 ++++++++ 4 files changed, 131 insertions(+), 3 deletions(-) create mode 100644 tests/notification/duplicates_test.php create mode 100644 tests/notification/fixtures/duplicates.xml diff --git a/notification/base.php b/notification/base.php index 908e00c..fc9f0e1 100644 --- a/notification/base.php +++ b/notification/base.php @@ -324,6 +324,6 @@ public function create_insert_array($notification_data, $pre_create_data = array $this->set_data('announcement_url', $notification_data['announcement_url']); } - return parent::create_insert_array($notification_data, $pre_create_data); + parent::create_insert_array($notification_data, $pre_create_data); } } diff --git a/tests/notification/base.php b/tests/notification/base.php index dadaa85..6a0042b 100644 --- a/tests/notification/base.php +++ b/tests/notification/base.php @@ -82,10 +82,15 @@ protected function setUp(): void foreach ($this->get_notification_types() as $type) { $type_parts = explode('.', $type); - $class = $this->build_type('\\gn36\\versionchecknotifier\\notification\\' . array_pop($type_parts)); + $type_parts_last = array_pop($type_parts); + $class = $this->build_type('\\gn36\\versionchecknotifier\\notification\\' . $type_parts_last); $types[$type] = $class; - $this->container->set($type, $class); + // Like "shared: false" in services.yml: every request for the type yields a new instance + $class_name = '\\gn36\\versionchecknotifier\\notification\\' . $type_parts_last; + $this->container->set($type, array(function () use ($class_name) { + return $this->build_type($class_name); + }, array())); } $this->notifications->set_var('notification_types', $types); diff --git a/tests/notification/duplicates_test.php b/tests/notification/duplicates_test.php new file mode 100644 index 0000000..61bae15 --- /dev/null +++ b/tests/notification/duplicates_test.php @@ -0,0 +1,90 @@ +createXMLDataSet(dirname(__FILE__) . '/fixtures/duplicates.xml'); + } + + protected function setUp(): void + { + parent::setUp(); + + $board = new \phpbb\notification\method\board($this->user_loader, $this->db, $this->cache->get_driver(), $this->user, $this->config, 'phpbb_notification_types', 'phpbb_notifications'); + $this->container->set('notification.method.board', $board); + $this->notifications->set_var('notification_methods', array('notification.method.board' => $board)); + } + + protected function count_notifications($type) + { + $sql = 'SELECT COUNT(*) AS num FROM phpbb_notifications n + JOIN phpbb_notification_types nt ON n.notification_type_id = nt.notification_type_id + WHERE nt.notification_type_name = \'' . $this->db->sql_escape($type) . '\''; + $result = $this->db->sql_query($sql); + $num = (int) $this->db->sql_fetchfield('num'); + $this->db->sql_freeresult($result); + + return $num; + } + + /** + * Data as handed over by \gn36\versionchecknotifier\cron\versionchecknotifier + */ + protected function cron_data($new = '1.1.0') + { + return array( + 'name' => 'acme/demo', + 'old_version' => '1.0.0', + 'version' => array( + '1.1' => array( + 'current' => $new, + 'announcement' => 'https://example.com/announcement', + 'download' => 'https://example.com/download', + 'eol' => null, + 'security' => false, + ), + ), + ); + } + + public function test_same_update_is_only_notified_once() + { + $type = 'gn36.versionchecknotifier.notification.type.ext_update'; + + $this->notifications->add_notifications($type, $this->cron_data()); + $first = $this->count_notifications($type); + $this->assertGreaterThan(0, $first, 'Administrators should have been notified'); + + // Next cron run: same data again + $this->notifications->add_notifications($type, $this->cron_data()); + $this->notifications->add_notifications($type, $this->cron_data()); + $this->assertEquals($first, $this->count_notifications($type), 'The same update must not be notified again'); + } + + public function test_new_version_is_notified_again() + { + $type = 'gn36.versionchecknotifier.notification.type.ext_update'; + + $this->notifications->add_notifications($type, $this->cron_data('1.1.0')); + $first = $this->count_notifications($type); + + $this->notifications->add_notifications($type, $this->cron_data('1.1.1')); + $this->assertEquals(2 * $first, $this->count_notifications($type), 'A newer version needs a new notification'); + } +} diff --git a/tests/notification/fixtures/duplicates.xml b/tests/notification/fixtures/duplicates.xml new file mode 100644 index 0000000..634f36c --- /dev/null +++ b/tests/notification/fixtures/duplicates.xml @@ -0,0 +1,33 @@ + + + + user_id + username + username_clean + user_type + user_permissions + user_sig + + 2 + founder + founder + 3 + + + + + 3 + founder2 + founder2 + 3 + + + +
+ +
+ +
+ +
+