Merge pull request #34871 from owncloud/add-assert-to-risky-unit-test…

…s-phpunit-5 [master] Add explicit PHPUnit asserts to unit tests flagged as 'risky' by PHPUnit6
owncloud · Mar 28, 2019 · 3f4a433 · 3f4a433
2 parents 568f5b1 + d83e81a
commit 3f4a433
Show file tree

Hide file tree

Showing 13 changed files with 88 additions and 27 deletions.
diff --git a/apps/federatedfilesharing/lib/Middleware/OcmMiddleware.php b/apps/federatedfilesharing/lib/Middleware/OcmMiddleware.php
@@ -93,7 +93,7 @@ public function __construct(
 	 *
 	 * @param string[] $params
 	 *
-	 * @return bool
+	 * @return void
 	 *
 	 * @throws BadRequestException
 	 */

diff --git a/apps/federatedfilesharing/tests/Middleware/OcmMiddlewareTest.php b/apps/federatedfilesharing/tests/Middleware/OcmMiddlewareTest.php
@@ -102,7 +102,9 @@ public function testAssertNotNull($input, $expectedResult) {
 		if (\is_string($expectedResult)) {
 			$this->expectException($expectedResult);
 		}
-		$this->ocmMiddleware->assertNotNull($input);
+		$this->assertNull(
+			$this->ocmMiddleware->assertNotNull($input)
+		);
 	}
 
 	public function dataTestAssertNotNull() {
@@ -204,7 +206,9 @@ public function testAssertIncomingSharingEnabled($isSharingAppEnabled, $isIncomi
 		if ($isExceptionExpected) {
 			$this->expectException(NotImplementedException::class);
 		}
-		$this->ocmMiddleware->assertIncomingSharingEnabled();
+		$this->assertNull(
+			$this->ocmMiddleware->assertIncomingSharingEnabled()
+		);
 	}
 
 	public function dataTestAssertIncomingSharingEnabled() {
@@ -225,7 +229,9 @@ public function testAssertOutgoingSharingEnabled($isSharingAppEnabled, $isOutgoi
 		if ($isExceptionExpected) {
 			$this->expectException(NotImplementedException::class);
 		}
-		$this->ocmMiddleware->assertOutgoingSharingEnabled();
+		$this->assertNull(
+			$this->ocmMiddleware->assertOutgoingSharingEnabled()
+		);
 	}
 
 	public function dataTestAssertOutgoingSharingEnabled() {

diff --git a/apps/files/tests/Command/ScanTest.php b/apps/files/tests/Command/ScanTest.php
@@ -239,12 +239,20 @@ public function testMultipleGroups($input) {
 		if (\count($groups) === 2) {
 			$this->assertContains('Starting scan for user 1 out of 10 (user1)', $output);
 			$this->assertContains('Starting scan for user 1 out of 10 (user11)', $output);
-		}
-		if (\count($groups) === 3) {
+		} elseif (\count($groups) === 3) {
 			$this->assertContains('Starting scan for user 1 out of 10 (user1)', $output);
 			$this->assertContains('Starting scan for user 1 out of 10 (user11)', $output);
 			$this->assertContains('Starting scan for user 1 out of 10 (user21)', $output);
 			$this->assertContains('Starting scan for user 10 out of 10 (user30)', $output);
+		} elseif (\count($groups) === 4) {
+			$this->assertContains('Starting scan for user 1 out of 10 (user1)', $output);
+			$this->assertContains('Starting scan for user 1 out of 20 (user11)', $output);
+			$this->assertContains('Starting scan for user 11 out of 20 (user21)', $output);
+			$this->assertContains('Starting scan for user 10 out of 10 (user40)', $output);
+		} else {
+			$this->fail(
+				"testMultipleGroups supports testing with 2,3, or 4 groups but the input has $groups groups"
+			);
 		}
 	}
 

diff --git a/apps/files_sharing/tests/AppInfo/ApplicationTest.php b/apps/files_sharing/tests/AppInfo/ApplicationTest.php
@@ -31,6 +31,8 @@
 class ApplicationTest extends TestCase {
 	public function testConstructor() {
 		$app = new Application();
-		$app->getContainer()->query('Share20OcsController');
+		$this->assertNotNull(
+			$app->getContainer()->query('Share20OcsController')
+		);
 	}
 }
diff --git a/apps/files_trashbin/tests/StorageTest.php b/apps/files_trashbin/tests/StorageTest.php
@@ -747,9 +747,13 @@ public function testSingleStorageDeleteFileLoggedOut() {
 		$this->logout();
 
 		if (!$this->userView->file_exists('test.txt')) {
-			$this->markTestSkipped('Skipping since the current home storage backend requires the user to logged in');
+			$this->markTestSkipped(
+				'Skipping since the current home storage backend requires the user to be logged in'
+			);
 		} else {
-			$this->userView->unlink('test.txt');
+			$this->assertNotNull(
+				$this->userView->unlink('test.txt')
+			);
 		}
 	}
 

diff --git a/apps/files_versions/tests/FileHelperTest.php b/apps/files_versions/tests/FileHelperTest.php
@@ -41,7 +41,9 @@ public function testCreateMissingDirectories() {
 				[$this->equalTo(FileHelper::VERSIONS_RELATIVE_PATH . '/some/long/path')],
 				[$this->equalTo(FileHelper::VERSIONS_RELATIVE_PATH . '/some/long/path/to')]
 			);
-		$fileHelper->createMissingDirectories($viewMock, $path);
+		$this->assertNull(
+			$fileHelper->createMissingDirectories($viewMock, $path)
+		);
 	}
 
 	/**

diff --git a/tests/lib/DB/ConnectionTest.php b/tests/lib/DB/ConnectionTest.php
@@ -12,6 +12,7 @@
 use Doctrine\DBAL\Platforms\SqlitePlatform;
 use OC\DB\MDB2SchemaManager;
 use OCP\DB\QueryBuilder\IQueryBuilder;
+use PHPUnit_Framework_Constraint_IsType as IsType;
 
 /**
  * Class Connection
@@ -181,19 +182,23 @@ public function testSetValuesOverWritePreconditionFailed() {
 
 	public function testSetValuesSameNoError() {
 		$this->makeTestTable();
-		$this->connection->setValues('table', [
+		$numNewRows = $this->connection->setValues('table', [
 			'integerfield' => 1
 		], [
 			'textfield' => 'foo',
 			'clobfield' => 'not_null'
 		]);
 
+		$this->assertInternalType(IsType::TYPE_INT, $numNewRows);
+
 		// this will result in 'no affected rows' on certain optimizing DBs
 		// ensure the PreConditionNotMetException isn't thrown
-		$this->connection->setValues('table', [
+		$numNewRows = $this->connection->setValues('table', [
 			'integerfield' => 1
 		], [
 			'textfield' => 'foo'
 		]);
+
+		$this->assertInternalType(IsType::TYPE_INT, $numNewRows);
 	}
 }
diff --git a/tests/lib/Events/EventEmitterTraitTest.php b/tests/lib/Events/EventEmitterTraitTest.php
@@ -97,6 +97,10 @@ public function testEmittingCallWithAdditionalArgument($className, $eventName, $
 			$this->assertInstanceOf(GenericEvent::class, $calledAfterEvent[1]);
 			$this->assertArrayHasKey('item', $calledAfterEvent[1]);
 			$this->assertEquals('testing', $calledAfterEvent[1]->getArgument('item'));
+		} else {
+			$this->assertEquals($calledAfterEvent[0], "$className.after$eventName");
+			$this->assertArrayHasKey('item', $calledAfterEvent[1]);
+			$this->assertEquals('testing', $calledAfterEvent[1]->getArgument('item'));
 		}
 	}
 }
diff --git a/tests/lib/Files/Storage/LocalTest.php b/tests/lib/Files/Storage/LocalTest.php
@@ -23,6 +23,7 @@
 namespace Test\Files\Storage;
 
 use OC\Files\Storage\Local;
+use PHPUnit_Framework_Constraint_IsType as IsType;
 
 /**
  * Class LocalTest
@@ -99,7 +100,7 @@ public function testDisallowSymlinksOutsideDatadir() {
 		$storage->file_put_contents('sym/foo', 'bar');
 	}
 
-	public function testDisallowSymlinksInsideDatadir() {
+	public function testAllowSymlinksInsideDatadir() {
 		$subDir1 = $this->tmpDir . 'sub1';
 		$subDir2 = $this->tmpDir . 'sub1/sub2';
 		$sym = $this->tmpDir . 'sub1/sym';
@@ -110,7 +111,8 @@ public function testDisallowSymlinksInsideDatadir() {
 
 		$storage = new \OC\Files\Storage\Local(['datadir' => $subDir1]);
 
-		$storage->file_put_contents('sym/foo', 'bar');
+		$numBytes = $storage->file_put_contents('sym/foo', 'bar');
+		$this->assertInternalType(IsType::TYPE_INT, $numBytes);
 	}
 
 	/**

diff --git a/tests/lib/Group/Backend.php b/tests/lib/Group/Backend.php
@@ -155,7 +155,13 @@ public function testSearchUsers() {
 	public function testAddDouble() {
 		$group = $this->getGroupName();
 
-		$this->backend->createGroup($group);
-		$this->backend->createGroup($group);
+		$this->assertTrue(
+			$this->backend->createGroup($group),
+			"there was a problem creating $group"
+		);
+		$this->assertFalse(
+			$this->backend->createGroup($group),
+			"there was a problem creating $group a second time"
+		);
 	}
 }
diff --git a/tests/lib/Repair/RepairMismatchFileCachePathTest.php b/tests/lib/Repair/RepairMismatchFileCachePathTest.php
@@ -792,6 +792,6 @@ public function testVersions($version) {
 				->method('info')
 				->with($this->repair->getName() . ' is not executed');
 		}
-		$this->repair->run($outputMock);
+		$this->assertNull($this->repair->run($outputMock));
 	}
 }
diff --git a/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php b/tests/lib/Security/CSP/ContentSecurityPolicyManagerTest.php
@@ -36,7 +36,11 @@ public function setUp() {
 	}
 
 	public function testAddDefaultPolicy() {
-		$this->contentSecurityPolicyManager->addDefaultPolicy(new \OCP\AppFramework\Http\ContentSecurityPolicy());
+		$this->assertNull(
+			$this->contentSecurityPolicyManager->addDefaultPolicy(
+				new \OCP\AppFramework\Http\ContentSecurityPolicy()
+			)
+		);
 	}
 
 	public function testGetDefaultPolicyWithPolicies() {

diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php
@@ -1202,7 +1202,9 @@ public function testUserCreateChecksShareWithGroupMembersOnlySharedGroup() {
 			->with($path)
 			->willReturn([]);
 
-		$this->invokePrivate($this->manager, 'userCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'userCreateChecks', [$share])
+		);
 	}
 
 	/**
@@ -1337,7 +1339,9 @@ public function testUserCreateChecksIdenticalPathNotSharedWithUser() {
 			->with($path)
 			->willReturn([$share2]);
 
-		$this->invokePrivate($this->manager, 'userCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'userCreateChecks', [$share])
+		);
 	}
 
 	/**
@@ -1431,7 +1435,9 @@ public function testGroupCreateChecksShareWithGroupMembersOnlyInGroup() {
 				['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
 			]));
 
-		$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'groupCreateChecks', [$share])
+		);
 	}
 
 	/**
@@ -1486,7 +1492,9 @@ public function testGroupCreateChecksPathAlreadySharedWithDifferentGroup() {
 				['core', 'shareapi_allow_group_sharing', 'yes', 'yes'],
 			]));
 
-		$this->invokePrivate($this->manager, 'groupCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'groupCreateChecks', [$share])
+		);
 	}
 
 	/**
@@ -1554,7 +1562,9 @@ public function testLinkCreateChecksPublicUpload() {
 				['core', 'shareapi_allow_public_upload', 'yes', 'yes']
 			]));
 
-		$this->invokePrivate($this->manager, 'linkCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'linkCreateChecks', [$share])
+		);
 	}
 
 	public function testLinkCreateChecksReadOnly() {
@@ -1569,7 +1579,9 @@ public function testLinkCreateChecksReadOnly() {
 				['core', 'shareapi_allow_public_upload', 'yes', 'no']
 			]));
 
-		$this->invokePrivate($this->manager, 'linkCreateChecks', [$share]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'linkCreateChecks', [$share])
+		);
 	}
 
 	/**
@@ -1601,13 +1613,17 @@ public function testPathCreateChecksContainsNoSharedMount() {
 
 		$this->mountManager->method('findIn')->with('path')->willReturn([$mount]);
 
-		$this->invokePrivate($this->manager, 'pathCreateChecks', [$path]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'pathCreateChecks', [$path])
+		);
 	}
 
 	public function testPathCreateChecksContainsNoFolder() {
 		$path = $this->createMock(File::class);
 
-		$this->invokePrivate($this->manager, 'pathCreateChecks', [$path]);
+		$this->assertNull(
+			$this->invokePrivate($this->manager, 'pathCreateChecks', [$path])
+		);
 	}
 
 	public function dataIsSharingDisabledForUser() {
@@ -3427,7 +3443,9 @@ public function testUpdateShareForRecipient() {
 
 		$this->defaultProvider->method('move')->with($share, 'recipient')->will($this->returnArgument(0));
 
-		$this->manager->updateShareForRecipient($share, 'recipient');
+		$this->assertNull(
+			$this->manager->updateShareForRecipient($share, 'recipient')
+		);
 	}
 
 	public function testGetSharedWith() {