Conversation
WalkthroughThis change replaces the previous task-based card game round management system with a new, modular, room-specific round management architecture. Old classes and enums related to task execution and game state are removed, and new entities for round and phase handling are introduced. Configuration is now externalized, and the public API is refactored for clarity and flexibility. Test code and monitoring endpoints are updated to align with the new structure. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant CardGameService
participant RoundManagerRegistry
participant RoomRoundManager
participant CardGame
participant Room
Client->>CardGameService: start(playable, joinCode)
CardGameService->>RoundManagerRegistry: getOrCreate(joinCode)
RoundManagerRegistry->>RoomRoundManager: (create if not exists)
CardGameService->>CardGame: startGame(players)
CardGameService->>RoomRoundManager: executePhase(cardGame, room, onStateChange)
RoomRoundManager->>CardGame: (phase logic)
RoomRoundManager->>Room: (update state)
RoomRoundManager->>CardGameService: (callback on state change)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| log.info("방 {} - 카드게임 시작", joinCode); | ||
|
|
||
| // 해당 방의 전용 RoundManager 가져오기 (없으면 새로 생성) | ||
| RoomRoundManager roomRoundManager = roundManagerRegistry.getOrCreate(roomJoinCode); |
There was a problem hiding this comment.
현재는 가져오거나 새로 생성이지만, 미니게임이 변경될 때마다 새로운 게임 방식을 주입해서 처리할 수 있을 것 같습니다.
roundManagerRegistry에 GameType에 따라서 RoomRoundManager를 다르게 주입할 수 있을 것 같습니다.
ex)
RoomRoundManager roomRoundManager = roundManagerRegistry.put(roomJoinCode, gameType);
| private RoundState roundState; | ||
| private final int maxRounds; |
There was a problem hiding this comment.
roundState에 maxRounds를 필드로 가져서 처리해도 괜찮을 거 같다고 보는 데 어떻게 생각하시나요?
- CardGame 패키지에서 라운드를 next로 넘어갈 수 있도록 변경 - ROundState에서 maxRound를 필드로 주입하여 설정
| this.round = CardGameRound.READY; | ||
| this.state = CardGameState.READY; | ||
| this.deck = deckGenerator.generate(ADDITION_CARD_COUNT, MULTIPLIER_CARD_COUNT); | ||
| this.roundState = new RoundState(1, RoundPhase.READY, DEFAULT_MAX_ROUNDS); |
There was a problem hiding this comment.
roundState를 1로 주입하고 있지만, 기본적으로 시작하는 라운드는 1이므로, 초반 RoundPhase 상태와 최대 라운드만 주입해서 처리해도 좋지 않을까 생각합니다~
| public void moveToNextPhase() { | ||
| this.roundState = roundState.nextPhase(); |
There was a problem hiding this comment.
매번 상태를 set으로 변경할 필요없이 다음 상태로 전환한다면, CardGame의 상태도 변경될 수 있는 로직을 추가했습니다. 따라서 changeDoneState, changePlayingState와 같은 형태가 필요없어집니다.
| private final Duration duration; | ||
|
|
||
| public LoadingPhaseHandler(@Value("${card-game.phases.loading.duration:PT3S}") Duration duration) { | ||
| this.duration = duration; |
There was a problem hiding this comment.
시간을 외부 파일로 수집해서 테스트 환경에서 다르게 처리할 수 있을 것 같습니다~
There was a problem hiding this comment.
현재 RoundState는 CardGame에 맞추어서 관리되고 있으나, 이를 상속이나 추상화를 통해서 각 게임 타입에 맞게 RoundState를 관리할 수 있지 않을까 생각합니다~
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull Request Overview
This PR completely refactors the card game system by replacing the complex ChainedTask system with a cleaner, room-specific round management system. The new architecture uses handlers for each game phase and provides better configuration management.
- Removes the ChainedTask-based system and replaces it with RoundPhase handlers
- Implements room-specific RoundManager instances for better isolation
- Adds external configuration support for game timing and rules
Reviewed Changes
Copilot reviewed 34 out of 34 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/src/test/java/coffeeshout/minigame/ui/CardGameIntegrationTest.java | Comments out all integration tests temporarily |
| backend/src/test/java/coffeeshout/minigame/domain/cardgame/round/RoundStateTest.java | Adds comprehensive tests for the new RoundState class |
| backend/src/test/java/coffeeshout/minigame/domain/cardgame/PlayerHandsTest.java | Comments out all PlayerHands tests temporarily |
| backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardHandTest.java | Comments out tests with syntax errors in line 65 |
| backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameTest.java | Comments out all CardGame domain tests temporarily |
| backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameRoundTest.java | Comments out legacy CardGameRound enum tests |
| backend/src/test/java/coffeeshout/minigame/application/CardGameTestHelper.java | Adds utility class for test setup and round state management |
| backend/src/test/java/coffeeshout/minigame/application/CardGameServiceTest.java | Refactors service tests to use new RoundManager system |
| backend/src/test/java/coffeeshout/minigame/application/CardGameServiceRealTimeTest.java | Updates real-time tests to use RoundPhase instead of CardGameState |
| backend/src/main/resources/application.yml | Adds card-game configuration with phase durations |
| backend/src/main/java/coffeeshout/room/application/RoomService.java | Integrates CardGameService cleanup in room cleanup process |
| backend/src/main/java/coffeeshout/minigame/ui/response/MiniGameStateMessage.java | Updates to use new CardGame API methods |
| backend/src/main/java/coffeeshout/minigame/ui/RoundManagerMonitorController.java | Adds monitoring endpoints for active room count |
| backend/src/main/java/coffeeshout/minigame/domain/task/MiniGameTaskManager.java | Removes legacy task management system |
| backend/src/main/java/coffeeshout/minigame/domain/task/ChainedTask.java | Removes legacy chained task implementation |
| backend/src/main/java/coffeeshout/minigame/domain/task/CardGameTaskType.java | Removes legacy task type definitions |
| backend/src/main/java/coffeeshout/minigame/domain/round/handler/*.java | Implements phase-specific handlers (Loading, Playing, Scoring, Done, Ready) |
| backend/src/main/java/coffeeshout/minigame/domain/round/RoundState.java | Core round state management with phase transitions |
| backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhaseHandler.java | Interface for phase-specific logic handlers |
| backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhase.java | Enum defining game phases (READY, LOADING, PLAYING, SCORING, DONE) |
| backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java | Registry for managing room-specific RoundManager instances |
| backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java | Room-specific round manager with phase scheduling |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/PlayerHands.java | Updates player hand management for new round system |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardHand.java | Updates card hand logic for round-based operations |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameTaskExecutorsV2.java | Removes legacy task executor system |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameState.java | Removes legacy state enum |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameRound.java | Removes legacy round enum |
| backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGame.java | Major refactor to use new RoundState system |
| backend/src/main/java/coffeeshout/minigame/application/CardGameService.java | Complete rewrite to use RoundManager system |
| backend/MIGRATION_GUIDE.md | Comprehensive migration guide documenting all changes |
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.(CardGameRound.FIRST)).isTrue(); |
There was a problem hiding this comment.
Syntax error in commented code. There's a missing method name between 'cardHand.' and '(CardGameRound.FIRST)'. This should be fixed to 'cardHand.isSelected(CardGameRound.FIRST)' or updated to use the new API.
| // softly.assertThat(cardHand.(CardGameRound.FIRST)).isTrue(); | |
| // softly.assertThat(cardHand.isSelected(CardGameRound.FIRST)).isTrue(); |
| /** | ||
| * 특정 라운드에서 선택하지 않은 플레이어들 반환 | ||
| */ | ||
| public List<Player> getUnselectedPlayers(int roundNumber) { |
There was a problem hiding this comment.
The method 'hasCardForRound(roundNumber)' uses 1-based round numbering, but this should be clearly documented or validated to prevent off-by-one errors. Consider adding validation or clear documentation about the expected range.
| * 특정 라운드에서 카드를 선택했는지 확인 (1-based round number) | ||
| */ | ||
| public boolean hasCardForRound(int roundNumber) { | ||
| return roundNumber <= hand.size(); |
There was a problem hiding this comment.
The comment indicates '1-based round number' but there's no validation to ensure roundNumber is >= 1. Consider adding validation or documenting the expected behavior for invalid inputs.
| return roundNumber <= hand.size(); | |
| return roundNumber >= 1 && roundNumber <= hand.size(); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
♻️ Duplicate comments (3)
backend/src/main/java/coffeeshout/minigame/domain/round/handler/LoadingPhaseHandler.java (1)
21-23: Configuration is already externalized for testing flexibility.The duration is configurable via @value with a sensible default, which addresses the flexibility needed for different test environments as mentioned in the past review comment.
backend/src/main/java/coffeeshout/minigame/domain/round/RoundState.java (1)
1-6: Consider abstracting RoundState for different game typesAs mentioned in the previous review, the current
RoundStateis tightly coupled to CardGame. Consider creating an abstract base class or interface to support different game types in the future, aligning with the Open/Closed Principle.backend/src/main/java/coffeeshout/minigame/application/CardGameService.java (1)
53-53: Consider supporting different game types in registryAs suggested in the previous review, the registry could support different game types for better extensibility:
RoomRoundManager roomRoundManager = roundManagerRegistry.getOrCreate(roomJoinCode, gameType);This would align with the Open/Closed Principle and support future mini-game additions.
🧹 Nitpick comments (12)
backend/src/test/java/coffeeshout/minigame/application/CardGameTestHelper.java (1)
15-23: Add null checking for robustness.Consider adding null checking to prevent NPE in test scenarios.
public static void setPlayingState(CardGame cardGame) { + if (cardGame == null) { + throw new IllegalArgumentException("CardGame cannot be null"); + } + // READY → LOADING → PLAYING 순서로 변경 if (cardGame.getCurrentPhase() == RoundPhase.READY) { cardGame.moveToNextPhase(); // LOADING } if (cardGame.getCurrentPhase() == RoundPhase.LOADING) { cardGame.moveToNextPhase(); // PLAYING } }backend/src/main/java/coffeeshout/minigame/ui/RoundManagerMonitorController.java (2)
43-43: Externalize version information.The hardcoded version "v2.0" should be externalized to configuration to avoid manual updates with each release.
+@Value("${app.version:unknown}") +private final String version; - "version", "v2.0", + "version", version,
36-46: Consider enhancing health check logic.The status endpoint currently returns a hardcoded "healthy" status. For better operational monitoring, consider adding actual health validation logic.
@GetMapping("/status") public Map<String, Object> getStatus() { int activeRoomCount = cardGameService.getActiveRoomCount(); + + // Add basic health checks + String status = "healthy"; + try { + // Validate that the service is responsive + cardGameService.getActiveRoomCount(); + } catch (Exception e) { + status = "unhealthy"; + } return Map.of( - "status", "healthy", + "status", status, "activeRoomCount", activeRoomCount, "version", version, "timestamp", System.currentTimeMillis() ); }backend/src/main/java/coffeeshout/minigame/domain/round/RoundState.java (2)
20-29: Remove unreachable default case in switch expressionThe switch expression covers all
RoundPhaseenum values. The default case at line 27 is unreachable and can be removed. If you want to guard against future enum additions, consider making it explicit.private RoundPhase getNextPhase() { return switch (phase) { case READY -> RoundPhase.LOADING; case LOADING -> RoundPhase.PLAYING; case PLAYING -> RoundPhase.SCORING; case SCORING -> getRoundPhase(); case DONE -> RoundPhase.DONE; // DONE 상태는 계속 DONE 유지 - default -> throw new IllegalStateException("다음 단계를 결정할 수 없습니다: " + phase); }; }
31-37: Rename method for clarityThe method name
getRoundPhase()is misleading as it determines whether to end the game or continue to the next round. Consider a more descriptive name.-private RoundPhase getRoundPhase() { +private RoundPhase getPhaseAfterScoring() { // 현재 라운드가 최대 라운드와 같다면 게임 종료 if (roundNumber >= maxRounds) { return RoundPhase.DONE; } return RoundPhase.LOADING; }Also update the reference in
getNextPhase():- case SCORING -> getRoundPhase(); + case SCORING -> getPhaseAfterScoring();backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java (1)
118-129: Consider stack overflow risk with recursive executionThe method calls
executePhaserecursively, which could potentially cause stack overflow in games with many rounds/phases. While unlikely in normal scenarios, it's worth considering an iterative approach for robustness.Consider using a queue-based approach or limiting recursion depth for very long games. For typical card games with reasonable round limits, this should be fine.
backend/src/test/java/coffeeshout/minigame/application/CardGameServiceTest.java (4)
83-83: Remove unnecessary empty linesThese empty lines disrupt the flow of the test method.
- Playable currentGame = room.startNextGame(host.getName().value()); - - // when + Playable currentGame = room.startNextGame(host.getName().value()); + // when- softly.assertThat(cardGame.getRoundState().phase()).isNotEqualTo(RoundPhase.READY); - - // RoundManager가 등록되었는지 확인 + softly.assertThat(cardGame.getRoundState().phase()).isNotEqualTo(RoundPhase.READY); + // RoundManager가 등록되었는지 확인Also applies to: 95-95
146-146: Remove unnecessary empty lineThis empty line disrupts the test flow.
- Playable miniGame = room.startNextGame(host.getName().value()); - - // when + Playable miniGame = room.startNextGame(host.getName().value()); + // when
248-251: Add clarifying comment for phase transitionsThe two
moveToNextPhase()calls correctly move to PLAYING state, but this should be documented for clarity.- // PLAYING 상태로 변경 - cardGame.moveToNextPhase(); - cardGame.moveToNextPhase(); - + // PLAYING 상태로 변경 + cardGame.moveToNextPhase(); // READY -> LOADING + cardGame.moveToNextPhase(); // LOADING -> PLAYING
363-363: Test assumption about configuration value could be fragileThe test hardcodes the expected value of 2 rounds, which could break if the configuration changes. Consider making this more robust.
Consider injecting the configuration value or using a test-specific configuration to make the test more maintainable:
@Value("${card-game.max-rounds:2}") private int configuredMaxRounds; // then in test assertThat(cardGame.getMaxRounds()).isEqualTo(configuredMaxRounds);backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGame.java (1)
108-111: Empty placeholder method for round score calculationThe
calculateRoundScore()method is currently empty. Consider either implementing the logic or adding a TODO comment to track this for future implementation.public void calculateRoundScore() { - // 현재는 실시간으로 점수가 계산되므로 특별한 로직 없음 - // 필요시 라운드별 점수 계산 로직 추가 가능 + // TODO: Implement round-based score calculation if needed + // Currently scores are calculated in real-time via getScores() }backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhaseHandler.java (1)
1-27: Excellent interface design implementing the state patternThis new
RoundPhaseHandlerinterface is a well-designed foundation for the state pattern refactoring mentioned in the PR objectives. The interface effectively addresses multiple stated goals:
- Eliminates repetitive code: Each phase handler encapsulates its specific logic in
execute()- Enables flexible timing:
getDuration()supports configurable phase durations- Supports externalized configuration: Phase behavior can be configured per implementation
- Type-safe phase management:
getPhase()provides clear phase identificationThe method signatures are clean and provide appropriate context (
CardGame,Room) for phase-specific operations. The Javadoc documentation is clear and helpful.Consider adding a default method for common phase transition logic if applicable:
/** * 다음 단계로의 전환을 처리합니다. 기본 구현은 아무 작업도 수행하지 않습니다. */ default void onPhaseExit(CardGame game, Room room) { // Default: no action }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
backend/MIGRATION_GUIDE.md(1 hunks)backend/src/main/java/coffeeshout/minigame/application/CardGameService.java(5 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGame.java(3 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameRound.java(0 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameState.java(0 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameTaskExecutorsV2.java(0 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardHand.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/cardgame/PlayerHands.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhase.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/RoundState.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/handler/DonePhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/handler/LoadingPhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/handler/PlayingPhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/handler/ReadyPhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/round/handler/ScoringPhaseHandler.java(1 hunks)backend/src/main/java/coffeeshout/minigame/domain/task/CardGameTaskType.java(0 hunks)backend/src/main/java/coffeeshout/minigame/domain/task/ChainedTask.java(0 hunks)backend/src/main/java/coffeeshout/minigame/domain/task/MiniGameTaskManager.java(0 hunks)backend/src/main/java/coffeeshout/minigame/ui/RoundManagerMonitorController.java(1 hunks)backend/src/main/java/coffeeshout/minigame/ui/response/MiniGameStateMessage.java(1 hunks)backend/src/main/java/coffeeshout/room/application/RoomService.java(4 hunks)backend/src/main/resources/application.yml(1 hunks)backend/src/test/java/coffeeshout/minigame/application/CardGameServiceRealTimeTest.java(3 hunks)backend/src/test/java/coffeeshout/minigame/application/CardGameServiceTest.java(5 hunks)backend/src/test/java/coffeeshout/minigame/application/CardGameTestHelper.java(1 hunks)backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameRoundTest.java(1 hunks)backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameTest.java(1 hunks)backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardHandTest.java(1 hunks)backend/src/test/java/coffeeshout/minigame/domain/cardgame/PlayerHandsTest.java(1 hunks)backend/src/test/java/coffeeshout/minigame/domain/cardgame/round/RoundStateTest.java(1 hunks)backend/src/test/java/coffeeshout/minigame/ui/CardGameIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (6)
- backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameTaskExecutorsV2.java
- backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameRound.java
- backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGameState.java
- backend/src/main/java/coffeeshout/minigame/domain/task/MiniGameTaskManager.java
- backend/src/main/java/coffeeshout/minigame/domain/task/ChainedTask.java
- backend/src/main/java/coffeeshout/minigame/domain/task/CardGameTaskType.java
🧰 Additional context used
🧬 Code Graph Analysis (2)
backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java (8)
backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGame.java (1)
Getter(21-147)backend/src/main/java/coffeeshout/minigame/domain/round/handler/DonePhaseHandler.java (1)
Slf4j(15-38)backend/src/main/java/coffeeshout/minigame/domain/round/handler/ScoringPhaseHandler.java (1)
Slf4j(15-43)backend/src/main/java/coffeeshout/minigame/domain/round/handler/PlayingPhaseHandler.java (1)
Slf4j(15-41)backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java (1)
Slf4j(16-87)backend/src/main/java/coffeeshout/minigame/domain/round/handler/LoadingPhaseHandler.java (1)
Slf4j(15-39)backend/src/main/java/coffeeshout/minigame/domain/round/handler/ReadyPhaseHandler.java (1)
Slf4j(11-28)backend/src/main/java/coffeeshout/minigame/application/CardGameService.java (1)
Slf4j(21-103)
backend/src/test/java/coffeeshout/minigame/application/CardGameServiceTest.java (1)
backend/src/test/java/coffeeshout/minigame/application/CardGameServiceRealTimeTest.java (1)
Nested(71-149)
🔇 Additional comments (29)
backend/src/main/resources/application.yml (1)
10-19: Excellent externalization of game configuration!This configuration successfully addresses the PR objective of externalizing card game states and rules to YAML. The structure is clean with:
- Configurable
max-roundssolving the inflexibility issue mentioned in #389- Phase durations using standard ISO-8601 format
- Clear Korean comments for maintainability
This will enable dynamic adjustment of game timing and rounds without code changes.
backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhase.java (1)
1-13: Well-designed enum for the new state pattern!The
RoundPhaseenum effectively consolidates phase representation with:
- Clear phase names following logical game progression
- Comprehensive documentation explaining the flow (LOADING → PLAYING → SCORING)
- Clean replacement of the previous
CardGameStateandCardGameRoundenumsThis supports the state pattern refactoring objective from #389.
backend/src/main/java/coffeeshout/minigame/ui/response/MiniGameStateMessage.java (2)
44-51: LGTM! Correctly updated to use new round management API.The changes properly migrate from the deprecated
CardGameState/CardGameRoundenums to the newRoundPhaseenum and numeric round tracking system.The API calls now use:
getCurrentPhase().name()instead ofgetState().name()String.valueOf(getCurrentRoundNumber())instead ofgetRound().name()This aligns perfectly with the state pattern refactoring objectives.
46-47: Well-aligned API migration to new round management systemThe changes successfully migrate from the deprecated
CardGameState/CardGameRoundenums to the newRoundPhaseenum and numeric round tracking. This addresses the PR objectives by:
- Replacing the inflexible EnumType round system with numeric round numbers
- Using the new phase-based state management via
getCurrentPhase()- Providing more flexibility for dynamic round configuration
The API changes indicate a cleaner domain model where phases and rounds are managed separately and more flexibly.
backend/src/main/java/coffeeshout/minigame/domain/round/RoundPhaseHandler.java (1)
1-27: Excellent interface design for the state pattern implementation.This interface effectively implements the Strategy pattern for handling different round phases, addressing the PR objectives of refactoring to use a state pattern.
The design demonstrates good software engineering principles:
- Single Responsibility: Each handler manages one specific phase
- Clear Contract: Well-defined methods with appropriate signatures
- Flexible:
getDuration()allows dynamic timing configuration- Testable: Clean interface that's easy to mock and test
- Well-documented: Clear JavaDoc explains each method's purpose
This interface will enable the modular, configurable round management system described in the PR objectives.
backend/src/test/java/coffeeshout/minigame/application/CardGameServiceRealTimeTest.java (6)
13-13: LGTM! Correctly updated import for new round management system.
64-67: Good addition to ensure proper test setup.Setting all players to ready state is likely required for the new round management system to function correctly during tests.
102-141: Excellent migration to new RoundPhase API.The test assertions have been properly updated to use the new round management system:
cardGame.getCurrentPhase()instead ofcardGame.getState()RoundPhaseenum values instead ofCardGameState- Phase naming updated (e.g.,
SCORE_BOARD→SCORING)The test maintains its coverage of real-time phase transitions while correctly adapting to the refactored state pattern implementation.
13-13: Successfully migrated imports to new round management systemGood replacement of the deprecated
CardGameStateimport with the newRoundPhaseenum, aligning with the state pattern refactoring.
64-67: Important setup addition for new round managementThe explicit player ready state setup is likely required for the new round management system to function properly. This ensures all players are ready before game phases begin.
102-141: Excellent migration to new phase-based assertionsThe test has been successfully updated to use the new round management API:
cardGame.getCurrentPhase()replaces the deprecatedcardGame.getState()RoundPhaseenum values (LOADING,PLAYING,SCORING,DONE) replaceCardGameState- The phase progression flow is logical and well-documented with timing comments
- Real-time testing approach is maintained while working with the new system
This demonstrates that the new state pattern implementation works correctly for the complete game lifecycle.
backend/src/main/java/coffeeshout/minigame/domain/round/handler/ReadyPhaseHandler.java (1)
13-28: LGTM! Clean implementation of the state pattern.The ReadyPhaseHandler properly implements the RoundPhaseHandler interface and fits well into the new modular round management architecture. The zero duration is appropriate for a ready phase that should transition immediately.
backend/src/test/java/coffeeshout/minigame/application/CardGameTestHelper.java (1)
9-42: Well-designed test utility that supports the new architecture.This helper class effectively supports testing of the new phase-based round management system. The methods are well-documented and provide useful abstractions for manipulating game state in tests.
backend/src/main/java/coffeeshout/minigame/domain/round/handler/DonePhaseHandler.java (2)
20-26: LGTM! Proper completion phase handling.The execute method correctly handles the game completion by applying the final result to the room. The logic flow is clear and appropriate for the DONE phase.
17-38: Excellent implementation of the completion phase handler.The DonePhaseHandler properly implements the state pattern and encapsulates the game completion logic. The class structure is clean with good documentation and appropriate annotations.
backend/src/main/java/coffeeshout/minigame/domain/round/handler/PlayingPhaseHandler.java (3)
21-24: Good use of externalized configuration.The configurable duration via Spring properties addresses the PR objective of making the system more flexible and configurable. The default value of 10 seconds (PT10S) is reasonable.
27-30: Clean and focused phase execution.The execute method appropriately handles the playing phase by starting play and providing clear logging. The implementation aligns well with the single responsibility principle.
17-41: Excellent state pattern implementation with configurable behavior.This phase handler exemplifies the benefits of the refactoring - it's modular, configurable, and follows single responsibility principle. The externalized duration configuration addresses the flexibility concerns mentioned in the PR objectives.
backend/src/main/java/coffeeshout/minigame/ui/RoundManagerMonitorController.java (1)
16-47: Good addition for operational monitoring.This controller provides valuable operational visibility into the new round management system. The REST API design is clean and the endpoints serve their intended purpose well.
backend/src/test/java/coffeeshout/minigame/domain/cardgame/round/RoundStateTest.java (1)
9-137: Comprehensive test coverage for RoundState transitions.The test suite thoroughly covers all phase transitions, edge cases, and helper methods. The Korean naming convention is consistent with the project style, and the test logic correctly validates the state machine behavior.
backend/src/main/java/coffeeshout/room/application/RoomService.java (4)
3-3: LGTM: CardGameService integration.Adding the CardGameService dependency properly integrates the new round management system with room lifecycle management.
23-26: LGTM: Logging setup.The @slf4j annotation and Service annotation are properly positioned for clean logging integration.
35-35: LGTM: CardGameService dependency injection.The CardGameService is properly injected via constructor for cleanup coordination.
121-133: Well-designed cleanup coordination.The approach of immediate game resource cleanup followed by delayed room data cleanup is sound. This ensures game-related resources (round managers, schedulers) are promptly released while allowing room data to persist for potential recovery scenarios.
backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardHand.java (1)
35-53: Well-implemented migration from enum to integer-based rounds.The new methods properly replace the enum-based approach with integer round numbers. The 1-based indexing aligns with domain concepts, and the validation in
getCardForRoundcorrectly handles edge cases with clear error messages.backend/src/main/java/coffeeshout/minigame/domain/round/handler/LoadingPhaseHandler.java (1)
17-39: Clean phase handler implementation.The LoadingPhaseHandler follows the expected pattern with configurable duration, appropriate logging, and minimal execution logic suitable for a loading phase.
backend/src/main/java/coffeeshout/minigame/domain/round/handler/ScoringPhaseHandler.java (1)
17-43: Appropriate scoring phase logic implementation.The handler correctly implements scoring phase behavior by assigning random cards to unselected players before scoring calculation. The configurable duration and logging follow the established pattern for phase handlers.
backend/src/main/java/coffeeshout/minigame/domain/cardgame/PlayerHands.java (1)
59-92: Well-implemented round management methodsThe refactored methods properly support the new integer-based round management system, successfully addressing the inflexibility issue with the previous EnumType approach mentioned in the PR objectives.
backend/MIGRATION_GUIDE.md (1)
1-154: Excellent migration guide documentation!This migration guide comprehensively documents the refactoring from the task-based system to the new state pattern-based round management. It clearly addresses all the issues mentioned in #389:
- ✅ Removes inflexible EnumType for round count
- ✅ Eliminates task duplication in CardGameTaskType
- ✅ Replaces inefficient MiniGameTaskManager
- ✅ Externalizes configuration to YAML
- ✅ Improves type safety and testability
The before/after examples and troubleshooting section will be very helpful for developers working with this code.
| public void cleanupRoom(String joinCode) { | ||
| JoinCode roomJoinCode = new JoinCode(joinCode); | ||
| log.info("방 {} 정리 요청", joinCode); | ||
| roundManagerRegistry.remove(roomJoinCode); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify complete cleanup of game resources
The cleanup method removes the round manager but should ensure all associated resources are properly released.
🏁 Script executed:
#!/bin/bash
# Check if there are other resources that need cleanup when a room ends
rg -A 10 "class CardGame" backend/src/main/java/coffeeshout/minigame/domain/cardgame/CardGame.java | head -20
# Check if there are any event listeners or observers that need to be unregistered
rg "addEventListener|registerListener|subscribe" backend/src/main/java/coffeeshout/minigame/Length of output: 546
🏁 Script executed:
#!/bin/bash
# Locate the RoundManagerRegistry implementation and its remove method
rg -n "class RoundManagerRegistry" -A 20 backend/src/main/java/coffeeshout/minigame/
rg -n "remove(" -A 20 backend/src/main/java/coffeeshout/minigame/
# Locate the RoundManager class to check for any internal cleanup or shutdown hooks
rg -n "class RoundManager" -A 20 backend/src/main/java/coffeeshout/minigame/Length of output: 7699
#!/bin/bash
# Search for the remove method in RoundManagerRegistry
rg -F -n "remove(" backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java
# Dump the full RoundManagerRegistry class to inspect cleanup logic
sed -n '1,200p' backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java
# Locate and show RoomRoundManager to check for any shutdown or cancellation hooks
rg -n "class RoomRoundManager" -A 200 backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java#!/bin/bash
# Locate the remove method in RoundManagerRegistry to inspect its implementation
rg -n "remove(" -F backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java
# Dump the RoomRoundManager class to check for any internal cleanup hooks
sed -n '1,200p' backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java
# Search within RoomRoundManager for scheduler cancellation or shutdown logic
rg -nE "cancel|shutdown|dispose|stop" backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java🏁 Script executed:
#!/bin/bash
# Show the remove method implementation in RoundManagerRegistry
rg -F -n "remove(" -A 10 backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java
# Print first 200 lines of RoomRoundManager to inspect any cleanup logic
sed -n '1,200p' backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java
# Search for scheduler or other cleanup calls inside RoomRoundManager
rg -nE "cancel|shutdown|dispose|stop" backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.javaLength of output: 4765
Ensure full teardown of RoomRoundManager on room cleanup
The current cleanupRoom implementation only removes the manager from the registry, but leaves its scheduled tasks and active flag untouched—potentially allowing background jobs to keep running after the room ends. To fix:
- In RoundManagerRegistry.remove(JoinCode) (backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java:58–60), capture and call a shutdown method on the manager before removing it.
- In RoomRoundManager (backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java), add a public shutdown() that:
- sets isActive to false
- grabs and cancels the current ScheduledFuture
Example diff:
// RoundManagerRegistry.java
public void remove(JoinCode joinCode) {
- roomManagers.remove(joinCode);
+ RoomRoundManager manager = roomManagers.remove(joinCode);
+ if (manager != null) {
+ manager.shutdown();
+ }
}
// RoomRoundManager.java
public class RoomRoundManager {
// existing fields...
+
+ /** Stops all future scheduling and marks the manager inactive. */
+ public void shutdown() {
+ isActive = false;
+ ScheduledFuture<?> future = currentTask.getAndSet(null);
+ if (future != null) {
+ future.cancel(false);
+ }
+ log.info("방 {} RoundManager 종료 완료", joinCode.value());
+ }
}This ensures no lingering tasks or state remain once a room is cleaned up.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void cleanupRoom(String joinCode) { | |
| JoinCode roomJoinCode = new JoinCode(joinCode); | |
| log.info("방 {} 정리 요청", joinCode); | |
| roundManagerRegistry.remove(roomJoinCode); | |
| } | |
| // In RoundManagerRegistry.java | |
| public void remove(JoinCode joinCode) { | |
| RoomRoundManager manager = roomManagers.remove(joinCode); | |
| if (manager != null) { | |
| manager.shutdown(); | |
| } | |
| } |
| public void cleanupRoom(String joinCode) { | |
| JoinCode roomJoinCode = new JoinCode(joinCode); | |
| log.info("방 {} 정리 요청", joinCode); | |
| roundManagerRegistry.remove(roomJoinCode); | |
| } | |
| // In RoomRoundManager.java | |
| public class RoomRoundManager { | |
| // existing fields... | |
| /** Stops all future scheduling and marks the manager inactive. */ | |
| public void shutdown() { | |
| isActive = false; | |
| ScheduledFuture<?> future = currentTask.getAndSet(null); | |
| if (future != null) { | |
| future.cancel(false); | |
| } | |
| log.info("방 {} RoundManager 종료 완료", joinCode.value()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/coffeeshout/minigame/application/CardGameService.java
around lines 91 to 95, the cleanupRoom method only removes the RoomRoundManager
from the registry but does not stop its scheduled tasks or deactivate it. To fix
this, update RoundManagerRegistry.remove(JoinCode) to first retrieve the manager
and call a new public shutdown() method on it before removal. Then, in
RoomRoundManager, implement the shutdown() method to set isActive to false and
cancel any ongoing ScheduledFuture tasks, ensuring all background jobs are
stopped when a room is cleaned up.
| public class ReadyPhaseHandler implements RoundPhaseHandler { | ||
| @Override | ||
| public void execute(CardGame game, Room room) { | ||
| log.debug("라운드 {} 로딩 시작", game.getRoundState().roundNumber()); |
There was a problem hiding this comment.
Fix inconsistent log message for READY phase.
The log message says "로딩 시작" (loading start) but this is the READY phase handler. The message should reflect the actual phase being handled.
- log.debug("라운드 {} 로딩 시작", game.getRoundState().roundNumber());
+ log.debug("라운드 {} 준비 완료", game.getRoundState().roundNumber());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| log.debug("라운드 {} 로딩 시작", game.getRoundState().roundNumber()); | |
| log.debug("라운드 {} 준비 완료", game.getRoundState().roundNumber()); |
🤖 Prompt for AI Agents
In
backend/src/main/java/coffeeshout/minigame/domain/round/handler/ReadyPhaseHandler.java
at line 16, the log message incorrectly states "로딩 시작" which means "loading
start" but this handler is for the READY phase. Update the log message to
accurately reflect the READY phase, such as changing it to "준비 시작" or a similar
phrase indicating the start of the READY phase.
| private final AtomicReference<ScheduledFuture<?>> currentTask = new AtomicReference<>(); | ||
| /** | ||
| * -- GETTER -- | ||
| * 현재 활성 상태인지 확인합니다. | ||
| */ | ||
| @Getter | ||
| private volatile boolean isActive = true; |
There was a problem hiding this comment.
Add proper shutdown mechanism
The class has an isActive flag and currentTask reference but lacks a proper shutdown method to cancel scheduled tasks and clean up resources. This causes the memory leak issue mentioned in RoundManagerRegistry.
Add a shutdown method:
+ /**
+ * Shuts down this round manager and cancels any scheduled tasks
+ */
+ public void shutdown() {
+ isActive = false;
+ ScheduledFuture<?> task = currentTask.getAndSet(null);
+ if (task != null && !task.isDone()) {
+ task.cancel(false);
+ log.info("방 {} - 예약된 작업 취소됨", joinCode.value());
+ }
+ log.info("방 {} - RoundManager 종료", joinCode.value());
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private final AtomicReference<ScheduledFuture<?>> currentTask = new AtomicReference<>(); | |
| /** | |
| * -- GETTER -- | |
| * 현재 활성 상태인지 확인합니다. | |
| */ | |
| @Getter | |
| private volatile boolean isActive = true; | |
| private final AtomicReference<ScheduledFuture<?>> currentTask = new AtomicReference<>(); | |
| /** | |
| * -- GETTER -- | |
| * 현재 활성 상태인지 확인합니다. | |
| */ | |
| @Getter | |
| private volatile boolean isActive = true; | |
| /** | |
| * Shuts down this round manager and cancels any scheduled tasks | |
| */ | |
| public void shutdown() { | |
| isActive = false; | |
| ScheduledFuture<?> task = currentTask.getAndSet(null); | |
| if (task != null && !task.isDone()) { | |
| task.cancel(false); | |
| log.info("방 {} - 예약된 작업 취소됨", joinCode.value()); | |
| } | |
| log.info("방 {} - RoundManager 종료", joinCode.value()); | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java
around lines 33 to 39, the class has an isActive flag and currentTask reference
but lacks a proper shutdown method to cancel scheduled tasks and clean up
resources. Add a public shutdown method that sets isActive to false, cancels the
currentTask if present, and clears any related resources to prevent memory
leaks.
| ScheduledFuture<?> future = scheduler.schedule( | ||
| () -> { | ||
| if (isActive) { | ||
| moveToNextPhase(game, room, onStateChange); | ||
| } | ||
| }, | ||
| Instant.now().plus(duration) | ||
| ); | ||
|
|
||
| currentTask.set(future); |
There was a problem hiding this comment.
Cancel previous task before scheduling new one
When scheduling a new task, the previous task (if any) should be cancelled to prevent task accumulation.
ScheduledFuture<?> future = scheduler.schedule(
() -> {
if (isActive) {
moveToNextPhase(game, room, onStateChange);
}
},
Instant.now().plus(duration)
);
+// Cancel previous task if exists
+ScheduledFuture<?> previousTask = currentTask.getAndSet(future);
+if (previousTask != null && !previousTask.isDone()) {
+ previousTask.cancel(false);
+}
-currentTask.set(future);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ScheduledFuture<?> future = scheduler.schedule( | |
| () -> { | |
| if (isActive) { | |
| moveToNextPhase(game, room, onStateChange); | |
| } | |
| }, | |
| Instant.now().plus(duration) | |
| ); | |
| currentTask.set(future); | |
| ScheduledFuture<?> future = scheduler.schedule( | |
| () -> { | |
| if (isActive) { | |
| moveToNextPhase(game, room, onStateChange); | |
| } | |
| }, | |
| Instant.now().plus(duration) | |
| ); | |
| // Cancel previous task if exists | |
| ScheduledFuture<?> previousTask = currentTask.getAndSet(future); | |
| if (previousTask != null && !previousTask.isDone()) { | |
| previousTask.cancel(false); | |
| } |
🤖 Prompt for AI Agents
In backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java
around lines 102 to 111, before scheduling the new task with scheduler.schedule,
check if currentTask contains a previously scheduled task and cancel it if
present. This prevents accumulation of multiple scheduled tasks by ensuring only
one active task exists at a time. After cancelling the previous task, proceed to
schedule the new one and update currentTask with the new ScheduledFuture.
| /** | ||
| * 참조되는 인스턴스가 존재한다면, 해당 인스턴스의 스케줄링 작업이 제거되지 않을 수 있다. | ||
| * 우선은 문제가 없으니 스케줄링 제거는 없애지 않는다. | ||
| */ | ||
|
|
||
| public void remove(JoinCode joinCode) { | ||
| roomManagers.remove(joinCode); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical: Address potential scheduling task leak
The comment indicates awareness of a potential memory/resource leak where scheduled tasks may not be removed if instances are still referenced. This could lead to:
- Memory leaks from uncancelled scheduled tasks
- Unnecessary CPU usage from tasks running for non-existent rooms
- Potential NPEs or unexpected behavior
Consider implementing proper cleanup:
public void remove(JoinCode joinCode) {
+ RoomRoundManager manager = roomManagers.get(joinCode);
+ if (manager != null) {
+ manager.shutdown(); // Add shutdown method to cancel scheduled tasks
+ }
roomManagers.remove(joinCode);
}Let me verify if RoomRoundManager has any cleanup mechanism:
🏁 Script executed:
#!/bin/bash
# Check if RoomRoundManager has any cleanup/shutdown methods
ast-grep --pattern 'class RoomRoundManager {
$$$
shutdown($$$) {
$$$
}
$$$
}'
# Check for any task cancellation logic
rg -A 5 "currentTask.*cancel|future.*cancel" backend/src/main/java/coffeeshout/minigame/domain/round/Length of output: 195
🏁 Script executed:
#!/bin/bash
set -e
# Locate RoomRoundManager.java
file=$(fd 'RoomRoundManager.java' -t f)
echo "Inspecting: $file"
# Show how scheduled tasks are created
echo -e "\n=== Scheduling Task Creation ==="
rg -n -C 2 "schedule" "$file" || true
rg -n -C 2 "ScheduledExecutorService" "$file" || true
rg -n -C 2 "Timer" "$file" || true
# Show any cancellation or shutdown hooks
echo -e "\n=== Cancellation / Shutdown Logic ==="
rg -n -C 2 "cancel" "$file" || true
rg -n -C 2 "shutdown" "$file" || trueLength of output: 2327
Implement cleanup to prevent scheduled-task leaks
The registry’s remove(JoinCode) simply drops the manager from the map, but each RoomRoundManager continues scheduling tasks (via TaskScheduler.schedule) without ever cancelling them or marking them inactive. This will lead to:
- Uncancelled tasks holding onto references → memory leaks
- CPU usage from orphaned task executions
- Unexpected behavior if tasks fire after the room is “removed”
Please address this by:
• In RoomRoundManager (backend/src/main/java/coffeeshout/minigame/domain/round/RoomRoundManager.java):
– Add a shutdown() (or cancelAll()) method that:
1. Sets isActive = false
2. Calls currentTask.get().cancel(false) (if non-null) to stop the pending task
3. Clears the currentTask reference
• In RoundManagerRegistry.remove(JoinCode) (backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java):
– Lookup the manager before removal and invoke manager.shutdown()
– Then call roomManagers.remove(joinCode)
Diff sketch:
// in RoomRoundManager.java
+ public void shutdown() {
+ isActive = false;
+ ScheduledFuture<?> future = currentTask.getAndSet(null);
+ if (future != null) {
+ future.cancel(false);
+ }
+ }
// ensure scheduleNextPhase stores the future:
private void scheduleNextPhase(...) {
ScheduledFuture<?> future = scheduler.schedule(...);
+ currentTask.set(future);
}
// in RoundManagerRegistry.java
public void remove(JoinCode joinCode) {
+ RoomRoundManager manager = roomManagers.get(joinCode);
+ if (manager != null) {
+ manager.shutdown();
+ }
roomManagers.remove(joinCode);
}This will ensure all pending tasks are properly cancelled when a room is removed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * 참조되는 인스턴스가 존재한다면, 해당 인스턴스의 스케줄링 작업이 제거되지 않을 수 있다. | |
| * 우선은 문제가 없으니 스케줄링 제거는 없애지 않는다. | |
| */ | |
| public void remove(JoinCode joinCode) { | |
| roomManagers.remove(joinCode); | |
| } | |
| /** | |
| * 참조되는 인스턴스가 존재한다면, 해당 인스턴스의 스케줄링 작업이 제거되지 않을 수 있다. | |
| * 우선은 문제가 없으니 스케줄링 제거는 없애지 않는다. | |
| */ | |
| public void remove(JoinCode joinCode) { | |
| RoomRoundManager manager = roomManagers.get(joinCode); | |
| if (manager != null) { | |
| manager.shutdown(); | |
| } | |
| roomManagers.remove(joinCode); | |
| } |
🤖 Prompt for AI Agents
In
backend/src/main/java/coffeeshout/minigame/domain/round/RoundManagerRegistry.java
around lines 53 to 60, the remove(JoinCode) method currently only removes the
manager from the map without cancelling its scheduled tasks, causing potential
memory leaks and unwanted task executions. To fix this, first update
RoomRoundManager.java to add a shutdown() method that sets isActive to false,
cancels the current scheduled task if present, and clears the task reference.
Then, modify RoundManagerRegistry.remove(JoinCode) to retrieve the manager
instance before removal, call its shutdown() method to cancel tasks, and finally
remove it from the map.
| //package coffeeshout.minigame.domain.cardgame; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThat; | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| // | ||
| //import org.assertj.core.api.SoftAssertions; | ||
| //import org.junit.jupiter.api.Test; | ||
| //import org.junit.jupiter.params.ParameterizedTest; | ||
| //import org.junit.jupiter.params.provider.CsvSource; | ||
| // | ||
| //class CardGameRoundTest { | ||
| // | ||
| // @Test | ||
| // void 다음_라운드로_진행한다() { | ||
| // // given & when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(CardGameRound.READY.next()).isEqualTo(CardGameRound.FIRST); | ||
| // softly.assertThat(CardGameRound.FIRST.next()).isEqualTo(CardGameRound.SECOND); | ||
| // softly.assertThat(CardGameRound.SECOND.next()).isEqualTo(CardGameRound.END); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 마지막_라운드에서_다음_라운드로_진행하면_예외가_발생한다() { | ||
| // // when & then | ||
| // assertThatThrownBy(() -> CardGameRound.END.next()) | ||
| // .isInstanceOf(IllegalStateException.class); | ||
| // } | ||
| // | ||
| // @ParameterizedTest | ||
| // @CsvSource({ | ||
| // "READY, 0", | ||
| // "FIRST, 1", | ||
| // "SECOND, 2", | ||
| // "END, 3" | ||
| // }) | ||
| // void 라운드를_정수로_변환한다(CardGameRound round, int expected) { | ||
| // // when | ||
| // int result = round.toInteger(); | ||
| // | ||
| // // then | ||
| // assertThat(result).isEqualTo(expected); | ||
| // } | ||
| //} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider removing this file entirely instead of commenting out.
While commenting out these tests aligns with the migration from CardGameRound enum to the new state pattern, leaving commented-out code can create maintenance debt.
Consider either:
- Removing this file entirely if the functionality is covered by new tests (like
RoundStateTest) - Replacing with tests for the new round management system
This keeps the codebase cleaner and avoids confusion about which tests are active.
🤖 Prompt for AI Agents
In
backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameRoundTest.java
lines 1 to 44, the entire test class is commented out due to migration from the
CardGameRound enum to a new state pattern. Instead of leaving this commented
code, remove the file entirely if the new round management system is covered by
other tests like RoundStateTest, or replace these tests with ones targeting the
new implementation. This will keep the codebase clean and avoid confusion about
active tests.
| //package coffeeshout.minigame.domain.cardgame; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThat; | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| // | ||
| //import coffeeshout.fixture.CardGameDeckStub; | ||
| //import coffeeshout.fixture.CardGameFake; | ||
| //import coffeeshout.fixture.PlayerProbabilitiesFixture; | ||
| //import coffeeshout.minigame.domain.MiniGameScore; | ||
| //import coffeeshout.minigame.domain.cardgame.card.Card; | ||
| //import coffeeshout.minigame.domain.cardgame.card.CardGameDeckGenerator; | ||
| //import coffeeshout.room.domain.player.Player; | ||
| //import coffeeshout.room.domain.player.PlayerName; | ||
| //import coffeeshout.room.domain.player.Players; | ||
| //import java.util.Map; | ||
| //import java.util.Optional; | ||
| //import org.assertj.core.api.SoftAssertions; | ||
| //import org.junit.jupiter.api.BeforeEach; | ||
| //import org.junit.jupiter.api.Nested; | ||
| //import org.junit.jupiter.api.Test; | ||
| // | ||
| //class CardGameTest { | ||
| // | ||
| // CardGame cardGame; | ||
| // Players players; | ||
| // CardGameDeckGenerator deckGenerator = new CardGameDeckStub(); | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // players = new Players(); | ||
| // PlayerProbabilitiesFixture.PLAYERS.forEach(players::join); | ||
| // | ||
| // cardGame = new CardGameFake(deckGenerator); | ||
| // cardGame.startGame(players.getPlayers()); | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 게임_초기화_테스트 { | ||
| // | ||
| // @Test | ||
| // void 카드게임이_READY_상태로_시작한다() { | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardGame.getRound()).isEqualTo(CardGameRound.READY); | ||
| // softly.assertThat(cardGame.getState()).isEqualTo(CardGameState.READY); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 게임_시작시_첫번째_라운드로_진행한다() { | ||
| // // when | ||
| // cardGame.startRound(); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardGame.getRound()).isEqualTo(CardGameRound.FIRST); | ||
| // softly.assertThat(cardGame.getState()).isEqualTo(CardGameState.LOADING); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드_시작() { | ||
| // // given | ||
| // cardGame.startRound(); // FIRST 라운드 | ||
| // | ||
| // // when | ||
| // cardGame.startRound(); // SECOND 라운드 | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getRound()).isEqualTo(CardGameRound.SECOND); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_선택_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); // FIRST 라운드 시작 | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 플레이어가_카드를_선택한다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // | ||
| // // when | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(player, 0); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getPlayerHands().totalHandSize()).isEqualTo(1); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 게임이_진행중이_아닐때_카드_선택시_예외_발생() { | ||
| // // given | ||
| // cardGame.changeScoreBoardState(); // PLAYING이 아닌 상태로 변경 | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> cardGame.selectCard(player, 0)) | ||
| // .isInstanceOf(IllegalStateException.class) | ||
| // .hasMessage("현재 게임이 진행중인 상태가 아닙니다."); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 여러_플레이어가_카드를_선택한다() { | ||
| // // when | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getPlayerHands().totalHandSize()).isEqualTo(3); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 플레이어들이_같은_카드를_선택하면_예외를_반환한다(){ | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // Player player1 = players.getPlayer(new PlayerName("꾹이")); | ||
| // Player player2 = players.getPlayer(new PlayerName("루키")); | ||
| // | ||
| // // when | ||
| // cardGame.selectCard(player1, 0); | ||
| // | ||
| // // then | ||
| // assertThatThrownBy(() -> cardGame.selectCard(player2, 0)) | ||
| // .isInstanceOf(IllegalStateException.class); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 점수_계산_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 플레이어들의_점수를_계산한다() { | ||
| // // given | ||
| // Player player1 = players.getPlayer(new PlayerName("꾹이")); | ||
| // Player player2 = players.getPlayer(new PlayerName("루키")); | ||
| // Player player3 = players.getPlayer(new PlayerName("엠제이")); | ||
| // Player player4 = players.getPlayer(new PlayerName("한스")); | ||
| // | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(player1, 0); | ||
| // cardGame.selectCard(player2, 1); | ||
| // cardGame.selectCard(player3, 2); | ||
| // cardGame.selectCard(player4, 3); | ||
| // | ||
| // // when | ||
| // Map<Player, MiniGameScore> scores = cardGame.getScores(); | ||
| // | ||
| // // then - 점수가 계산되는지 확인 (shuffle에 의해 실제 값은 변할 수 있음) | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(scores).hasSize(4); | ||
| // softly.assertThat(scores.get(player1)).isNotNull(); | ||
| // softly.assertThat(scores.get(player2)).isNotNull(); | ||
| // softly.assertThat(scores.get(player3)).isNotNull(); | ||
| // softly.assertThat(scores.get(player4)).isNotNull(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두_라운드를_진행할_수_있다() { | ||
| // // given | ||
| // Player player1 = players.getPlayer(new PlayerName("꾹이")); | ||
| // Player player2 = players.getPlayer(new PlayerName("루키")); | ||
| // Player player3 = players.getPlayer(new PlayerName("엠제이")); | ||
| // Player player4 = players.getPlayer(new PlayerName("한스")); | ||
| // | ||
| // // 첫 번째 라운드 완료 | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(player1, 0); | ||
| // cardGame.selectCard(player2, 1); | ||
| // cardGame.selectCard(player3, 2); | ||
| // cardGame.selectCard(player4, 3); | ||
| // | ||
| // // when - 두 번째 라운드 시작 가능한지 확인 | ||
| // assertThat(cardGame.isFinished(CardGameRound.FIRST)).isTrue(); | ||
| // | ||
| // // then - 점수가 계산되는지 확인 | ||
| // Map<Player, MiniGameScore> scores = cardGame.getScores(); | ||
| // assertThat(scores).hasSize(4); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 라운드_완료_확인_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드가_완료되지_않음() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // | ||
| // // when & then | ||
| // assertThat(cardGame.isFinished(CardGameRound.FIRST)).isFalse(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드가_완료됨() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("한스")), 3); | ||
| // | ||
| // // when & then | ||
| // assertThat(cardGame.isFinished(CardGameRound.FIRST)).isTrue(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 현재_라운드_완료_확인() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // assertThat(cardGame.isFinishedThisRound()).isFalse(); | ||
| // | ||
| // // when | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("한스")), 3); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.isFinishedThisRound()).isTrue(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 라운드_진행_상태를_확인한다() { | ||
| // // given - 첫 번째 라운드 완료 | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // assertThat(cardGame.isFinished(CardGameRound.FIRST)).isFalse(); | ||
| // | ||
| // // when - 마지막 플레이어가 선택 | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("한스")), 3); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.isFinished(CardGameRound.FIRST)).isTrue(); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 플레이어_조회_테스트 { | ||
| // | ||
| // @Test | ||
| // void 이름으로_플레이어를_찾는다() { | ||
| // // given | ||
| // String playerName = "꾹이"; | ||
| // | ||
| // // when | ||
| // Player foundPlayer = cardGame.findPlayerByName(new PlayerName(playerName)); | ||
| // | ||
| // // then | ||
| // assertThat(foundPlayer.getName().value()).isEqualTo(playerName); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 존재하지_않는_플레이어_조회시_예외_발생() { | ||
| // // given | ||
| // String nonExistentName = "존재하지않는플레이어"; | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> cardGame.findPlayerByName(new PlayerName(nonExistentName))) | ||
| // .isInstanceOf(IllegalArgumentException.class); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 라운드_상태_확인_테스트 { | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드_확인() { | ||
| // // given | ||
| // cardGame.startRound(); | ||
| // | ||
| // // when & then | ||
| // assertThat(cardGame.getRound()).isEqualTo(CardGameRound.FIRST); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드_확인() { | ||
| // // given | ||
| // cardGame.startRound(); // FIRST | ||
| // cardGame.startRound(); // SECOND | ||
| // | ||
| // // when & then | ||
| // assertThat(cardGame.getRound()).isEqualTo(CardGameRound.SECOND); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 랜덤_카드_할당_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 선택하지_않은_플레이어에게_랜덤_카드_할당() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // // players.getPlayer(new PlayerName("엠제이")), players.getPlayer(new PlayerName("한스"))은 선택하지 않음 | ||
| // | ||
| // // when | ||
| // cardGame.assignRandomCardsToUnselectedPlayers(); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getPlayerHands().totalHandSize()).isEqualTo(4); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 모든_플레이어가_선택한_경우_변화_없음() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("한스")), 3); | ||
| // int sizeBefore = cardGame.getPlayerHands().totalHandSize(); | ||
| // | ||
| // // when | ||
| // cardGame.assignRandomCardsToUnselectedPlayers(); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getPlayerHands().totalHandSize()).isEqualTo(sizeBefore); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_소유자_조회_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 현재_라운드의_카드_소유자를_찾는다() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // cardGame.selectCard(player, 0); | ||
| // Card selectedCard = cardGame.getDeck().getCards().get(0); | ||
| // | ||
| // // when | ||
| // Optional<Player> cardOwner = cardGame.findCardOwnerInCurrentRound(selectedCard); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardOwner).isPresent(); | ||
| // softly.assertThat(cardOwner.get()).isEqualTo(player); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 선택되지_않은_카드의_소유자를_찾으면_빈_Optional_반환() { | ||
| // // given | ||
| // Card unselectedCard = cardGame.getDeck().getCards().get(5); | ||
| // | ||
| // // when | ||
| // Optional<Player> cardOwner = cardGame.findCardOwnerInCurrentRound(unselectedCard); | ||
| // | ||
| // // then | ||
| // assertThat(cardOwner).isEmpty(); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 게임_상태_변경_테스트 { | ||
| // | ||
| // @Test | ||
| // void 점수판_상태로_변경() { | ||
| // // when | ||
| // cardGame.changeScoreBoardState(); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getState()).isEqualTo(CardGameState.SCORE_BOARD); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 라운드를_시작하면_로딩부터_시작된다() { | ||
| // // when | ||
| // cardGame.startRound(); | ||
| // | ||
| // // then | ||
| // assertThat(cardGame.getState()).isEqualTo(CardGameState.LOADING); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 게임_결과_테스트 { | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardGame.startRound(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 게임_결과를_반환한다() { | ||
| // // given | ||
| // cardGame.startPlay(); | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("꾹이")), 0); // 40 | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("루키")), 1); // 30 | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("엠제이")), 2); // 20 | ||
| // cardGame.selectCard(players.getPlayer(new PlayerName("한스")), 3); // 10 | ||
| // | ||
| // // when | ||
| // var result = cardGame.getResult(); | ||
| // | ||
| // // then | ||
| // assertThat(result).isNotNull(); | ||
| // } | ||
| // } | ||
| //} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Significant test coverage loss requires verification.
This comprehensive test file covered critical card game functionality including game initialization, card selection, scoring, round progression, and player management. Commenting out 400+ lines of tests without replacement raises concerns about regression risk.
Please verify that equivalent test coverage exists for the new round management system, particularly for:
- Game state transitions
- Card selection validation
- Score calculation
- Round completion logic
- Player management
Consider creating a migration checklist to ensure critical test scenarios are preserved in the new testing approach.
🏁 Script executed:
#!/bin/bash
# Description: Check for new test files that might replace this functionality
# Expected: Find new test files for round management and card game functionality
echo "=== Searching for new card game test files ==="
fd -e java -x basename {} | grep -i test | grep -E "(round|cardgame|phase)" | sort -u
echo -e "\n=== Searching for RoundState and RoundPhase tests ==="
rg -l "RoundState|RoundPhase" --type java --glob "*Test.java"
echo -e "\n=== Checking CardGameService test coverage ==="
rg -A 5 -B 5 "@Test" backend/src/test/java/coffeeshout/minigame/application/CardGameServiceTest.java | head -20Length of output: 1428
Tests Migration Coverage Incomplete – Action Required
We’ve identified the new test suite files under backend/src/test/java/coffeeshout/minigame/…:
• RoundStateTest – verifies round‐state transitions
• CardGameServiceTest & CardGameServiceRealTimeTest – cover game start and RoundManager registration
• CardGameIntegrationTest – end-to-end integration flows
However, the original CardGameTest also exercised critical behaviors that aren’t covered by these new tests:
• Card selection rules (valid selections, duplicate selections, invalid-state exceptions)
• Score calculation and retrieval (getScores())
• Round completion logic (isFinished(), isFinishedThisRound())
• Random card assignment to unselected players
• Player lookup by name (findPlayerByName())
• Determining card owners in a round (findCardOwnerInCurrentRound())
• Final game result retrieval (getResult())
Please ensure we have equivalent unit or integration tests for each of these scenarios in the new framework. Consider creating or extending test classes to cover:
- Card selection validations and exception paths
- Score computation stability under varying shuffle orders
- Full round lifecycle checks (all players, partial selections)
- Random fallback card assignments
- Player‐lookup and card‐owner queries
- Final game result generation
Once these are in place, update the migration checklist and mark this review comment as resolved.
🤖 Prompt for AI Agents
In backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardGameTest.java
lines 1 to 435, the original CardGameTest covers critical behaviors missing in
the new test suites, including card selection rules, score calculation, round
completion logic, random card assignment, player lookup, card owner
determination, and final game result retrieval. To fix this, ensure that
equivalent unit or integration tests are created or extended in the new test
classes to cover all these scenarios comprehensively. Add tests for card
selection validations and exceptions, score computation under shuffle, full
round lifecycle, random card assignments, player and card owner queries, and
final game result generation, then update the migration checklist and mark this
review as resolved.
| //package coffeeshout.minigame.domain.cardgame; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThat; | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| // | ||
| //import coffeeshout.minigame.domain.cardgame.card.AdditionCard; | ||
| //import coffeeshout.minigame.domain.cardgame.card.Card; | ||
| //import coffeeshout.minigame.domain.cardgame.card.MultiplierCard; | ||
| //import org.assertj.core.api.SoftAssertions; | ||
| //import org.junit.jupiter.api.BeforeEach; | ||
| //import org.junit.jupiter.api.Nested; | ||
| //import org.junit.jupiter.api.Test; | ||
| // | ||
| //class CardHandTest { | ||
| // | ||
| // private CardHand cardHand; | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // cardHand = new CardHand(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 가지고_있는_카드들의_점수합을_반환한다() { | ||
| // // given | ||
| // cardHand.put(new AdditionCard(10)); | ||
| // cardHand.put(new MultiplierCard(-2)); | ||
| // cardHand.put(new AdditionCard(30)); | ||
| // | ||
| // // when | ||
| // CardGameScore cardGameScore = cardHand.calculateCardGameScore(); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardGameScore.getValue()).isEqualTo(-80); | ||
| // softly.assertThat(cardHand.size()).isEqualTo(3); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 해당_라운드에_해당_카드를_뽑았으면_true를_반환한다() { | ||
| // // given | ||
| // Card firstRoundCard = new AdditionCard(10); | ||
| // Card secondRoundCard = new MultiplierCard(-2); | ||
| // Card notPickedCard = new MultiplierCard(1); | ||
| // cardHand.put(firstRoundCard); | ||
| // cardHand.put(secondRoundCard); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.hasCardForRound(1)).isTrue(); | ||
| // softly.assertThat(cardHand.hasCardForRound(2)).isTrue(); | ||
| // softly.assertThat(cardHand.hasCardForRound(notPickedCard, CardGameRound.FIRST)).isFalse(); | ||
| // softly.assertThat(cardHand.hasCardForRound(notPickedCard, CardGameRound.SECOND)).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 해당_라운드에_카드를_뽑았으면_true를_반환한다() { | ||
| // // given | ||
| // cardHand.put(new AdditionCard(10)); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.(CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.SECOND)).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_추가_테스트 { | ||
| // | ||
| // @Test | ||
| // void 카드를_추가한다() { | ||
| // // given | ||
| // Card card = AdditionCard.PLUS_40; | ||
| // | ||
| // // when | ||
| // cardHand.put(card); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.size()).isEqualTo(1); | ||
| // softly.assertThat(cardHand.getCard(0)).isEqualTo(card); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 여러_카드를_추가한다() { | ||
| // // given | ||
| // Card card1 = AdditionCard.PLUS_40; | ||
| // Card card2 = MultiplierCard.DOUBLE; | ||
| // | ||
| // // when | ||
| // cardHand.put(card1); | ||
| // cardHand.put(card2); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.size()).isEqualTo(2); | ||
| // softly.assertThat(cardHand.getCard(0)).isEqualTo(card1); | ||
| // softly.assertThat(cardHand.getCard(1)).isEqualTo(card2); | ||
| // }); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_조회_테스트 { | ||
| // | ||
| // @Test | ||
| // void 인덱스로_카드를_조회한다() { | ||
| // // given | ||
| // Card card = AdditionCard.PLUS_30; | ||
| // cardHand.put(card); | ||
| // | ||
| // // when | ||
| // Card retrievedCard = cardHand.getCard(0); | ||
| // | ||
| // // then | ||
| // assertThat(retrievedCard).isEqualTo(card); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 잘못된_인덱스로_카드를_조회하면_예외가_발생한다() { | ||
| // // when & then | ||
| // assertThatThrownBy(() -> cardHand.getCard(0)) | ||
| // .isInstanceOf(IndexOutOfBoundsException.class); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 점수_계산_테스트 { | ||
| // | ||
| // @Test | ||
| // void 빈_핸드의_점수를_계산한다() { | ||
| // // when | ||
| // CardGameScore score = cardHand.calculateCardGameScore(); | ||
| // | ||
| // // then | ||
| // assertThat(score.getValue()).isEqualTo(0); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 덧셈_카드만_있는_핸드의_점수를_계산한다() { | ||
| // // given | ||
| // cardHand.put(AdditionCard.PLUS_40); | ||
| // cardHand.put(AdditionCard.PLUS_30); | ||
| // | ||
| // // when | ||
| // CardGameScore score = cardHand.calculateCardGameScore(); | ||
| // | ||
| // // then | ||
| // assertThat(score.getValue()).isEqualTo(70); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 라운드_선택_확인_테스트 { | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드에서_선택했는지_확인한다() { | ||
| // // given | ||
| // cardHand.put(AdditionCard.PLUS_40); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.SECOND)).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드에서_선택했는지_확인한다() { | ||
| // // given | ||
| // cardHand.put(AdditionCard.PLUS_40); // 첫 번째 라운드 | ||
| // cardHand.put(MultiplierCard.DOUBLE); // 두 번째 라운드 | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.SECOND)).isTrue(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 아무_카드도_선택하지_않았을_때_확인한다() { | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.FIRST)).isFalse(); | ||
| // softly.assertThat(cardHand.isSelected(CardGameRound.SECOND)).isFalse(); | ||
| // }); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_할당_확인_테스트 { | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드에_할당된_카드인지_확인한다() { | ||
| // // given | ||
| // Card card1 = AdditionCard.PLUS_40; | ||
| // Card card2 = MultiplierCard.DOUBLE; | ||
| // cardHand.put(card1); | ||
| // cardHand.put(card2); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.hasCardForRound(card1, CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.hasCardForRound(card2, CardGameRound.FIRST)).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드에_할당된_카드인지_확인한다() { | ||
| // // given | ||
| // Card card1 = AdditionCard.PLUS_40; | ||
| // Card card2 = MultiplierCard.DOUBLE; | ||
| // cardHand.put(card1); | ||
| // cardHand.put(card2); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.hasCardForRound(card1, CardGameRound.SECOND)).isFalse(); | ||
| // softly.assertThat(cardHand.hasCardForRound(card2, CardGameRound.SECOND)).isTrue(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 존재하지_않는_카드는_할당되지_않은_카드이다() { | ||
| // // given | ||
| // Card existingCard = AdditionCard.PLUS_40; | ||
| // Card nonExistentCard = AdditionCard.PLUS_30; | ||
| // cardHand.put(existingCard); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.hasCardForRound(existingCard, CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.hasCardForRound(nonExistentCard, CardGameRound.FIRST)).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 라운드_범위를_벗어나면_할당되지_않은_카드이다() { | ||
| // // given | ||
| // Card card = AdditionCard.PLUS_40; | ||
| // cardHand.put(card); | ||
| // | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardHand.hasCardForRound(card, CardGameRound.FIRST)).isTrue(); | ||
| // softly.assertThat(cardHand.hasCardForRound(card, CardGameRound.SECOND)).isFalse(); | ||
| // }); | ||
| // } | ||
| // } | ||
| //} No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rewrite unit tests instead of commenting them out.
The CardHand class is still part of the system and requires proper test coverage. Commenting out all unit tests creates a testing gap that should be addressed.
The tests need to be updated to work with the new round management system:
- Replace
CardGameRoundenum parameters with integer round numbers - Update method calls to use the new API (e.g.,
hasCardForRound(int roundNumber)) - Verify the tests still cover all important CardHand functionality
-//package coffeeshout.minigame.domain.cardgame;
+package coffeeshout.minigame.domain.cardgame;
-//
-//import static org.assertj.core.api.Assertions.assertThat;
+
+import static org.assertj.core.api.Assertions.assertThat;
// ... (rewrite tests for new system)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardHandTest.java
from lines 1 to 255, the entire unit test class is commented out, leaving no
test coverage for CardHand. Rewrite the tests to remove comments and update them
to use the new round management system by replacing all CardGameRound enum
parameters with integer round numbers. Adjust method calls accordingly, such as
changing hasCardForRound(CardGameRound) to hasCardForRound(int roundNumber), and
ensure all tests still validate key CardHand behaviors like score calculation,
card addition, and round selection.
💡 Verification agent
🧩 Analysis chain
Another critical loss of test coverage with incomplete migration
This test file has been entirely commented out, removing all unit test coverage for the CardHand class. The commented code reveals an incomplete migration:
- Lines 51-52: Successfully updated to use integer-based rounds (
hasCardForRound(1),hasCardForRound(2)) - Lines 53-54: Still reference deprecated
CardGameRound.FIRSTandCardGameRound.SECONDenums - Line 65: Contains a syntax error with incomplete method call
cardHand.(CardGameRound.FIRST)
Rather than commenting out the entire test suite, these tests should be properly migrated to work with the new round management system. The CardHand class is likely a core component that needs comprehensive test coverage.
Check if the CardHand class methods have been properly updated to support the new integer-based round system:
🏁 Script executed:
#!/bin/bash
# Verify CardHand class method signatures and usage
ast-grep --pattern 'class CardHand {
$$$
}'
# Check for any remaining references to deprecated CardGameRound enum
rg "CardGameRound" backend/src/main --type javaLength of output: 95
🏁 Script executed:
#!/bin/bash
# Locate the CardHand class file
FILE=$(fd --extension java CardHand.java | head -n1)
echo "CardHand file: $FILE"
echo "----- First 200 lines of CardHand.java -----"
sed -n '1,200p' "$FILE"
echo
echo "----- Lines referencing 'round' or 'CardGameRound' -----"
grep -nE 'round|CardGameRound' "$FILE" || echo "No matches found in CardHand.java"Length of output: 2438
Restore and refactor CardHandTest to use the new integer-based round API
The entire CardHandTest.java suite has been commented out, removing all unit-test coverage for CardHand. Meanwhile, CardHand.java now only exposes:
- hasCardForRound(int roundNumber)
- getCardForRound(int roundNumber)
All enum-based methods (CardGameRound.FIRST/SECOND) and calls like isSelected(CardGameRound) or hasCardForRound(Card, CardGameRound) no longer exist.
Please un-comment and update backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardHandTest.java to:
- Replace every
hasCardForRound(CardGameRound.FIRST)/SECONDwithhasCardForRound(1)/2. - Replace any
getCardForRound(CardGameRound…)withgetCardForRound(…). - Remove or rewrite tests for non-existent methods (
isSelected(...),hasCardForRound(Card, CardGameRound)). - Fix syntax errors (e.g., stray
cardHand.(…)calls).
This will restore coverage and ensure the tests compile against the updated API.
🤖 Prompt for AI Agents
In backend/src/test/java/coffeeshout/minigame/domain/cardgame/CardHandTest.java
from lines 1 to 255, un-comment the entire test class and update it to use the
new integer-based round API by replacing all calls using CardGameRound enums
with integer round numbers (1 for FIRST, 2 for SECOND). Remove or refactor tests
that use removed methods like isSelected(CardGameRound) and
hasCardForRound(Card, CardGameRound). Also, fix syntax errors such as the stray
call cardHand.(CardGameRound.FIRST) by replacing it with the correct method call
using integers. This will restore test coverage and ensure compatibility with
the updated CardHand API.
| //package coffeeshout.minigame.domain.cardgame; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThat; | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| // | ||
| //import coffeeshout.fixture.PlayerFixture; | ||
| //import coffeeshout.minigame.domain.MiniGameScore; | ||
| //import coffeeshout.minigame.domain.cardgame.card.AdditionCard; | ||
| //import coffeeshout.minigame.domain.cardgame.card.Card; | ||
| //import coffeeshout.minigame.domain.cardgame.card.MultiplierCard; | ||
| //import coffeeshout.room.domain.player.Player; | ||
| //import coffeeshout.room.domain.player.PlayerName; | ||
| //import coffeeshout.room.domain.player.Players; | ||
| //import java.util.List; | ||
| //import java.util.Map; | ||
| //import java.util.Optional; | ||
| //import org.assertj.core.api.SoftAssertions; | ||
| //import org.junit.jupiter.api.BeforeEach; | ||
| //import org.junit.jupiter.api.Nested; | ||
| //import org.junit.jupiter.api.Test; | ||
| // | ||
| //class PlayerHandsTest { | ||
| // | ||
| // private PlayerHands playerHands; | ||
| // private Players players; | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // players = new Players(); | ||
| // players.join(PlayerFixture.호스트꾹이()); | ||
| // players.join(PlayerFixture.호스트루키()); | ||
| // players.join(PlayerFixture.호스트한스()); | ||
| // players.join(PlayerFixture.호스트엠제이()); | ||
| // | ||
| // playerHands = new PlayerHands(players.getPlayers()); | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 플레이어_핸드_생성_테스트 { | ||
| // | ||
| // @Test | ||
| // void 플레이어_핸드가_생성된다() { | ||
| // // when & then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(playerHands.playerCount()).isEqualTo(4); | ||
| // softly.assertThat(playerHands.totalHandSize()).isEqualTo(0); | ||
| // }); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_추가_테스트 { | ||
| // | ||
| // @Test | ||
| // void 플레이어에게_카드를_추가한다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // Card card = AdditionCard.PLUS_40; | ||
| // | ||
| // // when | ||
| // playerHands.put(player, card); | ||
| // | ||
| // // then | ||
| // assertThat(playerHands.totalHandSize()).isEqualTo(1); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 여러_플레이어에게_카드를_추가한다() { | ||
| // // given | ||
| // Player player1 = players.getPlayer(new PlayerName("꾹이")); | ||
| // Player player2 = players.getPlayer(new PlayerName("루키")); | ||
| // | ||
| // // when | ||
| // playerHands.put(player1, AdditionCard.PLUS_40); | ||
| // playerHands.put(player2, AdditionCard.PLUS_30); | ||
| // | ||
| // // then | ||
| // assertThat(playerHands.totalHandSize()).isEqualTo(2); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 한_플레이어에게_여러_카드를_추가한다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // | ||
| // // when | ||
| // playerHands.put(player, AdditionCard.PLUS_40); | ||
| // playerHands.put(player, MultiplierCard.DOUBLE); | ||
| // | ||
| // // then | ||
| // assertThat(playerHands.totalHandSize()).isEqualTo(2); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 라운드_종료_확인_테스트 { | ||
| // | ||
| // @Test | ||
| // void 카드가_없으면_라운드가_끝나지_않는다() { | ||
| // // when & then | ||
| // assertThat(playerHands.isRoundFinished()).isFalse(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드가_끝났는지_확인한다() { | ||
| // // given | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), AdditionCard.PLUS_40); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), AdditionCard.PLUS_30); | ||
| // playerHands.put(players.getPlayer(new PlayerName("한스")), AdditionCard.PLUS_20); | ||
| // assertThat(playerHands.isRoundFinished()).isFalse(); | ||
| // | ||
| // // when | ||
| // playerHands.put(players.getPlayer(new PlayerName("엠제이")), AdditionCard.PLUS_10); | ||
| // | ||
| // // then | ||
| // assertThat(playerHands.isRoundFinished()).isTrue(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드가_끝났는지_확인한다() { | ||
| // // given - 첫 번째 라운드 | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), AdditionCard.PLUS_40); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), AdditionCard.PLUS_30); | ||
| // playerHands.put(players.getPlayer(new PlayerName("한스")), AdditionCard.PLUS_20); | ||
| // playerHands.put(players.getPlayer(new PlayerName("엠제이")), AdditionCard.PLUS_10); | ||
| // | ||
| // // when - 두 번째 라운드 | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), MultiplierCard.DOUBLE); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), MultiplierCard.QUADRUPLE); | ||
| // playerHands.put(players.getPlayer(new PlayerName("한스")), MultiplierCard.NULLIFY); | ||
| // assertThat(playerHands.isRoundFinished()).isFalse(); | ||
| // playerHands.put(players.getPlayer(new PlayerName("엠제이")), AdditionCard.ZERO); | ||
| // | ||
| // // then | ||
| // assertThat(playerHands.isRoundFinished()).isTrue(); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 플레이어_검색_테스트 { | ||
| // | ||
| // @Test | ||
| // void 이름으로_플레이어를_찾는다() { | ||
| // // given | ||
| // String playerName = "꾹이"; | ||
| // | ||
| // // when | ||
| // Player foundPlayer = playerHands.findPlayerByName(new PlayerName(playerName)); | ||
| // | ||
| // // then | ||
| // assertThat(foundPlayer.getName().value()).isEqualTo(playerName); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 존재하지_않는_플레이어_이름으로_검색하면_예외가_발생한다() { | ||
| // // given | ||
| // String nonExistentName = "존재하지않는플레이어"; | ||
| // | ||
| // // when & then | ||
| // assertThatThrownBy(() -> playerHands.findPlayerByName(new PlayerName(nonExistentName))) | ||
| // .isInstanceOf(IllegalArgumentException.class); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 점수_계산_테스트 { | ||
| // | ||
| // @Test | ||
| // void 플레이어별_점수를_계산한다() { | ||
| // // given | ||
| // Player player1 = players.getPlayer(new PlayerName("꾹이")); | ||
| // Player player2 = players.getPlayer(new PlayerName("루키")); | ||
| // | ||
| // playerHands.put(player1, AdditionCard.PLUS_40); | ||
| // playerHands.put(player1, MultiplierCard.DOUBLE); | ||
| // playerHands.put(player2, AdditionCard.PLUS_30); | ||
| // playerHands.put(player2, MultiplierCard.NULLIFY); | ||
| // | ||
| // // when | ||
| // Map<Player, MiniGameScore> scores = playerHands.scoreByPlayer(); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(scores.get(player1).getValue()).isEqualTo(80); | ||
| // softly.assertThat(scores.get(player2).getValue()).isEqualTo(0); | ||
| // softly.assertThat(scores).hasSize(4); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 카드가_없는_플레이어의_점수는_0이다() { | ||
| // // when | ||
| // Map<Player, MiniGameScore> scores = playerHands.scoreByPlayer(); | ||
| // | ||
| // // then | ||
| // scores.values().forEach(score -> | ||
| // assertThat(score.getValue()).isEqualTo(0) | ||
| // ); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 선택하지_않은_플레이어_조회_테스트 { | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드에서_선택하지_않은_플레이어를_조회한다() { | ||
| // // given | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), AdditionCard.PLUS_40); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), AdditionCard.PLUS_30); | ||
| // | ||
| // // when | ||
| // List<Player> unselectedPlayers = playerHands.getUnselectedPlayers(CardGameRound.FIRST); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(unselectedPlayers).hasSize(2); | ||
| // softly.assertThat(unselectedPlayers).contains( | ||
| // players.getPlayer(new PlayerName("한스")), | ||
| // players.getPlayer(new PlayerName("엠제이")) | ||
| // ); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드에서_선택하지_않은_플레이어를_조회한다() { | ||
| // // given - 첫 번째 라운드 완료 | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), AdditionCard.PLUS_40); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), AdditionCard.PLUS_30); | ||
| // playerHands.put(players.getPlayer(new PlayerName("한스")), AdditionCard.PLUS_20); | ||
| // playerHands.put(players.getPlayer(new PlayerName("엠제이")), AdditionCard.PLUS_10); | ||
| // | ||
| // // 두 번째 라운드 일부 선택 | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), MultiplierCard.DOUBLE); | ||
| // | ||
| // // when | ||
| // List<Player> unselectedPlayers = playerHands.getUnselectedPlayers(CardGameRound.SECOND); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(unselectedPlayers).hasSize(3); | ||
| // softly.assertThat(unselectedPlayers).contains( | ||
| // players.getPlayer(new PlayerName("루키")), | ||
| // players.getPlayer(new PlayerName("한스")), | ||
| // players.getPlayer(new PlayerName("엠제이")) | ||
| // ); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 모든_플레이어가_선택했으면_빈_리스트를_반환한다() { | ||
| // // given | ||
| // playerHands.put(players.getPlayer(new PlayerName("꾹이")), AdditionCard.PLUS_40); | ||
| // playerHands.put(players.getPlayer(new PlayerName("루키")), AdditionCard.PLUS_30); | ||
| // playerHands.put(players.getPlayer(new PlayerName("한스")), AdditionCard.PLUS_20); | ||
| // playerHands.put(players.getPlayer(new PlayerName("엠제이")), AdditionCard.PLUS_10); | ||
| // | ||
| // // when | ||
| // List<Player> unselectedPlayers = playerHands.getUnselectedPlayers(CardGameRound.FIRST); | ||
| // | ||
| // // then | ||
| // assertThat(unselectedPlayers).isEmpty(); | ||
| // } | ||
| // } | ||
| // | ||
| // @Nested | ||
| // class 카드_소유자_조회_테스트 { | ||
| // | ||
| // @Test | ||
| // void 첫번째_라운드에서_카드_소유자를_찾는다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // Card card = AdditionCard.PLUS_40; | ||
| // playerHands.put(player, card); | ||
| // | ||
| // // when | ||
| // Optional<Player> cardOwner = playerHands.findCardOwner(card, CardGameRound.FIRST); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardOwner).isPresent(); | ||
| // softly.assertThat(cardOwner.get()).isEqualTo(player); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 두번째_라운드에서_카드_소유자를_찾는다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // playerHands.put(player, AdditionCard.PLUS_40); // 첫 번째 라운드 | ||
| // Card secondRoundCard = MultiplierCard.DOUBLE; | ||
| // playerHands.put(player, secondRoundCard); // 두 번째 라운드 | ||
| // | ||
| // // when | ||
| // Optional<Player> cardOwner = playerHands.findCardOwner(secondRoundCard, CardGameRound.SECOND); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(cardOwner).isPresent(); | ||
| // softly.assertThat(cardOwner.get()).isEqualTo(player); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 존재하지_않는_카드의_소유자를_찾으면_빈_Optional을_반환한다() { | ||
| // // given | ||
| // Card nonExistentCard = AdditionCard.PLUS_40; | ||
| // | ||
| // // when | ||
| // Optional<Player> cardOwner = playerHands.findCardOwner(nonExistentCard, CardGameRound.FIRST); | ||
| // | ||
| // // then | ||
| // assertThat(cardOwner).isEmpty(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 잘못된_라운드로_카드_소유자를_찾으면_빈_Optional을_반환한다() { | ||
| // // given | ||
| // Player player = players.getPlayer(new PlayerName("꾹이")); | ||
| // Card card = AdditionCard.PLUS_40; | ||
| // playerHands.put(player, card); // 첫 번째 라운드 | ||
| // | ||
| // // when - 두 번째 라운드에서 첫 번째 라운드 카드를 찾음 | ||
| // Optional<Player> cardOwner = playerHands.findCardOwner(card, CardGameRound.SECOND); | ||
| // | ||
| // // then | ||
| // assertThat(cardOwner).isEmpty(); | ||
| // } | ||
| // } | ||
| //} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Critical PlayerHands functionality loses test coverage.
This test file provided comprehensive coverage for player hand operations including:
- Card addition and management
- Round completion detection
- Player searches and scoring
- Unselected player identification
Since PlayerHands likely still exists in the new architecture (based on AI summary mentioning it was updated), ensure these critical operations have equivalent test coverage in the new system.
The loss of tests for methods like getUnselectedPlayers() and findCardOwner() could leave important edge cases untested.
Consider creating new tests that verify the updated PlayerHands class works correctly with the integer-based round numbers mentioned in the AI summary.
🏁 Script executed:
#!/bin/bash
set -e
# Search for the PlayerHands class implementation
echo "== Locating PlayerHands class =="
rg -n "class PlayerHands" || echo "PlayerHands class not found"
# Search for usages of getUnselectedPlayers in the codebase
echo "== Checking getUnselectedPlayers references =="
rg -n "getUnselectedPlayers" || echo "No references to getUnselectedPlayers"
# Search for usages of findCardOwner in the codebase
echo "== Checking findCardOwner references =="
rg -n "findCardOwner" || echo "No references to findCardOwner"
# Search for any existing PlayerHandsTest files or their replacements
echo "== Locating PlayerHandsTest in tests =="
rg -n "PlayerHandsTest" || echo "No PlayerHandsTest files found"
# Search for any new tests targeting PlayerHands functionality
echo "== Searching for tests mentioning PlayerHands =="
rg -n "PlayerHands" --glob "*Test.java" || echo "No other tests for PlayerHands found"Length of output: 6627
Restore PlayerHands unit test coverage
The commented-out PlayerHandsTest leaves key methods untested. Although integration tests touch playerCount and totalHandSize, we lack focused unit tests for:
- getUnselectedPlayers(int roundNumber)
- findCardOwner(Card card, int roundNumber)
Please add new unit tests covering:
- All edge cases for unselected players across rounds (e.g., no selections, partial selections, full selections).
- Card ownership lookup in each round, including non-existent cards and wrong round numbers.
Pinpointed locations for test additions:
- backend/src/main/java/coffeeshout/minigame/domain/cardgame/PlayerHands.java
• getUnselectedPlayers(int)
• findCardOwner(Card, int) - backend/src/test/java/coffeeshout/minigame/domain/cardgame/PlayerHandsTest.java (replace commented file with fresh tests)
Ensure the new tests use integer round numbers matching the updated API.
🤖 Prompt for AI Agents
In
backend/src/test/java/coffeeshout/minigame/domain/cardgame/PlayerHandsTest.java
lines 1 to 329, the existing commented-out tests do not cover the updated
PlayerHands methods getUnselectedPlayers(int roundNumber) and findCardOwner(Card
card, int roundNumber) which now use integer round numbers. To fix this, rewrite
and add new unit tests that thoroughly cover all edge cases for
getUnselectedPlayers including no selections, partial selections, and full
selections for different rounds, and tests for findCardOwner verifying correct
card ownership per round, handling non-existent cards, and wrong round numbers.
Replace the commented code with fresh tests aligned to the current API using
integer round parameters.
| //package coffeeshout.minigame.ui; | ||
| // | ||
| //import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
| //import static org.assertj.core.api.AssertionsForClassTypes.assertThat; | ||
| // | ||
| //import coffeeshout.fixture.RoomFixture; | ||
| //import coffeeshout.fixture.TestStompSession; | ||
| //import coffeeshout.fixture.TestStompSession.MessageCollector; | ||
| //import coffeeshout.fixture.WebSocketIntegrationTestSupport; | ||
| //import coffeeshout.global.ui.WebSocketResponse; | ||
| //import coffeeshout.minigame.domain.MiniGameType; | ||
| //import coffeeshout.minigame.domain.cardgame.CardGameRound; | ||
| //import coffeeshout.minigame.domain.cardgame.CardGameState; | ||
| //import coffeeshout.minigame.domain.cardgame.CardGameTaskExecutorsV2; | ||
| //import coffeeshout.minigame.domain.task.CardGameTaskType; | ||
| //import coffeeshout.minigame.domain.task.MiniGameTaskManager; | ||
| //import coffeeshout.minigame.ui.request.CommandType; | ||
| //import coffeeshout.minigame.ui.request.MiniGameMessage; | ||
| //import coffeeshout.minigame.ui.request.command.SelectCardCommand; | ||
| //import coffeeshout.minigame.ui.request.command.StartMiniGameCommand; | ||
| //import coffeeshout.minigame.ui.response.MiniGameStartMessage; | ||
| //import coffeeshout.minigame.ui.response.MiniGameStateMessage; | ||
| //import coffeeshout.minigame.ui.response.MiniGameStateMessage.CardInfoMessage; | ||
| //import coffeeshout.room.domain.JoinCode; | ||
| //import coffeeshout.room.domain.Room; | ||
| //import coffeeshout.room.domain.player.Player; | ||
| //import coffeeshout.room.domain.player.PlayerName; | ||
| //import coffeeshout.room.domain.repository.RoomRepository; | ||
| //import com.fasterxml.jackson.core.type.TypeReference; | ||
| //import com.fasterxml.jackson.databind.ObjectMapper; | ||
| //import java.util.List; | ||
| //import java.util.concurrent.ExecutionException; | ||
| //import java.util.concurrent.TimeUnit; | ||
| //import java.util.concurrent.TimeoutException; | ||
| //import org.assertj.core.api.SoftAssertions; | ||
| //import org.junit.jupiter.api.AfterEach; | ||
| //import org.junit.jupiter.api.BeforeEach; | ||
| //import org.junit.jupiter.api.Test; | ||
| //import org.springframework.beans.factory.annotation.Autowired; | ||
| // | ||
| //class CardGameIntegrationTest extends WebSocketIntegrationTestSupport { | ||
| // | ||
| // @Autowired | ||
| // RoomRepository roomRepository; | ||
| // | ||
| // @Autowired | ||
| // ObjectMapper objectMapper; | ||
| // | ||
| // @Autowired | ||
| // CardGameTaskExecutorsV2 cardGameTaskExecutors; | ||
| // | ||
| // JoinCode joinCode; | ||
| // | ||
| // Player host; | ||
| // | ||
| // @BeforeEach | ||
| // void setUp() { | ||
| // joinCode = new JoinCode("A4B2C"); | ||
| // | ||
| // Room room = RoomFixture.호스트_꾹이(); | ||
| // host = room.getHost(); | ||
| // room.addMiniGame(host.getName(), MiniGameType.CARD_GAME.createMiniGame()); | ||
| // | ||
| // roomRepository.save(room); | ||
| // } | ||
| // | ||
| // @AfterEach | ||
| // void tearDown() { | ||
| // // 실행 중인 모든 태스크 취소 및 스레드 정리 | ||
| // MiniGameTaskManager<CardGameTaskType> manager = cardGameTaskExecutors.get(joinCode); | ||
| // manager.cancelAll(); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 카드_게임을_시작한다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // String gameStartUrlFormat = "/topic/room/%s/round"; | ||
| // | ||
| // MiniGameMessage startGameRequest = new MiniGameMessage( | ||
| // CommandType.START_MINI_GAME, | ||
| // objectMapper.valueToTree(new StartMiniGameCommand(host.getName().value())) | ||
| // ); | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStartMessage>> startResponses = session.subscribe( | ||
| // String.format(gameStartUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // // when | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), startGameRequest); | ||
| // | ||
| // // then | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // MiniGameStartMessage startResponse = startResponses.get().data(); | ||
| // softly.assertThat(startResponse.miniGameType()).isEqualTo(MiniGameType.CARD_GAME); | ||
| // | ||
| // MiniGameStateMessage loadingStateResponse = responses.get().data(); | ||
| // softly.assertThat(loadingStateResponse.cardGameState()).isEqualTo(CardGameState.LOADING.name()); | ||
| // softly.assertThat(loadingStateResponse.currentRound()).isEqualTo(CardGameRound.FIRST.name()); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 카드를_선택한다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<WebSocketResponse<MiniGameStateMessage>>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // | ||
| // MiniGameStateMessage loadingState = responses.get().data(); // 게임 로딩 state 응답 (LOADING) | ||
| // assertThat(loadingState.cardGameState()).isEqualTo(CardGameState.LOADING.name()); | ||
| // | ||
| // cardGameTaskExecutors.get(joinCode).join(CardGameTaskType.FIRST_ROUND_LOADING); | ||
| // | ||
| // MiniGameStateMessage playingState = responses.get().data(); // 게임 시작 state 응답 (PLAYING) | ||
| // assertThat(playingState.cardGameState()).isEqualTo(CardGameState.PLAYING.name()); | ||
| // | ||
| // String playerName = "꾹이"; | ||
| // int cardIndex = 0; | ||
| // | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand(playerName, cardIndex)) | ||
| // ); | ||
| // | ||
| // // when | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), request); | ||
| // | ||
| // // then | ||
| // MiniGameStateMessage result = responses.get().data(); | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(result.currentRound()).isEqualTo(CardGameRound.FIRST.name()); | ||
| // softly.assertThat(result.cardInfoMessages()).extracting(CardInfoMessage::playerName) | ||
| // .contains(playerName); | ||
| // softly.assertThat(result.allSelected()).isFalse(); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 전체_게임_플로우를_진행한다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // List<Player> players = roomRepository.findByJoinCode(joinCode).get().getPlayers(); | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // // when & then | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // | ||
| // // LOADING 상태 확인 | ||
| // MiniGameStateMessage loadingState = responses.get().data(); | ||
| // assertThat(loadingState.cardGameState()).isEqualTo(CardGameState.LOADING.name()); | ||
| // | ||
| // | ||
| // // PLAYING 상태 확인 | ||
| // MiniGameStateMessage playingState = responses.get().data(); | ||
| // | ||
| // cardGameTaskExecutors.get(joinCode).join(CardGameTaskType.FIRST_ROUND_LOADING); | ||
| // | ||
| // assertThat(playingState.cardGameState()).isEqualTo(CardGameState.PLAYING.name()); | ||
| // assertThat(playingState.currentRound()).isEqualTo(CardGameRound.FIRST.name()); | ||
| // | ||
| // // 모든 플레이어가 카드 선택 | ||
| // for (int i = 0; i < players.size(); i++) { | ||
| // Player player = players.get(i); | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand(player.getName().value(), i)) | ||
| // ); | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), request); | ||
| // responses.get(); | ||
| // } | ||
| // | ||
| // // 첫 번째 라운드 완료 후 SCORE_BOARD 상태로 변경 | ||
| // MiniGameStateMessage scoreBoardState = responses.get(10, TimeUnit.SECONDS).data(); | ||
| // assertThat(scoreBoardState.cardGameState()).isEqualTo(CardGameState.SCORE_BOARD.name()); | ||
| // | ||
| // // 두 번째 라운드 시작 | ||
| // MiniGameStateMessage secondLoadingState = responses.get().data(); | ||
| // assertThat(secondLoadingState.cardGameState()).isEqualTo(CardGameState.LOADING.name()); | ||
| // | ||
| // // 두 번째 라운드 시작 | ||
| // MiniGameStateMessage secondRoundState = responses.get().data(); | ||
| // assertThat(secondRoundState.cardGameState()).isEqualTo(CardGameState.PLAYING.name()); | ||
| // assertThat(secondRoundState.currentRound()).isEqualTo(CardGameRound.SECOND.name()); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 시간제한이_끝나면_라운드가_종료된다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // responses.get(); // LOADING | ||
| // responses.get(); // PLAYING | ||
| // | ||
| // // when | ||
| // | ||
| // // then | ||
| // MiniGameStateMessage result = responses.get(15, TimeUnit.SECONDS).data(); | ||
| // assertThat(result.allSelected()).isTrue(); // 단일 플레이어이므로 모든 선택 완료 | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 게임_상태_메시지에_카드_정보가_포함된다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // // when | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // responses.get(); // LOADING | ||
| // | ||
| // // then | ||
| // MiniGameStateMessage playingState = responses.get().data(); // PLAYING | ||
| // | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(playingState.cardInfoMessages()).isNotEmpty(); | ||
| // softly.assertThat(playingState.cardInfoMessages()).hasSize(9); // 6개 덧셈카드 + 3개 곱셈카드 | ||
| // softly.assertThat(playingState.cardInfoMessages()) | ||
| // .allMatch(cardInfo -> cardInfo.cardType().equals("ADDITION") || cardInfo.cardType() | ||
| // .equals("MULTIPLIER")); | ||
| // softly.assertThat(playingState.cardInfoMessages()) | ||
| // .allMatch(cardInfo -> !cardInfo.selected()); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 잘못된_카드_인덱스로_선택을_시도하면_예외가_발생한다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // responses.get(); // LOADING | ||
| // responses.get(); // PLAYING | ||
| // | ||
| // // when | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand("꾹이", 999)) // 잘못된 인덱스 | ||
| // ); | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), request); | ||
| // | ||
| // // then | ||
| // assertThatThrownBy(responses::get) | ||
| // .isInstanceOf(RuntimeException.class) | ||
| // .hasMessage("메시지 수신 대기 시간을 초과했습니다"); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 게임이_진행중이_아닐때_카드_선택을_시도하면_예외가_발생한다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // // 게임을 시작하지 않고 카드 선택 시도 | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand("꾹이", 0)) | ||
| // ); | ||
| // | ||
| // // when | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), request); | ||
| // | ||
| // // then | ||
| // assertThatThrownBy(responses::get) | ||
| // .isInstanceOf(RuntimeException.class) | ||
| // .hasMessage("메시지 수신 대기 시간을 초과했습니다"); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 카드_선택_후_상태_메시지에_선택된_카드_정보가_반영된다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses = session.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // sendStartGame(session, joinCode, host.getName().value()); | ||
| // responses.get(); // LOADING | ||
| // responses.get(); // PLAYING | ||
| // | ||
| // String playerName = "꾹이"; | ||
| // int cardIndex = 0; | ||
| // | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand(playerName, cardIndex)) | ||
| // ); | ||
| // | ||
| // // when | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), request); | ||
| // | ||
| // // then | ||
| // MiniGameStateMessage result = responses.get().data(); | ||
| // | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // // 선택된 카드가 있는지 확인 | ||
| // long selectedCount = result.cardInfoMessages().stream() | ||
| // .mapToLong(cardInfo -> cardInfo.selected() ? 1 : 0) | ||
| // .sum(); | ||
| // softly.assertThat(selectedCount).isEqualTo(1); | ||
| // | ||
| // // 선택된 카드의 플레이어 이름이 올바른지 확인 | ||
| // CardInfoMessage selectedCard = result.cardInfoMessages().stream() | ||
| // .filter(CardInfoMessage::selected) | ||
| // .findFirst() | ||
| // .orElse(null); | ||
| // softly.assertThat(selectedCard).isNotNull(); | ||
| // softly.assertThat(selectedCard.playerName()).isEqualTo(playerName); | ||
| // }); | ||
| // } | ||
| // | ||
| // @Test | ||
| // void 멀티플레이어_환경에서_각_플레이어가_다른_카드를_선택할_수_있다() throws ExecutionException, InterruptedException, TimeoutException { | ||
| // // given | ||
| // TestStompSession session1 = createSession(); | ||
| // TestStompSession session2 = createSession(); | ||
| // | ||
| // String subscribeUrlFormat = "/topic/room/%s/gameState"; | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // | ||
| // joinCode = new JoinCode("ABCDE"); | ||
| // Room room = new Room(joinCode, new PlayerName("플레이어1"), null); | ||
| // room.joinGuest(new PlayerName("플레이어2"), null); | ||
| // room.findPlayer(new PlayerName("플레이어2")).updateReadyState(true); | ||
| // room.addMiniGame(new PlayerName("플레이어1"), MiniGameType.CARD_GAME.createMiniGame()); | ||
| // roomRepository.save(room); | ||
| // | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses1 = session1.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // MessageCollector<WebSocketResponse<MiniGameStateMessage>> responses2 = session2.subscribe( | ||
| // String.format(subscribeUrlFormat, joinCode.value()), | ||
| // new TypeReference<>() { | ||
| // } | ||
| // ); | ||
| // | ||
| // sendStartGame(session1, joinCode, room.getHost().getName().value()); | ||
| // | ||
| // responses1.get(); | ||
| // responses2.get(); | ||
| // | ||
| // responses1.get(); | ||
| // responses2.get(); | ||
| // | ||
| // // when | ||
| // String[] playerNames = {"플레이어1", "플레이어2"}; | ||
| // int[] cardIndices = {0, 1}; | ||
| // TestStompSession[] sessions = {session1, session2}; | ||
| // MessageCollector[] responses = {responses1, responses2}; | ||
| // | ||
| // for (int i = 0; i < playerNames.length; i++) { | ||
| // MiniGameMessage request = new MiniGameMessage( | ||
| // CommandType.SELECT_CARD, | ||
| // objectMapper.valueToTree(new SelectCardCommand(playerNames[i], cardIndices[i])) | ||
| // ); | ||
| // sessions[i].send(String.format(requestUrlFormat, this.joinCode.value()), request); | ||
| // responses[i].get(); | ||
| // } | ||
| // | ||
| // // then | ||
| // MiniGameStateMessage finalState = responses1.get().data(); | ||
| // | ||
| // SoftAssertions.assertSoftly(softly -> { | ||
| // softly.assertThat(finalState.allSelected()).isTrue(); | ||
| // | ||
| // long selectedCount = finalState.cardInfoMessages().stream() | ||
| // .mapToLong(cardInfo -> cardInfo.selected() ? 1 : 0) | ||
| // .sum(); | ||
| // softly.assertThat(selectedCount).isEqualTo(2); | ||
| // }); | ||
| // } | ||
| // | ||
| // private void sendStartGame(TestStompSession session, JoinCode joinCode, String hostName) { | ||
| // String requestUrlFormat = "/app/room/%s/minigame/command"; | ||
| // MiniGameMessage startGameRequest = new MiniGameMessage( | ||
| // CommandType.START_MINI_GAME, | ||
| // objectMapper.valueToTree(new StartMiniGameCommand(hostName)) | ||
| // ); | ||
| // session.send(String.format(requestUrlFormat, joinCode.value()), startGameRequest); | ||
| // } | ||
| //} |
There was a problem hiding this comment.
Critical: Replace commented integration tests with new system equivalents.
Commenting out the entire integration test suite creates a significant testing gap for the new round-phase system. Integration tests are crucial for ensuring the refactored card game works correctly end-to-end.
The commented tests covered important scenarios that must be verified in the new system:
- Game start and phase transitions
- Card selection and player interactions
- Full game flow across multiple rounds
- Timeout and error handling
- Multiplayer scenarios
Please rewrite these integration tests to work with the new RoundPhase, RoomRoundManager, and RoundManagerRegistry system rather than simply commenting them out.
-//package coffeeshout.minigame.ui;
+package coffeeshout.minigame.ui;
-//
-//import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
// ... (rewrite imports and tests for new system)Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/src/test/java/coffeeshout/minigame/ui/CardGameIntegrationTest.java
lines 1 to 451, the entire integration test suite is commented out, removing
critical end-to-end tests for the card game. To fix this, rewrite the
integration tests to use the new round-phase system by incorporating the
RoundPhase, RoomRoundManager, and RoundManagerRegistry components. Ensure the
tests cover game start, phase transitions, card selection, full game flow,
timeout and error handling, and multiplayer scenarios, adapting the existing
test logic to interact with the new system APIs and state management.
💡 Verification agent
🧩 Analysis chain
Critical: Complete loss of integration test coverage
The entire integration test suite for card game functionality has been commented out, eliminating all integration test coverage. While this aligns with the migration from the task-based system to the new round management architecture, completely removing tests without replacement is problematic.
The PR objectives mention refactoring to use a state pattern, but integration tests should be updated to work with the new system rather than being disabled entirely. Consider:
- Updating the existing tests to use the new
RoundPhaseenum andRoomRoundManagersystem - Creating new integration tests that verify the state pattern implementation
- Ensuring critical user journeys (game start, card selection, round progression) are still covered
Please verify if replacement integration tests exist or are planned:
🏁 Script executed:
#!/bin/bash
# Check for any new or updated integration test files for card game
fd -e java -p ".*[Ii]ntegration.*[Tt]est.*" backend/src/test | grep -i card
fd -e java -p ".*[Tt]est.*" backend/src/test | xargs rg -l "RoundPhase|RoomRoundManager" | head -10Length of output: 586
Critical: Integration tests for Card Game UI have been disabled without replacement
All end-to-end coverage for the card game UI was removed when CardGameIntegrationTest.java was fully commented out, and no new UI integration tests were added to validate the migration to the state-pattern (RoundPhase/RoomRoundManager).
Locations requiring attention:
backend/src/test/java/coffeeshout/minigame/ui/CardGameIntegrationTest.java– currently entirely commented out- No other UI integration tests referencing
RoundPhaseorRoomRoundManagerunderbackend/src/test/java/coffeeshout/minigame/ui
Next steps:
- Restore and update the integration tests to use the new state-pattern APIs (e.g.,
RoundPhase,RoomRoundManager) - Or add new UI integration tests covering all critical flows:
• Game start & round initialization
• Card selection (valid, invalid indices, out-of-phase)
• Round transitions (time-out, multi-player selection)
• Final scoring and state messages
Ensuring these tests are in place is essential before merging this refactor.
🤖 Prompt for AI Agents
In backend/src/test/java/coffeeshout/minigame/ui/CardGameIntegrationTest.java
lines 1 to 451, the entire integration test class is commented out, removing all
end-to-end UI test coverage for the card game after migrating to the
state-pattern with RoundPhase and RoomRoundManager. To fix this, restore the
integration test class and update the tests to interact with the new
state-pattern APIs, ensuring coverage of key flows such as game start, card
selection (including invalid and out-of-phase cases), round transitions, and
final scoring. Alternatively, create new UI integration tests covering these
critical scenarios before merging the refactor.
1 participant
🔥 연관 이슈
🚀 작업 내용
💬 리뷰 중점사항
중점사항
이전에 문제라고 판단되었던 부분입니다. 해당 부분을 참고하고 pr 코멘트를 중점으로 답변해주시면 될 거 같습니다.
https://www.notion.so/7iwook/24720b26527780e29789f476cd10ef47?source=copy_link
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Tests
Documentation