Merge pull request #266 from nanotaboada/test/bdd-given-when-then-pattern

test: adopt BDD Given-When-Then pattern across all tests

Author: nanotaboada

.coderabbit.yaml

@@ -84,7 +84,7 @@ reviews:
         - Test classes should use JUnit 5 (Jupiter)
         - Verify proper Spring test annotations (@WebMvcTest, @DataJpaTest, etc.)
         - Check use of @MockitoBean (Spring Boot 4.0 style)
-        - Ensure test naming follows method_scenario_outcome pattern (concise)
+        - Ensure test naming follows givenX_whenY_thenZ BDD pattern
         - Validate BDD semantics in JavaDoc comments (Given/When/Then)
         - Validate use of AssertJ for fluent assertions
         - Check @DisplayName for readable test descriptions

.github/copilot-instructions.md

@@ -43,7 +43,7 @@ http://localhost:9001/actuator/health       # Health check
 - **Validation**: Bean Validation annotations in DTOs (`@NotNull`, `@Min`, etc.)
 - **Caching**: Spring `@Cacheable` on service layer (1-hour TTL)
 - **DTO Pattern**: ModelMapper for entity ↔ DTO transformations
-- **Repository**: Spring Data JPA with derived queries and custom JPQL
+- **Repository**: Spring Data JPA with derived queries (prefer over custom JPQL)
 
 ## Code Conventions
 
@@ -67,21 +67,21 @@ http://localhost:9001/actuator/health       # Health check
 ## Testing
 
 - **Structure**: `*Tests.java` in `src/test/java/` (mirrors main package structure)
-- **Naming Pattern**: `method_scenario_outcome`
-  - `method`: Method under test (e.g., `post`, `findById`, `create`)
-  - `scenario`: Context (e.g., `playerExists`, `invalidData`, `noMatches`)
-  - `outcome`: Expected result (e.g., `returnsPlayer`, `returnsConflict`, `returnsEmpty`)
+- **Naming Pattern**: `givenX_whenY_thenZ` (BDD Given-When-Then)
+  - `given`: Preconditions/context (e.g., `givenPlayerExists`, `givenInvalidData`, `givenNoMatches`)
+  - `when`: Action being tested (e.g., `whenPost`, `whenFindById`, `whenCreate`)
+  - `then`: Expected outcome (e.g., `thenReturnsPlayer`, `thenReturnsConflict`, `thenReturnsEmpty`)
 - **Examples**:
 
   ```java
   // Controller
-  void post_squadNumberExists_returnsConflict()
+  void givenSquadNumberExists_whenPost_thenReturnsConflict()
 
   // Service
-  void create_noConflict_returnsPlayerDTO()
+  void givenNoConflict_whenCreate_thenReturnsPlayerDTO()
 
   // Repository
-  void findById_playerExists_returnsPlayer()
+  void givenPlayerExists_whenFindById_thenReturnsPlayer()
   ```
 
 - **JavaDoc**: BDD Given/When/Then structure in test comments
@@ -97,7 +97,7 @@ http://localhost:9001/actuator/health       # Health check
   ```
 
 - **Annotations**: `@SpringBootTest`, `@AutoConfigureMockMvc`, `@Test`
-- **Assertions**: AssertJ (fluent, e.g., `assertThat(result).isNotNull()`)
+- **Assertions**: AssertJ BDD style (e.g., `then(result).isNotNull()`)
 - **Mocking**: Mockito for service layer tests
 - **Database**: Tests use in-memory SQLite (auto-cleared after each test)
 - **Coverage**: Target high coverage (JaCoCo reports in `target/site/jacoco/`)

README.md

@@ -243,10 +243,10 @@ open target/site/jacoco/index.html
 - **Test Database** - SQLite in-memory (jdbc:sqlite::memory:) for fast, isolated test execution
 - **Mocking** - Mockito with `@MockitoBean` for dependency mocking
 - **Assertions** - AssertJ fluent assertions
-- **Naming Convention** - `method_scenario_outcome` pattern:
-  - `getAll_playersExist_returnsOkWithAllPlayers()`
-  - `post_squadNumberExists_returnsConflict()`
-  - `findById_playerExists_returnsPlayer()`
+- **Naming Convention** - `givenX_whenY_thenZ` BDD pattern:
+  - `givenPlayersExist_whenGetAll_thenReturnsOkWithAllPlayers()`
+  - `givenSquadNumberExists_whenPost_thenReturnsConflict()`
+  - `givenPlayerExists_whenFindById_thenReturnsPlayer()`
 
 **Coverage Targets:**
 

src/main/java/ar/com/nanotaboada/java/samples/spring/boot/controllers/PlayersController.java

@@ -39,7 +39,7 @@
  * <li><b>GET</b> {@code /players} - Retrieve all players</li>
  * <li><b>GET</b> {@code /players/{id}} - Retrieve player by ID</li>
  * <li><b>GET</b> {@code /players/search/league/{league}} - Search players by league name</li>
- * <li><b>GET</b> {@code /players/search/squadnumber/{number}} - Search player by squad number</li>
+ * <li><b>GET</b> {@code /players/squadnumber/{number}} - Retrieve player by squad number</li>
  * <li><b>POST</b> {@code /players} - Create a new player</li>
  * <li><b>PUT</b> {@code /players/{id}} - Update an existing player</li>
  * <li><b>DELETE</b> {@code /players/{id}} - Delete a player by ID</li>
@@ -113,6 +113,24 @@ public ResponseEntity<Void> post(@RequestBody @Valid PlayerDTO playerDTO) {
      * -----------------------------------------------------------------------------------------------------------------------
      */
 
+    /**
+     * Retrieves all players in the squad.
+     * <p>
+     * Returns the complete Argentina 2022 FIFA World Cup squad (26 players).
+     * </p>
+     *
+     * @return 200 OK with array of all players (empty array if none found)
+     */
+    @GetMapping("/players")
+    @Operation(summary = "Retrieves all players")
+    @ApiResponses(value = {
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(mediaType = "application/json", schema = @Schema(implementation = PlayerDTO[].class)))
+    })
+    public ResponseEntity<List<PlayerDTO>> getAll() {
+        List<PlayerDTO> players = playersService.retrieveAll();
+        return ResponseEntity.status(HttpStatus.OK).body(players);
+    }
+
     /**
      * Retrieves a single player by their unique identifier.
      *
@@ -133,21 +151,26 @@ public ResponseEntity<PlayerDTO> getById(@PathVariable Long id) {
     }
 
     /**
-     * Retrieves all players in the squad.
+     * Retrieves a player by their squad number (unique identifier).
      * <p>
-     * Returns the complete Argentina 2022 FIFA World Cup squad (26 players).
+     * Squad numbers are unique jersey numbers (e.g., Messi is #10). This is a direct lookup similar to getById().
+     * Example: {@code /players/squadnumber/10} returns Lionel Messi
      * </p>
      *
-     * @return 200 OK with array of all players (empty array if none found)
+     * @param squadNumber the squad number to retrieve (jersey number, typically 1-99)
+     * @return 200 OK with player data, or 404 Not Found if no player has that number
      */
-    @GetMapping("/players")
-    @Operation(summary = "Retrieves all players")
+    @GetMapping("/players/squadnumber/{squadNumber}")
+    @Operation(summary = "Retrieves a player by squad number")
     @ApiResponses(value = {
-            @ApiResponse(responseCode = "200", description = "OK", content = @Content(mediaType = "application/json", schema = @Schema(implementation = PlayerDTO[].class)))
+            @ApiResponse(responseCode = "200", description = "OK", content = @Content(mediaType = "application/json", schema = @Schema(implementation = PlayerDTO.class))),
+            @ApiResponse(responseCode = "404", description = "Not Found", content = @Content)
     })
-    public ResponseEntity<List<PlayerDTO>> getAll() {
-        List<PlayerDTO> players = playersService.retrieveAll();
-        return ResponseEntity.status(HttpStatus.OK).body(players);
+    public ResponseEntity<PlayerDTO> getBySquadNumber(@PathVariable Integer squadNumber) {
+        PlayerDTO player = playersService.retrieveBySquadNumber(squadNumber);
+        return (player != null)
+                ? ResponseEntity.status(HttpStatus.OK).body(player)
+                : ResponseEntity.status(HttpStatus.NOT_FOUND).build();
     }
 
     /**
@@ -169,30 +192,6 @@ public ResponseEntity<List<PlayerDTO>> searchByLeague(@PathVariable String leagu
         return ResponseEntity.status(HttpStatus.OK).body(players);
     }
 
-    /**
-     * Searches for a player by their squad number.
-     * <p>
-     * Squad numbers are jersey numbers that users recognize (e.g., Messi is #10).
-     * Example: {@code /players/search/squadnumber/10} returns Lionel Messi
-     * </p>
-     *
-     * @param squadNumber the squad number to search for (jersey number, typically 1-99)
-     * @return 200 OK with player data, or 404 Not Found if no player has that
-     * number
-     */
-    @GetMapping("/players/search/squadnumber/{squadNumber}")
-    @Operation(summary = "Searches for a player by squad number")
-    @ApiResponses(value = {
-            @ApiResponse(responseCode = "200", description = "OK", content = @Content(mediaType = "application/json", schema = @Schema(implementation = PlayerDTO.class))),
-            @ApiResponse(responseCode = "404", description = "Not Found", content = @Content)
-    })
-    public ResponseEntity<PlayerDTO> searchBySquadNumber(@PathVariable Integer squadNumber) {
-        PlayerDTO player = playersService.searchBySquadNumber(squadNumber);
-        return (player != null)
-                ? ResponseEntity.status(HttpStatus.OK).body(player)
-                : ResponseEntity.status(HttpStatus.NOT_FOUND).build();
-    }
-
     /*
      * -----------------------------------------------------------------------------------------------------------------------
      * HTTP PUT

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

@@ -4,8 +4,6 @@
 import java.util.Optional;
 
 import org.springframework.data.jpa.repository.JpaRepository;
-import org.springframework.data.jpa.repository.Query;
-import org.springframework.data.repository.query.Param;
 import org.springframework.stereotype.Repository;
 
 import ar.com.nanotaboada.java.samples.spring.boot.models.Player;
@@ -26,8 +24,8 @@
  *
  * <h3>Query Strategies:</h3>
  * <ul>
- * <li><b>@Query:</b> Explicit JPQL for complex searches (findByLeagueContainingIgnoreCase)</li>
- * <li><b>Method Names:</b> Spring Data derives queries from method names (Query Creation)</li>
+ * <li><b>Derived Queries:</b> Spring Data derives queries from method names (findBySquadNumber,
+ * findByLeagueContainingIgnoreCase)</li>
  * </ul>
  *
  * @see Player
@@ -40,20 +38,6 @@
 @Repository
 public interface PlayersRepository extends JpaRepository<Player, Long> {
 
-    /**
-     * Finds players by league name using case-insensitive wildcard matching.
-     * <p>
-     * This method uses a custom JPQL query with LIKE operator for partial matches.
-     * For example, searching for "Premier" will match "Premier League".
-     * </p>
-     *
-     * @param league the league name to search for (partial matches allowed)
-     * @return a list of players whose league name contains the search term (empty
-     * list if none found)
-     */
-    @Query("SELECT p FROM Player p WHERE LOWER(p.league) LIKE LOWER(CONCAT('%', :league, '%'))")
-    List<Player> findByLeagueContainingIgnoreCase(@Param("league") String league);
-
     /**
      * Finds a player by their squad number (exact match).
      * <p>
@@ -67,4 +51,17 @@ public interface PlayersRepository extends JpaRepository<Player, Long> {
      * @return an Optional containing the player if found, empty Optional otherwise
      */
     Optional<Player> findBySquadNumber(Integer squadNumber);
+
+    /**
+     * Finds players by league name using case-insensitive wildcard matching.
+     * <p>
+     * This method uses Spring Data's derived query mechanism to perform partial matching.
+     * For example, searching for "Premier" will match "Premier League".
+     * </p>
+     *
+     * @param league the league name to search for (partial matches allowed)
+     * @return a list of players whose league name contains the search term (empty
+     * list if none found)
+     */
+    List<Player> findByLeagueContainingIgnoreCase(String league);
 }

src/main/java/ar/com/nanotaboada/java/samples/spring/boot/services/PlayersService.java

@@ -110,6 +110,24 @@ public PlayerDTO create(PlayerDTO playerDTO) {
      * -----------------------------------------------------------------------------------------------------------------------
      */
 
+    /**
+     * Retrieves all players from the database.
+     * <p>
+     * This method returns the complete Argentina 2022 FIFA World Cup squad (26 players).
+     * Results are cached to improve performance on subsequent calls.
+     * </p>
+     *
+     * @return a list of all players (empty list if none found)
+     * @see org.springframework.cache.annotation.Cacheable
+     */
+    @Cacheable(value = "players")
+    public List<PlayerDTO> retrieveAll() {
+        return playersRepository.findAll()
+                .stream()
+                .map(this::mapFrom)
+                .toList();
+    }
+
     /**
      * Retrieves a player by their unique identifier.
      * <p>
@@ -130,21 +148,21 @@ public PlayerDTO retrieveById(Long id) {
     }
 
     /**
-     * Retrieves all players from the database.
+     * Retrieves a player by their squad number (unique identifier).
      * <p>
-     * This method returns the complete Argentina 2022 FIFA World Cup squad (26 players).
-     * Results are cached to improve performance on subsequent calls.
+     * Squad numbers are unique jersey numbers (e.g., Messi is #10). This is a direct lookup by unique identifier,
+     * similar to retrieveById(). Results are cached to improve performance.
      * </p>
      *
-     * @return a list of all players (empty list if none found)
+     * @param squadNumber the squad number to retrieve (jersey number, typically 1-99)
+     * @return the player DTO if found, null otherwise
      * @see org.springframework.cache.annotation.Cacheable
      */
-    @Cacheable(value = "players")
-    public List<PlayerDTO> retrieveAll() {
-        return playersRepository.findAll()
-                .stream()
+    @Cacheable(value = "players", key = "'squad-' + #squadNumber", unless = "#result == null")
+    public PlayerDTO retrieveBySquadNumber(Integer squadNumber) {
+        return playersRepository.findBySquadNumber(squadNumber)
                 .map(this::mapFrom)
-                .toList();
+                .orElse(null);
     }
 
     /*
@@ -170,22 +188,6 @@ public List<PlayerDTO> searchByLeague(String league) {
                 .toList();
     }
 
-    /**
-     * Searches for a player by their squad number.
-     * <p>
-     * This method performs an exact match on the squad number field. Squad numbers are jersey numbers that users recognize
-     * (e.g., Messi is #10).
-     * </p>
-     *
-     * @param squadNumber the squad number to search for (jersey number, typically 1-99)
-     * @return the player DTO if found, null otherwise
-     */
-    public PlayerDTO searchBySquadNumber(Integer squadNumber) {
-        return playersRepository.findBySquadNumber(squadNumber)
-                .map(this::mapFrom)
-                .orElse(null);
-    }
-
     /*
      * -----------------------------------------------------------------------------------------------------------------------
      * Update