From 8d94b802cff1f00cf2ec5c432af1855ff57b6103 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Wed, 4 Mar 2026 13:17:37 -0800 Subject: [PATCH] Bunch of PHP 8.2 and other little fixes --- README.md | 2 +- tests/functional/acp_file_test.php | 42 +++++++++++++------------- tests/functional/acp_settings_test.php | 22 +++++++------- tests/unit/acp_module_test.php | 8 +++-- tests/unit/admin_controller_test.php | 40 ++++++++++++------------ tests/unit/event_listener_test.php | 29 ++++++++++-------- tests/unit/ext_test.php | 14 ++++----- tests/unit/helper_test.php | 22 ++++++++------ tests/unit/upload_test.php | 4 +-- 9 files changed, 97 insertions(+), 86 deletions(-) diff --git a/README.md b/README.md index e5f21cc..90f296c 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ Under development... -Gives phpBB board admins ability to manage web app icons and colour themes for their site. +Allows phpBB board admins to manage web app icons and colour themes for their site. [![Build Status](https://github.com/phpbb-extensions/pwakit/workflows/Tests/badge.svg)](https://github.com/phpbb-extensions/pwakit/actions) [![codecov](https://codecov.io/gh/phpbb-extensions/pwakit/graph/badge.svg?token=34V2MQSY3H)](https://codecov.io/gh/phpbb-extensions/pwakit) diff --git a/tests/functional/acp_file_test.php b/tests/functional/acp_file_test.php index 3b49bb2..5838325 100644 --- a/tests/functional/acp_file_test.php +++ b/tests/functional/acp_file_test.php @@ -39,8 +39,8 @@ protected function setUp(): void $this->fixtures = __DIR__ . '/../fixtures/'; $this->icons = __DIR__ . '/../../../../../images/site_icons/'; - $this->add_lang('posting'); - $this->add_lang_ext('phpbb/pwakit', ['acp_pwa', 'info_acp_pwa']); + self::add_lang('posting'); + self::add_lang_ext('phpbb/pwakit', ['acp_pwa', 'info_acp_pwa']); } protected function tearDown(): void @@ -71,9 +71,9 @@ private function upload_file($filename, $mimetype): Crawler $url = 'index.php?i=-phpbb-pwakit-acp-pwa_acp_module&mode=settings&sid=' . $this->sid; $crawler = self::$client->request('GET', $url); - $this->assertContainsLang('ACP_PWA_KIT_SETTINGS', $crawler->text()); + self::assertContainsLang('ACP_PWA_KIT_SETTINGS', $crawler->text()); - $file_form_data = array_merge(['upload' => $this->lang('ACP_PWA_IMG_UPLOAD_BTN')], $this->get_hidden_fields($crawler, $url)); + $file_form_data = array_merge(['upload' => self::lang('ACP_PWA_IMG_UPLOAD_BTN')], $this->get_hidden_fields($crawler, $url)); $file = [ 'tmp_name' => $this->fixtures . $filename, @@ -91,40 +91,40 @@ private function upload_file($filename, $mimetype): Crawler ); } - public function test_upload_empty_file() + public function test_upload_empty_file(): void { - $this->login(); - $this->admin_login(); + self::login(); + self::admin_login(); $crawler = $this->upload_file('empty.png', 'image/png'); - $this->assertEquals($this->lang('EMPTY_FILEUPLOAD'), $crawler->filter('div.errorbox > p')->text()); + $this->assertEquals(self::lang('EMPTY_FILEUPLOAD'), $crawler->filter('div.errorbox > p')->text()); } - public function test_upload_invalid_extension() + public function test_upload_invalid_extension(): void { - $this->login(); - $this->admin_login(); + self::login(); + self::admin_login(); $crawler = $this->upload_file('foo.gif', 'image/gif'); - $this->assertEquals($this->lang('DISALLOWED_EXTENSION', 'gif'), $crawler->filter('div.errorbox > p')->text()); + $this->assertEquals(self::lang('DISALLOWED_EXTENSION', 'gif'), $crawler->filter('div.errorbox > p')->text()); } - public function test_upload_valid_file() + public function test_upload_valid_file(): void { $test_image = 'foo.png'; // Check icon does not yet appear in the html tags $this->assertAppleTouchIconNotPresent(); - $this->login(); - $this->admin_login(); + self::login(); + self::admin_login(); $crawler = $this->upload_file($test_image, 'image/png'); // Ensure there was no error message rendered - $this->assertContainsLang('ACP_PWA_IMG_UPLOAD_SUCCESS', $crawler->text()); + self::assertContainsLang('ACP_PWA_IMG_UPLOAD_SUCCESS', $crawler->text()); // Check icon appears in the ACP as expected $this->assertIconInACP($test_image); @@ -133,7 +133,7 @@ public function test_upload_valid_file() $this->assertAppleTouchIconPresent($test_image); } - public function test_resync_delete_file() + public function test_resync_delete_file(): void { $test_image = 'bar.png'; @@ -143,8 +143,8 @@ public function test_resync_delete_file() // Check icon does not appear in the html tags $this->assertAppleTouchIconNotPresent(); - $this->login(); - $this->admin_login(); + self::login(); + self::admin_login(); // Ensure copied image does not appear in ACP $crawler = $this->assertIconsNotInACP(); @@ -188,7 +188,7 @@ private function performDelete(string $icon): void $crawler = self::submit($form); $form = $crawler->selectButton('confirm')->form(['delete' => $icon]); $crawler = self::submit($form); - $this->assertStringContainsString($this->lang('ACP_PWA_IMG_DELETED', $icon), $crawler->text()); + $this->assertStringContainsString(self::lang('ACP_PWA_IMG_DELETED', $icon), $crawler->text()); } /** @@ -222,7 +222,7 @@ private function assertAppleTouchIconPresent(string $icon): void private function assertIconsNotInACP(): Crawler { $crawler = self::request('GET', 'adm/index.php?i=-phpbb-pwakit-acp-pwa_acp_module&mode=settings&sid=' . $this->sid); - $this->assertContainsLang('ACP_PWA_KIT_NO_ICONS', $crawler->filter('fieldset')->eq(3)->html()); + self::assertContainsLang('ACP_PWA_KIT_NO_ICONS', $crawler->filter('fieldset')->eq(3)->html()); return $crawler; } diff --git a/tests/functional/acp_settings_test.php b/tests/functional/acp_settings_test.php index e671590..7db095d 100644 --- a/tests/functional/acp_settings_test.php +++ b/tests/functional/acp_settings_test.php @@ -23,28 +23,28 @@ protected static function setup_extensions(): array return ['phpbb/pwakit']; } - public function test_extension_enabled() + public function test_extension_enabled(): void { - $this->login(); - $this->admin_login(); - $this->add_lang('acp/extensions'); + self::login(); + self::admin_login(); + self::add_lang('acp/extensions'); $crawler = self::request('GET', 'adm/index.php?i=acp_extensions&mode=main&sid=' . $this->sid); $this->assertStringContainsString('Progressive Web App Kit', $crawler->filter('.ext_enabled')->eq(0)->text()); - $this->assertContainsLang('EXTENSION_DISABLE', $crawler->filter('.ext_enabled')->eq(0)->text()); + self::assertContainsLang('EXTENSION_DISABLE', $crawler->filter('.ext_enabled')->eq(0)->text()); } - public function test_basic_form() + public function test_basic_form(): void { - $this->login(); - $this->admin_login(); + self::login(); + self::admin_login(); - $this->add_lang_ext('phpbb/pwakit', 'info_acp_pwa'); + self::add_lang_ext('phpbb/pwakit', 'info_acp_pwa'); // Check ACP page loads $crawler = self::request('GET', 'adm/index.php?i=-phpbb-pwakit-acp-pwa_acp_module&mode=settings&sid=' . $this->sid); - $this->assertContainsLang('ACP_PWA_KIT_TITLE', $crawler->filter('div.main > h1')->text()); + self::assertContainsLang('ACP_PWA_KIT_TITLE', $crawler->filter('div.main > h1')->text()); // The _1 means these are for prosilver (style id 1) $form_data = [ @@ -61,7 +61,7 @@ public function test_basic_form() // Submit form $form = $crawler->selectButton('submit')->form($form_data); $crawler = self::submit($form); - $this->assertStringContainsString($this->lang('CONFIG_UPDATED'), $crawler->filter('.successbox')->text()); + $this->assertStringContainsString(self::lang('CONFIG_UPDATED'), $crawler->filter('.successbox')->text()); // Check saved data now appears in data fields $crawler = self::request('GET', 'adm/index.php?i=-phpbb-pwakit-acp-pwa_acp_module&mode=settings&sid=' . $this->sid); diff --git a/tests/unit/acp_module_test.php b/tests/unit/acp_module_test.php index ebd3e3e..5e92203 100644 --- a/tests/unit/acp_module_test.php +++ b/tests/unit/acp_module_test.php @@ -10,6 +10,7 @@ namespace phpbb\pwakit\tests\unit; +use RuntimeException; use Symfony\Component\DependencyInjection\ContainerInterface; use p_master; use phpbb\cache\driver\dummy; @@ -147,10 +148,11 @@ public function test_main_module_with_missing_controller(): void { global $phpbb_container, $template, $request; - $this->expectException(\RuntimeException::class); + $this->expectException(RuntimeException::class); $this->expectExceptionMessage('Service not found: phpbb.pwakit.admin.controller'); - if (!defined('IN_ADMIN')) { + if (!defined('IN_ADMIN')) + { define('IN_ADMIN', true); } @@ -173,7 +175,7 @@ public function test_main_module_with_missing_controller(): void ->expects($this->once()) ->method('get') ->with('phpbb.pwakit.admin.controller') - ->willThrowException(new \RuntimeException('Service not found: phpbb.pwakit.admin.controller')); + ->willThrowException(new RuntimeException('Service not found: phpbb.pwakit.admin.controller')); $p_master = new p_master(); $p_master->module_ary[0]['is_duplicate'] = 0; diff --git a/tests/unit/admin_controller_test.php b/tests/unit/admin_controller_test.php index b993bca..c732aed 100644 --- a/tests/unit/admin_controller_test.php +++ b/tests/unit/admin_controller_test.php @@ -14,6 +14,8 @@ use phpbb\db\driver\driver_interface as dbal; use phpbb_mock_cache; use phpbb_mock_event_dispatcher; +use PHPUnit\DbUnit\DataSet\DefaultDataSet; +use PHPUnit\DbUnit\DataSet\XmlDataSet; use PHPUnit\Framework\MockObject\MockObject; use phpbb\config\config; use phpbb\exception\runtime_exception; @@ -25,6 +27,7 @@ use phpbb\request\request_interface; use phpbb\template\template; use phpbb_database_test_case; +use phpbb\pwakit\acp\pwa_acp_module; class admin_controller_test extends phpbb_database_test_case { @@ -34,10 +37,10 @@ class admin_controller_test extends phpbb_database_test_case protected dbal $db; protected config $config; protected language $language; - protected request $request; - protected template|MockObject $template; - protected helper $helper; - protected upload $upload; + protected MockObject|request $request; + protected MockObject|template $template; + protected MockObject|helper $helper; + protected MockObject|upload $upload; protected string $phpbb_root_path; protected admin_controller $admin_controller; @@ -46,7 +49,7 @@ protected static function setup_extensions(): array return ['phpbb/pwakit']; } - protected function getDataSet() + protected function getDataSet(): DefaultDataSet|XmlDataSet { return $this->createXMLDataSet(__DIR__ . '/../fixtures/styles.xml'); } @@ -124,12 +127,11 @@ public static function module_access_test_data(): array * @return void * @dataProvider module_access_test_data */ - public function test_module_access($mode, $expected) + public function test_module_access($mode, $expected): void { $this->request->expects($expected ? $this->atLeastOnce() : $this->never()) ->method('is_set_post'); - $this->call_admin_controller($mode); } @@ -146,7 +148,7 @@ public static function form_checks_data(): array * @param $action * @dataProvider form_checks_data */ - public function test_form_checks($action) + public function test_form_checks($action): void { self::$valid_form = false; @@ -208,7 +210,7 @@ public static function display_settings_test_data(): array * @param $expected * @dataProvider display_settings_test_data */ - public function test_display_settings($configs, $expected) + public function test_display_settings($configs, $expected): void { foreach ($configs as $key => $value) { @@ -350,10 +352,10 @@ public static function submit_test_data(): array * @param $expected_msg * @dataProvider submit_test_data */ - public function test_submit($form_data, $expected, $expected_msg) + public function test_submit($form_data, $expected, $expected_msg): void { $is_success = $expected_msg === 'CONFIG_UPDATED'; - + if ($is_success) { $this->setExpectedTriggerError(E_USER_NOTICE, 'CONFIG_UPDATED'); @@ -380,7 +382,7 @@ public function test_submit($form_data, $expected, $expected_msg) ]); $this->request_submit('submit'); - + if ($is_success) { $this->call_admin_controller(); @@ -404,7 +406,7 @@ public function test_submit($form_data, $expected, $expected_msg) $this->assertSame($expected, $rows); } - public function test_upload() + public function test_upload(): void { $this->setExpectedTriggerError(E_USER_NOTICE, 'ACP_PWA_IMG_UPLOAD_SUCCESS'); @@ -417,7 +419,7 @@ public function test_upload() $this->call_admin_controller(); } - public function test_upload_error() + public function test_upload_error(): void { $this->request_submit('upload'); @@ -431,7 +433,7 @@ public function test_upload_error() $this->call_admin_controller(); } - public function test_resync() + public function test_resync(): void { $this->request_submit('resync'); @@ -468,7 +470,7 @@ public static function delete_test_data(): array * @param $error * @dataProvider delete_test_data */ - public function test_delete($image, $confirmed, $error) + public function test_delete($image, $confirmed, $error): void { self::$confirm = $confirmed; @@ -510,7 +512,7 @@ public function test_delete($image, $confirmed, $error) */ private function call_admin_controller(string $mode = 'settings'): void { - $this->admin_controller->main('\\phpbb\\pwakit\\acp\\pwa_acp_module', $mode, ''); + $this->admin_controller->main(pwa_acp_module::class, $mode, ''); } /** @@ -533,7 +535,7 @@ private function request_submit($value): void */ function check_form_key(): bool { - return \phpbb\pwakit\controller\admin_controller_test::$valid_form; + return admin_controller_test::$valid_form; } /** @@ -552,7 +554,7 @@ function add_form_key() */ function confirm_box(): bool { - return \phpbb\pwakit\controller\admin_controller_test::$confirm; + return admin_controller_test::$confirm; } /** diff --git a/tests/unit/event_listener_test.php b/tests/unit/event_listener_test.php index d28aa2b..7d01b5c 100644 --- a/tests/unit/event_listener_test.php +++ b/tests/unit/event_listener_test.php @@ -21,8 +21,8 @@ class event_listener_test extends phpbb_test_case { - protected user|MockObject $user; - protected template|MockObject $template; + protected MockObject|user $user; + protected MockObject|template $template; protected helper $pwa_helper; /** @@ -67,7 +67,7 @@ protected function get_listener(): main_listener /** * Test the event listener is constructed correctly */ - public function test_construct() + public function test_construct(): void { static::assertInstanceOf(EventSubscriberInterface::class, $this->get_listener()); } @@ -75,7 +75,7 @@ public function test_construct() /** * Test the event listener is subscribing events */ - public function test_getSubscribedEvents() + public function test_getSubscribedEvents(): void { $events = main_listener::getSubscribedEvents(); static::assertEquals([ @@ -152,7 +152,7 @@ private static function getValidIcons(): array * @return void * @dataProvider header_updates_test_data */ - public function test_header_updates($configs, $icons, $expected) + public function test_header_updates($configs, $icons, $expected): void { // Setup expectations $this->pwa_helper->expects(static::once()) @@ -170,7 +170,8 @@ public function test_header_updates($configs, $icons, $expected) ->with(static::identicalTo($templateVars)); // Apply configurations - foreach ($configs as $key => $value) { + foreach ($configs as $key => $value) + { $this->user->style[$key] = $value; } @@ -241,7 +242,7 @@ public static function manifest_updates_test_data(): array * @return void * @dataProvider manifest_updates_test_data */ - public function test_manifest_updates($board_path, $configs, $expected) + public function test_manifest_updates($board_path, $configs, $expected): void { $initialManifest = [ 'name' => 'Test Site', @@ -260,7 +261,8 @@ public function test_manifest_updates($board_path, $configs, $expected) // Set up and verify the initial state $this->assertSame($initialManifest, $event['manifest']); - foreach ($configs as $key => $value) { + foreach ($configs as $key => $value) + { $this->user->style[$key] = $value; } @@ -280,18 +282,21 @@ public function test_manifest_updates($board_path, $configs, $expected) $this->assertSame($expected, $event['manifest']); // Verify manifest structure - if (!empty($event['manifest'])) { + if (!empty($event['manifest'])) + { $this->assertArrayHasKey('name', $event['manifest']); $this->assertArrayHasKey('short_name', $event['manifest']); $this->assertArrayHasKey('display', $event['manifest']); $this->assertArrayHasKey('orientation', $event['manifest']); } - // Verify color format if present - if (isset($event['manifest']['theme_color'])) { + // Verify a color format if present + if (isset($event['manifest']['theme_color'])) + { $this->assertMatchesRegularExpression('/^#[0-9a-f]{6}$/i', $event['manifest']['theme_color']); } - if (isset($event['manifest']['background_color'])) { + if (isset($event['manifest']['background_color'])) + { $this->assertMatchesRegularExpression('/^#[0-9a-f]{6}$/i', $event['manifest']['background_color']); } } diff --git a/tests/unit/ext_test.php b/tests/unit/ext_test.php index 262ee5d..11fea71 100644 --- a/tests/unit/ext_test.php +++ b/tests/unit/ext_test.php @@ -25,8 +25,8 @@ class ext_test extends phpbb_test_case { protected ContainerInterface|MockObject $container; - protected finder|MockObject $extension_finder; - protected migrator|MockObject $migrator; + protected MockObject|finder $extension_finder; + protected MockObject|migrator $migrator; protected function setUp(): void { @@ -74,9 +74,9 @@ public static function ext_test_data(): array * * @dataProvider ext_test_data */ - public function test_ext($version, $expected) + public function test_ext($version, $expected): void { - // Instantiate config object and set config version + // Instantiate a config object and set a config version $config = new config([ 'version' => $version, ]); @@ -117,7 +117,7 @@ public static function enable_test_data(): array /** * @dataProvider enable_test_data */ - public function test_enable($file_exists, $old_state, $expected) + public function test_enable($file_exists, $old_state, $expected): void { $filesystem = $this->getMockBuilder(filesystem::class) ->disableOriginalConstructor() @@ -149,7 +149,7 @@ public function test_enable($file_exists, $old_state, $expected) self::assertSame($expected, $ext->enable_step($old_state)); } - public function test_enable_fails() + public function test_enable_fails(): void { $filesystem = $this->getMockBuilder(filesystem::class) ->disableOriginalConstructor() @@ -178,7 +178,7 @@ public function test_enable_fails() ->method('add') ->with('critical', '2', '1.0.0.01', 'LOG_PWA_DIR_FAIL', false, ['images/site_icons', 'Test Error']); - // Make container return filesystem first + // Make the container return filesystem first $this->container->expects($this->exactly(3)) ->method('get') ->willReturnCallback(function ($service) use ($filesystem, $log, $user) { diff --git a/tests/unit/helper_test.php b/tests/unit/helper_test.php index 8c0473c..c5d5b30 100644 --- a/tests/unit/helper_test.php +++ b/tests/unit/helper_test.php @@ -11,6 +11,8 @@ namespace phpbb\pwakit\tests\unit; use FastImageSize\FastImageSize; +use PHPUnit\DbUnit\DataSet\DefaultDataSet; +use PHPUnit\DbUnit\DataSet\XmlDataSet; use PHPUnit\Framework\MockObject\MockObject; use phpbb\cache\driver\driver_interface as cache; use phpbb\config\config; @@ -36,7 +38,7 @@ class helper_test extends phpbb_database_test_case protected const FIXTURES = __DIR__ . '/../fixtures/'; protected config $config; - protected template|MockObject $template; + protected MockObject|template $template; protected helper $helper; protected storage $storage; protected file_tracker $file_tracker; @@ -48,7 +50,7 @@ protected static function setup_extensions(): array return ['phpbb/pwakit']; } - protected function getDataSet() + protected function getDataSet(): DefaultDataSet|XmlDataSet { return $this->createXMLDataSet(self::FIXTURES . 'storage.xml'); } @@ -114,7 +116,7 @@ protected function setUp(): void 'phpbb_pwakit' ); - $this->helper = new \phpbb\pwakit\helper\helper( + $this->helper = new helper( $phpbb_extension_manager, new FastImageSize(), $this->storage, @@ -146,17 +148,17 @@ protected function tearDown(): void parent::tearDown(); } - public function test_get_tracked_files() + public function test_get_tracked_files(): void { $this->assertEquals(['foo.png'], $this->file_tracker->get_tracked_files()); } - public function test_get_storage_path() + public function test_get_storage_path(): void { $this->assertEquals($this->storage_path, $this->helper->get_storage_path()); } - public function test_get_icons() + public function test_get_icons(): void { $expected[] = [ 'src' => $this->storage_path . '/foo.png', @@ -167,7 +169,7 @@ public function test_get_icons() $this->assertEquals($expected, $this->helper->get_icons()); } - public function test_get_icons_empty() + public function test_get_icons_empty(): void { // delete physical foo.png file @unlink(self::FIXTURES . 'site_icons/foo.png'); @@ -217,7 +219,7 @@ public static function delete_icon_test_data(): array * @param $expected * @dataProvider delete_icon_test_data */ - public function test_delete_icon($icon, $exception, $expected) + public function test_delete_icon($icon, $exception, $expected): void { try { @@ -232,12 +234,12 @@ public function test_delete_icon($icon, $exception, $expected) $this->assertEquals($expected, $this->file_tracker->get_tracked_files()); } - public function test_resync_icons() + public function test_resync_icons(): void { // delete physical foo.png file @unlink(self::FIXTURES . 'site_icons/foo.png'); - // add new bar.png file + // add a new bar.png file @copy(self::FIXTURES . 'bar.png', self::FIXTURES . 'site_icons/bar.png'); // assert our storage tracking is currently still tracking the deleted image only diff --git a/tests/unit/upload_test.php b/tests/unit/upload_test.php index 0f5069f..251f25b 100644 --- a/tests/unit/upload_test.php +++ b/tests/unit/upload_test.php @@ -60,7 +60,7 @@ public static function upload_data(): array /** * @dataProvider upload_data */ - public function test_upload($file_move_success) + public function test_upload($file_move_success): void { $upload = $this->get_upload(); @@ -118,7 +118,7 @@ public function test_upload($file_move_success) } } - public function test_remove() + public function test_remove(): void { $upload = $this->get_upload();