From efb7d21b52eb033530e80e5e49d175e6e3b031f4 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Fri, 2 Oct 2020 17:50:59 +0200 Subject: Add strict types for bookmarks management Parameters typing and using strict types overall increase the codebase quality by enforcing the a given parameter will have the expected type. It also removes the need to unnecessary unit tests checking methods behavior with invalid input. --- tests/HistoryTest.php | 8 ---- tests/bookmark/BookmarkArrayTest.php | 13 ------- tests/bookmark/BookmarkFileServiceTest.php | 44 ---------------------- tests/bookmark/BookmarkTest.php | 38 ------------------- .../DeleteBookmarkTest.php | 4 ++ .../SaveBookmarkTest.php | 20 +++++++--- .../controller/admin/ThumbnailsControllerTest.php | 4 +- 7 files changed, 22 insertions(+), 109 deletions(-) (limited to 'tests') diff --git a/tests/HistoryTest.php b/tests/HistoryTest.php index 6dc0e5b7..e810104e 100644 --- a/tests/HistoryTest.php +++ b/tests/HistoryTest.php @@ -89,14 +89,6 @@ class HistoryTest extends \Shaarli\TestCase $this->assertEquals(History::CREATED, $actual['event']); $this->assertTrue(new DateTime('-2 seconds') < $actual['datetime']); $this->assertEquals(1, $actual['id']); - - $history = new History(self::$historyFilePath); - $bookmark = (new Bookmark())->setId('str'); - $history->addLink($bookmark); - $actual = $history->getHistory()[0]; - $this->assertEquals(History::CREATED, $actual['event']); - $this->assertTrue(new DateTime('-2 seconds') < $actual['datetime']); - $this->assertEquals('str', $actual['id']); } // /** diff --git a/tests/bookmark/BookmarkArrayTest.php b/tests/bookmark/BookmarkArrayTest.php index ebed9bfc..1953078c 100644 --- a/tests/bookmark/BookmarkArrayTest.php +++ b/tests/bookmark/BookmarkArrayTest.php @@ -90,19 +90,6 @@ class BookmarkArrayTest extends TestCase $array['nope'] = $bookmark; } - /** - * Test adding a bad entry: invalid ID type - */ - public function testArrayAccessAddBadEntryIdType() - { - $this->expectException(\Shaarli\Bookmark\Exception\InvalidBookmarkException::class); - - $array = new BookmarkArray(); - $bookmark = (new Bookmark())->setId('nope'); - $bookmark->validate(); - $array[] = $bookmark; - } - /** * Test adding a bad entry: ID/offset not consistent */ diff --git a/tests/bookmark/BookmarkFileServiceTest.php b/tests/bookmark/BookmarkFileServiceTest.php index 6c56dfaa..59c0608c 100644 --- a/tests/bookmark/BookmarkFileServiceTest.php +++ b/tests/bookmark/BookmarkFileServiceTest.php @@ -264,17 +264,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->add(new Bookmark()); } - /** - * Test add() method with an entry which is not a bookmark instance - */ - public function testAddNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->add(['title' => 'hi!']); - } - /** * Test add() method with a Bookmark already containing an ID */ @@ -412,17 +401,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->set(new Bookmark()); } - /** - * Test set() method with an entry which is not a bookmark instance - */ - public function testSetNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->set(['title' => 'hi!']); - } - /** * Test set() method with a Bookmark without an ID defined. */ @@ -496,17 +474,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->addOrSet(new Bookmark()); } - /** - * Test addOrSet() method with an entry which is not a bookmark instance - */ - public function testAddOrSetNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->addOrSet(['title' => 'hi!']); - } - /** * Test addOrSet() method for a bookmark without any field set and without writing the data store */ @@ -564,17 +531,6 @@ class BookmarkFileServiceTest extends TestCase $this->publicLinkDB->remove($bookmark); } - /** - * Test remove() method with an entry which is not a bookmark instance - */ - public function testRemoveNotABookmark() - { - $this->expectException(\Exception::class); - $this->expectExceptionMessage('Provided data is invalid'); - - $this->privateLinkDB->remove(['title' => 'hi!']); - } - /** * Test remove() method with a Bookmark with an unknown ID */ diff --git a/tests/bookmark/BookmarkTest.php b/tests/bookmark/BookmarkTest.php index afec2440..4c7ae4c0 100644 --- a/tests/bookmark/BookmarkTest.php +++ b/tests/bookmark/BookmarkTest.php @@ -153,25 +153,6 @@ class BookmarkTest extends TestCase $this->assertContainsPolyfill('- ID: '. PHP_EOL, $exception->getMessage()); } - /** - * Test validate() with a a bookmark with a non integer ID. - */ - public function testValidateNotValidStringId() - { - $bookmark = new Bookmark(); - $bookmark->setId('str'); - $bookmark->setShortUrl('abc'); - $bookmark->setCreated(\DateTime::createFromFormat('Ymd_His', '20190514_200102')); - $exception = null; - try { - $bookmark->validate(); - } catch (InvalidBookmarkException $e) { - $exception = $e; - } - $this->assertNotNull($exception); - $this->assertContainsPolyfill('- ID: str'. PHP_EOL, $exception->getMessage()); - } - /** * Test validate() with a a bookmark without short url. */ @@ -210,25 +191,6 @@ class BookmarkTest extends TestCase $this->assertContainsPolyfill('- Created: '. PHP_EOL, $exception->getMessage()); } - /** - * Test validate() with a a bookmark with a bad created datetime. - */ - public function testValidateNotValidBadCreated() - { - $bookmark = new Bookmark(); - $bookmark->setId(1); - $bookmark->setShortUrl('abc'); - $bookmark->setCreated('hi!'); - $exception = null; - try { - $bookmark->validate(); - } catch (InvalidBookmarkException $e) { - $exception = $e; - } - $this->assertNotNull($exception); - $this->assertContainsPolyfill('- Created: Not a DateTime object'. PHP_EOL, $exception->getMessage()); - } - /** * Test setId() and make sure that default fields are generated. */ diff --git a/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php b/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php index ba774e21..83bbee7c 100644 --- a/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php +++ b/tests/front/controller/admin/ManageShaareControllerTest/DeleteBookmarkTest.php @@ -356,6 +356,10 @@ class DeleteBookmarkTest extends TestCase ; $response = new Response(); + $this->container->bookmarkService->method('get')->with('123')->willReturn( + (new Bookmark())->setId(123)->setUrl('http://domain.tld')->setTitle('Title 123') + ); + $this->container->formatterFactory = $this->createMock(FormatterFactory::class); $this->container->formatterFactory ->expects(static::once()) diff --git a/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php b/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php index f7a68226..37542c26 100644 --- a/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php +++ b/tests/front/controller/admin/ManageShaareControllerTest/SaveBookmarkTest.php @@ -66,23 +66,27 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertFalse($save); $checkBookmark($bookmark); $bookmark->setId($id); + + return $bookmark; }) ; $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertTrue($save); $checkBookmark($bookmark); static::assertSame($id, $bookmark->getId()); + + return $bookmark; }) ; @@ -155,21 +159,25 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertFalse($save); $checkBookmark($bookmark); + + return $bookmark; }) ; $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($checkBookmark, $id): Bookmark { static::assertTrue($save); $checkBookmark($bookmark); static::assertSame($id, $bookmark->getId()); + + return $bookmark; }) ; @@ -230,8 +238,10 @@ class SaveBookmarkTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('addOrSet') - ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($thumb): void { + ->willReturnCallback(function (Bookmark $bookmark, bool $save) use ($thumb): Bookmark { static::assertSame($thumb, $bookmark->getThumbnail()); + + return $bookmark; }) ; diff --git a/tests/front/controller/admin/ThumbnailsControllerTest.php b/tests/front/controller/admin/ThumbnailsControllerTest.php index f4a8acff..e5749654 100644 --- a/tests/front/controller/admin/ThumbnailsControllerTest.php +++ b/tests/front/controller/admin/ThumbnailsControllerTest.php @@ -89,8 +89,10 @@ class ThumbnailsControllerTest extends TestCase $this->container->bookmarkService ->expects(static::once()) ->method('set') - ->willReturnCallback(function (Bookmark $bookmark) use ($thumb) { + ->willReturnCallback(function (Bookmark $bookmark) use ($thumb): Bookmark { static::assertSame($thumb, $bookmark->getThumbnail()); + + return $bookmark; }) ; -- cgit v1.2.3