From 525fb7a25d6d43e02111b05a85fed4813637ec27 Mon Sep 17 00:00:00 2001 From: marcelmanzel Date: Tue, 16 Jul 2024 19:07:40 +0200 Subject: [PATCH] OXDEV-8213: Refactor tests after review --- .../Controller/ThemeListControllerTest.php | 4 +- .../DataType/ThemeDataTypeFactoryTest.php | 6 +- .../Unit/Theme/DataType/ThemeFiltersTest.php | 67 ++++++++++------- .../ThemeListInfrastructureTest.php | 47 ++++++------ .../Theme/Service/ThemeFilterServiceTest.php | 72 +++++++++++++++---- .../Theme/Service/ThemeListServiceTest.php | 27 ++++--- 6 files changed, 140 insertions(+), 83 deletions(-) diff --git a/tests/Unit/Theme/Controller/ThemeListControllerTest.php b/tests/Unit/Theme/Controller/ThemeListControllerTest.php index 5889adc..20c044c 100644 --- a/tests/Unit/Theme/Controller/ThemeListControllerTest.php +++ b/tests/Unit/Theme/Controller/ThemeListControllerTest.php @@ -31,7 +31,7 @@ public function testThemesListWithFilter(): void ); $themeListServiceMock = $this->createMock(ThemeListServiceInterface::class); - $themeListServiceMock + $themeListServiceMock->expects($this->once()) ->method('getThemeList') ->with($themeFilters) ->willReturn([$theme]); @@ -48,7 +48,7 @@ public function testThemesListWithoutFilter(): void $themeFilters = new ThemeFilters(); $themeListServiceMock = $this->createMock(ThemeListServiceInterface::class); - $themeListServiceMock + $themeListServiceMock->expects($this->once()) ->method('getThemeList') ->with($themeFilters) ->willReturn([$theme]); diff --git a/tests/Unit/Theme/DataType/ThemeDataTypeFactoryTest.php b/tests/Unit/Theme/DataType/ThemeDataTypeFactoryTest.php index 5a0516c..217632e 100644 --- a/tests/Unit/Theme/DataType/ThemeDataTypeFactoryTest.php +++ b/tests/Unit/Theme/DataType/ThemeDataTypeFactoryTest.php @@ -29,14 +29,14 @@ public function testCreateThemeDataType(): void $expectedDescription = uniqid(); $expectedActive = true; - $themeMock->method('getInfo') - ->will($this->returnValueMap([ + $themeMock->expects($this->exactly(5))->method('getInfo') + ->willReturnMap([ ['title', $expectedTitle], ['id', $expectedIdentifier], ['version', $expectedVersion], ['description', $expectedDescription], ['active', $expectedActive], - ])); + ]); $factory = new ThemeDataTypeFactory(); $themeDataType = $factory->createFromCoreTheme($themeMock); diff --git a/tests/Unit/Theme/DataType/ThemeFiltersTest.php b/tests/Unit/Theme/DataType/ThemeFiltersTest.php index 9b972bd..544f087 100644 --- a/tests/Unit/Theme/DataType/ThemeFiltersTest.php +++ b/tests/Unit/Theme/DataType/ThemeFiltersTest.php @@ -22,84 +22,97 @@ class ThemeFiltersTest extends TestCase { /** @dataProvider themeByTitleDataProvider */ public function testFilterThemeByTitle( - string $expectedThemeTitle, bool $expectedResult, - bool $enableFilters, ): void { + $title = uniqid(); $stringFilterMock = $this->createMock(StringFilter::class); - $stringFilterMock->method('matches')->willReturn($expectedResult); + $stringFilterMock->expects($this->once())->method('matches')->with($title)->willReturn($expectedResult); $themeMock = $this->createMock(ThemeDataType::class); - $themeMock->method('getTitle')->willReturn($expectedThemeTitle); + $themeMock->expects($this->once())->method('getTitle')->willReturn($title); - $themeFilters = ($enableFilters) ? new ThemeFilters(titleFilter: $stringFilterMock) : new ThemeFilters(); + $themeFilters = new ThemeFilters(titleFilter: $stringFilterMock); $this->assertEquals($expectedResult, $themeFilters->filterThemeByTitle($themeMock)); } public static function themeByTitleDataProvider(): \Generator { yield "filter theme by titles matches" => [ - 'expectedThemeTitle' => 'test theme 1', 'expectedResult' => true, - 'enableFilters' => true ]; yield "filter theme by titles do not matches" => [ - 'expectedThemeTitle' => 'random theme title', 'expectedResult' => false, - 'enableFilters' => true ]; + } - yield "filter theme by title no filters" => [ - 'expectedThemeTitle' => 'test theme 1', - 'expectedResult' => true, - 'enableFilters' => false, - ]; + public function testThemeFiltersWithoutFilter(): void + { + $themeMock = $this->createStub(ThemeDataType::class); + + $themeFilters = new ThemeFilters(); + $this->assertTrue($themeFilters->filterThemeByTitle($themeMock)); + $this->assertTrue($themeFilters->filterThemeByStatus($themeMock)); } /** @dataProvider themeByStatusDataProvider */ public function testFilterThemeByStatus( - bool $expectedThemeStatus, + bool $filterStatus, bool $actualThemeStatus, bool $expectedResult ): void { $mockBoolFilter = $this->createMock(BoolFilter::class); - $mockBoolFilter->method('equals')->willReturn($expectedThemeStatus); + $mockBoolFilter->expects($this->exactly(2))->method('equals')->willReturn($filterStatus); $themeFilterList = new ThemeFilters(activeFilter: $mockBoolFilter); $mockThemeDataType = $this->createMock(ThemeDataType::class); - $mockThemeDataType->method('isActive')->willReturn($actualThemeStatus); + $mockThemeDataType->expects($this->once())->method('isActive')->willReturn($actualThemeStatus); $this->assertSame($expectedResult, $themeFilterList->filterThemeByStatus($mockThemeDataType)); } public static function themeByStatusDataProvider(): \Generator { - yield "filter theme by providing same theme status" => [ - 'expectedThemeStatus' => true, + yield "filter theme by true with same theme status" => [ + 'filterStatus' => true, 'actualThemeStatus' => true, 'expectedResult' => true ]; - yield "filter theme by providing different theme status" => [ - 'expectedThemeStatus' => true, + yield "filter theme by false with same theme status" => [ + 'filterStatus' => false, + 'actualThemeStatus' => false, + 'expectedResult' => true + ]; + + yield "filter theme by true with different theme status" => [ + 'filterStatus' => true, 'actualThemeStatus' => false, 'expectedResult' => false ]; + + yield "filter theme by false with different theme status" => [ + 'filterStatus' => false, + 'actualThemeStatus' => true, + 'expectedResult' => false + ]; } public function testCreateThemeFilterList(): void { - $stringFilter = $this->createMock(StringFilter::class); - $boolFilter = $this->createMock(BoolFilter::class); + $stringFilter = $this->createStub(StringFilter::class); + $boolFilter = $this->createStub(BoolFilter::class); + + $expectedThemeFilters = new ThemeFilters(titleFilter: $stringFilter, activeFilter: $boolFilter); - $themeFilterList = ThemeFilters::createThemeFilters($stringFilter, $boolFilter); - $this->assertInstanceOf(ThemeFilters::class, $themeFilterList); + $themeFilters = ThemeFilters::createThemeFilters($stringFilter, $boolFilter); + $this->assertEquals($expectedThemeFilters, $themeFilters); } public function testCreateThemeFilterListWithNull(): void { - $themeFilterList = ThemeFilters::createThemeFilters(null, null); - $this->assertInstanceOf(ThemeFilters::class, $themeFilterList); + $expectedThemeFilters = new ThemeFilters(); + $themeFilters = ThemeFilters::createThemeFilters(null, null); + $this->assertEquals($expectedThemeFilters, $themeFilters); } } diff --git a/tests/Unit/Theme/Infrastructure/ThemeListInfrastructureTest.php b/tests/Unit/Theme/Infrastructure/ThemeListInfrastructureTest.php index d88b47c..5ef3d8e 100644 --- a/tests/Unit/Theme/Infrastructure/ThemeListInfrastructureTest.php +++ b/tests/Unit/Theme/Infrastructure/ThemeListInfrastructureTest.php @@ -15,6 +15,7 @@ use OxidEsales\GraphQL\ConfigurationAccess\Theme\DataType\ThemeDataType; use OxidEsales\GraphQL\ConfigurationAccess\Theme\Exception\ThemesNotFound; use OxidEsales\GraphQL\ConfigurationAccess\Theme\Infrastructure\ThemeListInfrastructure; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; /** @@ -26,15 +27,8 @@ public function testGetThemes(): void { $theme1 = $this->createThemeMock(uniqid(), uniqid(), uniqid(), uniqid(), true); $theme2 = $this->createThemeMock(uniqid(), uniqid(), uniqid(), uniqid(), false); - $expectedThemes = [$theme1, $theme2]; - $coreThemeMock = $this->createMock(Theme::class); - $coreThemeMock->method('getList') - ->willReturn($expectedThemes); - - $coreThemeFactoryMock = $this->getCoreThemeFactoryMock($coreThemeMock); - $themeDataTypeFactoryMock = $this->createMock(ThemeDataTypeFactoryInterface::class); - $themeDataTypeFactoryMock->method('createFromCoreTheme')->will($this->returnCallback(function (Theme $theme) { + $convertThemeCallback = function (Theme $theme) { return new ThemeDataType( $theme->getInfo('title'), $theme->getInfo('id'), @@ -42,27 +36,31 @@ public function testGetThemes(): void $theme->getInfo('description'), $theme->getInfo('active') ); - })); + }; + $theme1DataType = $convertThemeCallback($theme1); + $theme2DataType = $convertThemeCallback($theme2); + + $coreThemeMock = $this->createMock(Theme::class); + $coreThemeMock->expects($this->once())->method('getList') + ->willReturn([$theme1, $theme2]); + + $coreThemeFactoryMock = $this->getCoreThemeFactoryMock($coreThemeMock); + $themeDataTypeFactoryMock = $this->createMock(ThemeDataTypeFactoryInterface::class); + $themeDataTypeFactoryMock->expects($this->exactly(2))->method('createFromCoreTheme') + ->with($this->logicalOr($theme1, $theme2)) + ->willReturnCallback($convertThemeCallback); $sut = $this->getSut(coreThemeFactory: $coreThemeFactoryMock, themeDataTypeFactory: $themeDataTypeFactoryMock); $actualThemesArray = $sut->getThemes(); - $actualTheme1 = $actualThemesArray[0]; - $actualTheme2 = $actualThemesArray[1]; - - $this->assertCount(2, $actualThemesArray); - $this->assertInstanceOf(ThemeDataType::class, $actualTheme1); - $this->assertSame($theme1->getInfo('title'), $actualTheme1->getTitle()); - $this->assertSame($theme2->getInfo('title'), $actualTheme2->getTitle()); - $this->assertSame($theme1->getInfo('active'), $actualTheme1->isActive()); - $this->assertSame($theme2->getInfo('active'), $actualTheme2->isActive()); + $this->assertEquals([$theme1DataType, $theme2DataType], $actualThemesArray); } public function testGetThemesThrowsException(): void { - $coreThemeStub = $this->createMock(Theme::class); - $coreThemeStub->method('getList') + $coreThemeMock = $this->createMock(Theme::class); + $coreThemeMock->expects($this->once())->method('getList') ->willReturn([]); - $coreThemeFactoryMock = $this->getCoreThemeFactoryMock($coreThemeStub); + $coreThemeFactoryMock = $this->getCoreThemeFactoryMock($coreThemeMock); $themeDataTypeFactoryMock = $this->createMock(ThemeDataTypeFactoryInterface::class); $sut = $this->getSut(coreThemeFactory: $coreThemeFactoryMock, themeDataTypeFactory: $themeDataTypeFactoryMock); @@ -77,9 +75,10 @@ private function createThemeMock( string $version, string $description, bool $active - ) { + ): Theme|MockObject { $themeMock = $this->createMock(Theme::class); - $themeMock->method('getInfo') + $themeMock->expects($this->exactly(10)) + ->method('getInfo') ->willReturnMap([ ['title', $title], ['id', $id], @@ -101,7 +100,7 @@ private function getSut( ); } - private function getCoreThemeFactoryMock(mixed $returnValue): CoreThemeFactoryInterface + private function getCoreThemeFactoryMock(Theme $returnValue): CoreThemeFactoryInterface { $coreThemeFactoryMock = $this->createMock(CoreThemeFactoryInterface::class); $coreThemeFactoryMock->expects($this->once()) diff --git a/tests/Unit/Theme/Service/ThemeFilterServiceTest.php b/tests/Unit/Theme/Service/ThemeFilterServiceTest.php index 829ba55..be5ca0e 100644 --- a/tests/Unit/Theme/Service/ThemeFilterServiceTest.php +++ b/tests/Unit/Theme/Service/ThemeFilterServiceTest.php @@ -19,23 +19,69 @@ */ class ThemeFilterServiceTest extends TestCase { - public function testFilterThemes(): void - { - $theme1 = new ThemeDataType(uniqid(), uniqid(), uniqid(), uniqid(), true); - $theme2 = new ThemeDataType(uniqid(), uniqid(), uniqid(), uniqid(), false); - $themesList = [$theme1,$theme2]; - + /** @dataProvider themeFilterResultProvider */ + public function testFilterThemes( + array $themeList, + array $titleFilterResults, + array $statusFilterResults, + array $expectedThemeListResult + ): void { $themeFiltersMock = $this->createMock(ThemeFiltersInterface::class); - $themeFiltersMock->method('filterThemeByTitle') - ->willReturnCallback(function (ThemeDataType $theme) { - return str_contains($theme->getTitle(), 'Test'); + $themeFiltersMock->expects($this->exactly(count($titleFilterResults)))->method('filterThemeByTitle') + ->willReturnCallback(function (ThemeDataType $theme) use (&$titleFilterResults) { + return array_shift($titleFilterResults); }); - $themeFiltersMock->method('filterThemeByStatus') - ->willReturnCallback(function (ThemeDataType $theme) { - return $theme->isActive(); + $themeFiltersMock->expects($this->exactly(count($statusFilterResults)))->method('filterThemeByStatus') + ->willReturnCallback(function (ThemeDataType $theme) use (&$statusFilterResults) { + return array_shift($statusFilterResults); }); $themeFilterService = new ThemeFilterService(); - $themeFilterService->filterThemes($themesList, $themeFiltersMock); + $themeListResult = $themeFilterService->filterThemes($themeList, $themeFiltersMock); + $this->assertCount(count($expectedThemeListResult), $themeListResult); + foreach ($expectedThemeListResult as $theme) { + $this->assertContains($theme, $themeListResult); + } + } + + public static function themeFilterResultProvider(): \Generator + { + $theme1 = new ThemeDataType('theme1', uniqid(), uniqid(), uniqid(), true); + $theme2 = new ThemeDataType('theme2', uniqid(), uniqid(), uniqid(), false); + + yield "filter with mixed results" => [ + 'themeList' => [$theme1, $theme2], + 'titleFilterResults' => [false, true], + 'statusFilterResults' => [false], + 'expectedThemeListResult' => [] + ]; + + yield "filter with all false" => [ + 'themeList' => [$theme1, $theme2], + 'titleFilterResults' => [false, false], + 'statusFilterResults' => [], + 'expectedThemeListResult' => [] + ]; + + yield "filter with first true" => [ + 'themeList' => [$theme1, $theme2], + 'titleFilterResults' => [true, false], + 'statusFilterResults' => [true], + 'expectedThemeListResult' => [$theme1] + ]; + + yield "filter with seconde true" => [ + 'themeList' => [$theme1, $theme2], + 'titleFilterResults' => [false, true], + 'statusFilterResults' => [true], + 'expectedThemeListResult' => [$theme2] + ]; + + yield "filter with all true" => [ + 'themeList' => [$theme1, $theme2], + 'titleFilterResults' => [true, true], + 'statusFilterResults' => [true, true], + 'expectedThemeListResult' => [$theme1, $theme2] + ]; } } diff --git a/tests/Unit/Theme/Service/ThemeListServiceTest.php b/tests/Unit/Theme/Service/ThemeListServiceTest.php index 18d1fa1..271cb93 100644 --- a/tests/Unit/Theme/Service/ThemeListServiceTest.php +++ b/tests/Unit/Theme/Service/ThemeListServiceTest.php @@ -27,29 +27,28 @@ public function testGetThemeListWithFilters(): void { $theme1 = new ThemeDataType(uniqid(), uniqid(), uniqid(), uniqid(), true); $theme2 = new ThemeDataType(uniqid(), uniqid(), uniqid(), uniqid(), false); + $themeList = [$theme1, $theme2]; + $filteredThemeList = [$theme1]; $themeListInfrastructureMock = $this->createMock(ThemeListInfrastructureInterface::class); - $themeListInfrastructureMock->method('getThemes') - ->willReturn([$theme1,$theme2]); - - $themeFilterServiceMock = $this->createMock(ThemeFilterServiceInterface::class); - $themeFilterServiceMock->method('filterThemes') - ->willReturn([$theme1]); + $themeListInfrastructureMock->expects($this->once())->method('getThemes') + ->willReturn($themeList); $filtersList = new ThemeFilters( - titleFilter: new StringFilter(contains: $theme1->getTitle()), - activeFilter: new BoolFilter(equals: $theme1->isActive()) + titleFilter: new StringFilter(contains: uniqid()), + activeFilter: new BoolFilter(equals: (bool)rand(0, 1)) ); + + $themeFilterServiceMock = $this->createMock(ThemeFilterServiceInterface::class); + $themeFilterServiceMock->expects($this->once())->method('filterThemes') + ->with($themeList, $filtersList) + ->willReturn($filteredThemeList); + $themeListService = new ThemeListService( themeListInfrastructure: $themeListInfrastructureMock, themeFilterService: $themeFilterServiceMock ); $actualThemes = $themeListService->getThemeList($filtersList); - $actualTheme = $actualThemes[0]; - - $this->assertCount(1, $actualThemes); - $this->assertSame($theme1->getTitle(), $actualTheme->getTitle()); - $this->assertSame($theme1->getVersion(), $actualTheme->getVersion()); - $this->assertSame(true, $actualTheme->isActive()); + $this->assertSame($filteredThemeList, $actualThemes); } }