Merge pull request #12074 from nextcloud/techdebt/noid/clean-unit-tests

fix(tests): Clean up unit test code
nextcloud · Apr 12, 2024 · a2bc199 · a2bc199
2 parents 3d98fa0 + f28f7e6
commit a2bc199
Show file tree

Hide file tree

Showing 49 changed files with 795 additions and 1,261 deletions.
diff --git a/tests/php/Activity/Provider/BaseTest.php b/tests/php/Activity/Provider/BaseTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <[email protected]>
  *
@@ -41,20 +43,13 @@
  * @package OCA\Talk\Tests\php\Activity
  */
 class BaseTest extends TestCase {
-	/** @var IFactory|MockObject */
-	protected $l10nFactory;
-	/** @var IURLGenerator|MockObject */
-	protected $url;
-	/** @var Config|MockObject */
-	protected $config;
-	/** @var IManager|MockObject */
-	protected $activityManager;
-	/** @var IUserManager|MockObject */
-	protected $userManager;
-	/** @var AvatarService|MockObject */
-	protected $avatarService;
-	/** @var Manager|MockObject */
-	protected $manager;
+	protected IFactory&MockObject $l10nFactory;
+	protected IURLGenerator&MockObject $url;
+	protected Config&MockObject $config;
+	protected IManager&MockObject $activityManager;
+	protected IUserManager&MockObject $userManager;
+	protected AvatarService&MockObject $avatarService;
+	protected Manager&MockObject $manager;
 
 	public function setUp(): void {
 		parent::setUp();
@@ -104,7 +99,7 @@ public static function dataPreParse(): array {
 	public function testPreParse(string $appId, bool $hasUser, bool $disabledForUser, bool $willThrowException): void {
 		$user = $hasUser ? $this->createMock(IUser::class) : null;
 
-		/** @var IEvent|MockObject $event */
+		/** @var IEvent&MockObject $event */
 		$event = $this->createMock(IEvent::class);
 		$event->expects($this->once())
 			->method('getApp')
@@ -135,8 +130,8 @@ public function testPreParse(string $appId, bool $hasUser, bool $disabledForUser
 		static::invokePrivate($provider, 'preParse', [$event]);
 	}
 
-	public function testPreParseThrows() {
-		/** @var IEvent|MockObject $event */
+	public function testPreParseThrows(): void {
+		/** @var IEvent&MockObject $event */
 		$event = $this->createMock(IEvent::class);
 		$event->expects($this->once())
 			->method('getApp')
@@ -146,7 +141,7 @@ public function testPreParseThrows() {
 		static::invokePrivate($provider, 'preParse', [$event]);
 	}
 
-	public static function dataSetSubject() {
+	public static function dataSetSubject(): array {
 		return [
 			['No placeholder', [], 'No placeholder'],
 			['This has one {placeholder}', ['placeholder' => ['name' => 'foobar']], 'This has one foobar'],
@@ -156,12 +151,8 @@ public static function dataSetSubject() {
 
 	/**
 	 * @dataProvider dataSetSubject
-	 *
-	 * @param string $subject
-	 * @param array $parameters
-	 * @param string $parsedSubject
 	 */
-	public function testSetSubject($subject, array $parameters, $parsedSubject) {
+	public function testSetSubject(string $subject, array $parameters, string $parsedSubject): void {
 		$provider = $this->getProvider();
 
 		$event = $this->createMock(IEvent::class);
@@ -177,7 +168,7 @@ public function testSetSubject($subject, array $parameters, $parsedSubject) {
 		self::invokePrivate($provider, 'setSubjects', [$event, $subject, $parameters]);
 	}
 
-	public static function dataGetRoom() {
+	public static function dataGetRoom(): array {
 		return [
 			[Room::TYPE_ONE_TO_ONE, 23, 'private-call', 'private-call', 'one2one'],
 			[Room::TYPE_GROUP, 42, 'group-call', 'group-call', 'group'],
@@ -190,14 +181,8 @@ public static function dataGetRoom() {
 
 	/**
 	 * @dataProvider dataGetRoom
-	 *
-	 * @param int $type
-	 * @param int $id
-	 * @param string $name
-	 * @param string $expectedName
-	 * @param string $expectedType
 	 */
-	public function testGetRoom($type, $id, $name, $expectedName, $expectedType) {
+	public function testGetRoom(int $type, int $id, string $name, string $expectedName, string $expectedType): void {
 		$provider = $this->getProvider();
 
 		$room = $this->createMock(Room::class);
@@ -239,10 +224,6 @@ public static function dataGetUser(): array {
 
 	/**
 	 * @dataProvider dataGetUser
-	 *
-	 * @param string $uid
-	 * @param bool $validUser
-	 * @param string $name
 	 */
 	public function testGetUser(string $uid, bool $validUser, string $name): void {
 		$provider = $this->getProvider();

diff --git a/tests/php/Activity/Provider/InvitationTest.php b/tests/php/Activity/Provider/InvitationTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <[email protected]>
  *
@@ -43,20 +45,13 @@
  * @package OCA\Talk\Tests\php\Activity
  */
 class InvitationTest extends TestCase {
-	/** @var IFactory|MockObject */
-	protected $l10nFactory;
-	/** @var IURLGenerator|MockObject */
-	protected $url;
-	/** @var Config|MockObject */
-	protected $config;
-	/** @var IManager|MockObject */
-	protected $activityManager;
-	/** @var IUserManager|MockObject */
-	protected $userManager;
-	/** @var AvatarService|MockObject */
-	protected $avatarService;
-	/** @var Manager|MockObject */
-	protected $manager;
+	protected IFactory&MockObject $l10nFactory;
+	protected IURLGenerator&MockObject $url;
+	protected Config&MockObject $config;
+	protected IManager&MockObject $activityManager;
+	protected IUserManager&MockObject $userManager;
+	protected AvatarService&MockObject $avatarService;
+	protected Manager&MockObject $manager;
 
 	public function setUp(): void {
 		parent::setUp();
@@ -100,8 +95,8 @@ protected function getProvider(array $methods = []) {
 		);
 	}
 
-	public function testParseThrowsWrongSubject() {
-		/** @var IEvent|MockObject $event */
+	public function testParseThrowsWrongSubject(): void {
+		/** @var IEvent&MockObject $event */
 		$event = $this->createMock(IEvent::class);
 		$event->expects($this->once())
 			->method('getApp')
@@ -128,7 +123,7 @@ public function testParseThrowsWrongSubject() {
 		$provider->parse('en', $event);
 	}
 
-	public static function dataParse() {
+	public static function dataParse(): array {
 		return [
 			['en', true, ['room' => 23, 'user' => 'test1'], ['actor' => ['actor-data'], 'call' => ['call-data']]],
 			['de', false, ['room' => 42, 'user' => 'test2'], ['actor' => ['actor-data'], 'call' => ['call-unknown']]],
@@ -137,24 +132,19 @@ public static function dataParse() {
 
 	/**
 	 * @dataProvider dataParse
-	 *
-	 * @param string $lang
-	 * @param bool $roomExists
-	 * @param array $params
-	 * @param array $expectedParams
 	 */
-	public function testParse($lang, $roomExists, array $params, array $expectedParams) {
+	public function testParse(string $lang, bool $roomExists, array $params, array $expectedParams): void {
 		$provider = $this->getProvider(['setSubjects', 'getUser', 'getRoom', 'getFormerRoom']);
 
-		/** @var IL10N|MockObject $l */
+		/** @var IL10N&MockObject $l */
 		$l = $this->createMock(IL10N::class);
 		$l->expects($this->any())
 			->method('t')
 			->willReturnCallback(function ($text, $parameters = []) {
 				return vsprintf($text, $parameters);
 			});
 
-		/** @var IEvent|MockObject $event */
+		/** @var IEvent&MockObject $event */
 		$event = $this->createMock(IEvent::class);
 		$event->expects($this->once())
 			->method('getApp')
@@ -180,7 +170,7 @@ public function testParse($lang, $roomExists, array $params, array $expectedPara
 			->willReturn(false);
 
 		if ($roomExists) {
-			/** @var Room|MockObject $room */
+			/** @var Room&MockObject $room */
 			$room = $this->createMock(Room::class);
 
 			$this->manager->expects($this->once())

diff --git a/tests/php/Activity/SettingTest.php b/tests/php/Activity/SettingTest.php
@@ -1,4 +1,6 @@
 <?php
+
+declare(strict_types=1);
 /**
  * @copyright Copyright (c) 2017 Joas Schilling <[email protected]>
  *
@@ -26,48 +28,44 @@
 use Test\TestCase;
 
 class SettingTest extends TestCase {
-	public static function dataSettings() {
+	public static function dataSettings(): array {
 		return [
 			[Setting::class],
 		];
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testImplementsInterface($settingClass) {
-		$setting = \OC::$server->get($settingClass);
+	public function testImplementsInterface(string $settingClass): void {
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertInstanceOf(ISetting::class, $setting);
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testGetIdentifier($settingClass) {
+	public function testGetIdentifier(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsString($setting->getIdentifier());
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testGetName($settingClass) {
+	public function testGetName(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsString($setting->getName());
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testGetPriority($settingClass) {
+	public function testGetPriority(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$priority = $setting->getPriority();
 		$this->assertIsInt($setting->getPriority());
 		$this->assertGreaterThanOrEqual(0, $priority);
@@ -76,41 +74,37 @@ public function testGetPriority($settingClass) {
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testCanChangeStream($settingClass) {
+	public function testCanChangeStream(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsBool($setting->canChangeStream());
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testIsDefaultEnabledStream($settingClass) {
+	public function testIsDefaultEnabledStream(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsBool($setting->isDefaultEnabledStream());
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testCanChangeMail($settingClass) {
+	public function testCanChangeMail(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsBool($setting->canChangeMail());
 	}
 
 	/**
 	 * @dataProvider dataSettings
-	 * @param string $settingClass
 	 */
-	public function testIsDefaultEnabledMail($settingClass) {
+	public function testIsDefaultEnabledMail(string $settingClass): void {
 		/** @var ISetting $setting */
-		$setting = \OC::$server->get($settingClass);
+		$setting = \OCP\Server::get($settingClass);
 		$this->assertIsBool($setting->isDefaultEnabledMail());
 	}
 }
diff --git a/tests/php/BackgroundJob/CheckHostedSignalingServerTest.php b/tests/php/BackgroundJob/CheckHostedSignalingServerTest.php
@@ -39,20 +39,13 @@
 use Test\TestCase;
 
 class CheckHostedSignalingServerTest extends TestCase {
-	/** @var ITimeFactory|MockObject */
-	protected $timeFactory;
-	/** @var HostedSignalingServerService|MockObject */
-	protected $hostedSignalingServerService;
-	/** @var IConfig|MockObject */
-	protected $config;
-	/** @var IManager|MockObject */
-	protected $notificationManager;
-	/** @var IGroupManager|MockObject */
-	protected $groupManager;
-	/** @var IURLGenerator|MockObject */
-	protected $urlGenerator;
-	/** @var LoggerInterface|MockObject */
-	protected $logger;
+	protected ITimeFactory&MockObject $timeFactory;
+	protected HostedSignalingServerService&MockObject $hostedSignalingServerService;
+	protected IConfig&MockObject $config;
+	protected IManager&MockObject $notificationManager;
+	protected IGroupManager&MockObject $groupManager;
+	protected IURLGenerator&MockObject $urlGenerator;
+	protected LoggerInterface&MockObject $logger;
 
 	public function setUp(): void {
 		parent::setUp();
@@ -78,7 +71,7 @@ public function getBackgroundJob(): CheckHostedSignalingServer {
 		);
 	}
 
-	public function testRunWithNoChange() {
+	public function testRunWithNoChange(): void {
 		$backgroundJob = $this->getBackgroundJob();
 
 		$this->config
@@ -92,10 +85,10 @@ public function testRunWithNoChange() {
 			->method('fetchAccountInfo')
 			->willReturn(['status' => 'pending']);
 
-		$this->invokePrivate($backgroundJob, 'run', ['']);
+		self::invokePrivate($backgroundJob, 'run', ['']);
 	}
 
-	public function testRunWithPendingToActiveChange() {
+	public function testRunWithPendingToActiveChange(): void {
 		$backgroundJob = $this->getBackgroundJob();
 		$newStatus = [
 			'status' => 'active',
@@ -142,6 +135,6 @@ public function testRunWithPendingToActiveChange() {
 			->method('fetchAccountInfo')
 			->willReturn($newStatus);
 
-		$this->invokePrivate($backgroundJob, 'run', ['']);
+		self::invokePrivate($backgroundJob, 'run', ['']);
 	}
 }