Skip to content

Commit 5670df3

Browse files
committed
refactor(tests): address code review findings
1 parent b844951 commit 5670df3

File tree

4 files changed

+12
-20
lines changed

4 files changed

+12
-20
lines changed

src/main/java/ar/com/nanotaboada/java/samples/spring/boot/repositories/PlayersRepository.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
import java.util.Optional;
55

66
import org.springframework.data.jpa.repository.JpaRepository;
7-
import org.springframework.data.jpa.repository.Query;
8-
import org.springframework.data.repository.query.Param;
97
import org.springframework.stereotype.Repository;
108

119
import ar.com.nanotaboada.java.samples.spring.boot.models.Player;
@@ -65,6 +63,5 @@ public interface PlayersRepository extends JpaRepository<Player, Long> {
6563
* @return a list of players whose league name contains the search term (empty
6664
* list if none found)
6765
*/
68-
@Query("SELECT p FROM Player p WHERE LOWER(p.league) LIKE LOWER(CONCAT('%', :league, '%'))")
69-
List<Player> findByLeagueContainingIgnoreCase(@Param("league") String league);
66+
List<Player> findByLeagueContainingIgnoreCase(String league);
7067
}

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/controllers/PlayersControllerTests.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import ar.com.nanotaboada.java.samples.spring.boot.controllers.PlayersController;
3434
import ar.com.nanotaboada.java.samples.spring.boot.models.PlayerDTO;
35-
import ar.com.nanotaboada.java.samples.spring.boot.repositories.PlayersRepository;
3635
import ar.com.nanotaboada.java.samples.spring.boot.services.PlayersService;
3736
import ar.com.nanotaboada.java.samples.spring.boot.test.PlayerDTOFakes;
3837

@@ -50,9 +49,6 @@ class PlayersControllerTests {
5049
@MockitoBean
5150
private PlayersService playersServiceMock;
5251

53-
@MockitoBean
54-
private PlayersRepository playersRepositoryMock;
55-
5652
@Autowired
5753
private ObjectMapper objectMapper;
5854

@@ -257,7 +253,7 @@ void givenPlayerExists_whenGetBySquadNumber_thenReturnsOk()
257253
// Given
258254
Integer squadNumber = 10;
259255
PlayerDTO expected = PlayerDTOFakes.createAll().stream()
260-
.filter(player -> player.getSquadNumber() == squadNumber)
256+
.filter(player -> squadNumber.equals(player.getSquadNumber()))
261257
.findFirst()
262258
.orElseThrow();
263259
Mockito

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/repositories/PlayersRepositoryTests.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ void givenPlayerDoesNotExist_whenFindById_thenReturnsEmpty() {
5858
}
5959

6060
/**
61-
* Given players exist in a specific league
61+
* Given players exist in a specific league (pre-seeded from dml.sql)
6262
* When searching by league name
6363
* Then a list of matching players is returned
6464
*/
@@ -89,22 +89,20 @@ void givenNoPlayersExist_whenFindByLeague_thenReturnsEmptyList() {
8989
}
9090

9191
/**
92-
* Given a player with a specific squad number exists in the database
92+
* Given a player with a specific squad number exists (pre-seeded from dml.sql)
9393
* When querying by that squad number
9494
* Then the player record is returned
9595
*/
9696
@Test
9797
void givenPlayerExists_whenFindBySquadNumber_thenReturnsPlayer() {
9898
// Given
99-
Player expected = PlayerFakes.createAll().stream()
100-
.filter(p -> p.getId().equals(10L))
101-
.findFirst()
102-
.orElseThrow();
99+
Integer expectedSquadNumber = 10; // Messi's squad number from pre-seeded data
103100
// When
104-
Optional<Player> actual = repository.findBySquadNumber(expected.getSquadNumber());
101+
Optional<Player> actual = repository.findBySquadNumber(expectedSquadNumber);
105102
// Then
106103
then(actual).isPresent();
107-
then(actual.get()).usingRecursiveComparison().isEqualTo(expected);
104+
then(actual.get().getSquadNumber()).isEqualTo(expectedSquadNumber);
105+
then(actual.get().getLastName()).isEqualTo("Messi");
108106
}
109107

110108
/**

src/test/java/ar/com/nanotaboada/java/samples/spring/boot/test/services/PlayersServiceTests.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ void givenPlayerAlreadyExists_whenCreate_thenReturnsNull() {
8989
PlayerDTO dto = PlayerDTOFakes.createOneValid();
9090
Integer expectedSquadNumber = 10;
9191
Player existingPlayer = PlayerFakes.createAll().stream()
92-
.filter(player -> player.getSquadNumber() == expectedSquadNumber)
92+
.filter(player -> expectedSquadNumber.equals(player.getSquadNumber()))
9393
.findFirst()
9494
.orElseThrow();
9595
Mockito
@@ -160,6 +160,7 @@ void givenAllPlayersExist_whenRetrieveAll_thenReturns26Players() {
160160
// Then
161161
verify(playersRepositoryMock, times(1)).findAll();
162162
then(actual).hasSize(26);
163+
then(actual).usingRecursiveComparison().isEqualTo(dtos);
163164
}
164165

165166
/**
@@ -216,11 +217,11 @@ void givenPlayerExists_whenRetrieveBySquadNumber_thenReturnsPlayerDTO() {
216217
// Given
217218
Integer squadNumber = 10;
218219
Player entity = PlayerFakes.createAll().stream()
219-
.filter(player -> player.getSquadNumber() == squadNumber)
220+
.filter(player -> squadNumber.equals(player.getSquadNumber()))
220221
.findFirst()
221222
.orElseThrow();
222223
PlayerDTO expected = PlayerDTOFakes.createAll().stream()
223-
.filter(player -> player.getSquadNumber() == squadNumber)
224+
.filter(player -> squadNumber.equals(player.getSquadNumber()))
224225
.findFirst()
225226
.orElseThrow();
226227
Mockito

0 commit comments

Comments
 (0)