Feature/add tests for statistic keeper module 406 #569
Conversation
…error responses from PlayerService Error was caused by `PlayerService` returning null instead of error in case the player is not found. Bug is fixed by adding if statement which returns ServiceError NOT_FOUND if the service returns null
|
Check that your husky is working, which should run prettier automatically, before the push, because there was wrong formatting in test file |
MikhailDeriabin
left a comment
MikhailDeriabin
left a comment
There was a problem hiding this comment.
Overall good and thorough tests, some small adjustments required
| return await module.resolve(PlayerStatisticService); | ||
| } | ||
|
|
||
| static async getPlayerService() { |
There was a problem hiding this comment.
Please use PlayerModule testing module to get PlayerService instead of adding a method it here. StatisticsKeeperModule should have only methods for creating own classes
There was a problem hiding this comment.
If I do that than the mocks will not work. I think because my original solution is getting a reference to the PlayerService instance that is a dependency of the PlayerStatisticService. The purposed solution will get a reference to an another PlayerService.
|
|
||
| static async getPlayerService() { | ||
| const module = await StatisticsKeeperCommonModule.getModule(); | ||
| return await module.resolve(PlayerService); |
There was a problem hiding this comment.
You do not have to specify the await here, since it is not necessary
| it('Should increase the players playedBattles if the input is valid | PlayerEvent.BATTLE_PLAYED', async () => { | ||
| const gameStatistics = gameStatisticsBuilder.setWonBattles(0).build(); | ||
|
|
||
| const player = playerBuilder |
There was a problem hiding this comment.
Move the creation of player object into beforeEach(), since you are using this object in almost all the cases. Then remove its creation from all of the test cases
| ); | ||
|
|
||
| expect(result).toBe(false); | ||
| expect(error[0]).toBeInstanceOf(ServiceError); |
There was a problem hiding this comment.
You can also use the .toContainSENOT_FOUND() instead of checking the error type and its reason
I think now it's working
|
…rvice.updatePlayerStatistic() tests
MikhailDeriabin
deleted the
feature/add-tests-for-statisticKeeper-module-406
branch
Brief description
Add tests for statisticKeeper module
Change list