Skip to content

Conversation

@ArchILLtect
Copy link
Owner

This pull request introduces a new service class for week 9 challenges and improves testing and database reset logic. The most important changes are the addition of the Week9ChallengeService class and its corresponding tests, as well as refactoring the database reset method for better reliability. Minor test improvements and formatting updates are also included.

New service and test coverage:

  • Added Week9ChallengeService class to wrap core challenge retrieval methods, providing a specialized interface for week 9 challenge operations.
  • Added Week9ChallengeServiceTest to verify delegation of calls to the underlying ChallengeService.

Database reset improvements:

  • Refactored DaoTestBase.resetDatabase() to use H2's TRUNCATE TABLE ... RESTART IDENTITY and temporarily disable referential integrity for reliable test isolation.

Challenge service test enhancements:

  • Added new tests to ChallengeServiceTest for findAll and findById, increasing coverage of challenge retrieval logic.

Minor test formatting updates:

  • Updated quote formatting in QuoteServiceTest assertions to use standard Unicode quote characters for clarity and consistency. [1] [2] [3]

Copilot AI review requested due to automatic review settings November 4, 2025 22:26
@ArchILLtect ArchILLtect added area:service Business logic and orchestration (e.g., DrillService, ChallengeRunService). api:external Work related to external/public API selection, integration, and resilience. project:mvp Use for all issues/PRs that belong to the MVP release. Will auto-add labeled items to the board. labels Nov 4, 2025
@ArchILLtect ArchILLtect merged commit dbda6fd into main Nov 4, 2025
6 checks passed
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Week 9 functionality by introducing a new service wrapper class and corresponding tests, along with improvements to test infrastructure and code clarity.

  • Adds Week9ChallengeService wrapper class to provide simplified access to challenge operations
  • Improves test infrastructure by using TRUNCATE TABLE with identity restart for cleaner database resets
  • Replaces Unicode escape sequences with actual characters in test assertions for better readability

Reviewed Changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/main/java/me/nickhanson/codeforge/service/Week9ChallengeService.java New service wrapper providing findAll() and findById() methods that delegate to ChallengeService
src/test/java/me/nickhanson/codeforge/service/Week9ChallengeServiceTest.java Unit tests verifying delegation behavior of the new wrapper service
src/test/java/me/nickhanson/codeforge/service/ChallengeServiceTest.java Adds tests for listChallenges() and getById() methods; removes unused import
src/test/java/me/nickhanson/codeforge/service/QuoteServiceTest.java Replaces Unicode escape sequences with actual quotation marks in assertions
src/test/java/me/nickhanson/codeforge/persistence/DaoTestBase.java Improves database reset using TRUNCATE TABLE with RESTART IDENTITY instead of DELETE
docs/screenshots/week-9-getById.png Screenshot documentation for Week 9 getById functionality
docs/screenshots/week-9-getAll.png Screenshot documentation for Week 9 getAll functionality
docs/projects/pd-presentation/screenshots/annotation-diagram.png Presentation diagram image

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +46 to +48
assertTrue(svc.getById(42L).isPresent());
assertEquals("Two Sum", svc.getById(42L).get().getTitle());
verify(dao, times(2)).getById(42L); // or call once then store the Optional
Copy link

Copilot AI Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test calls svc.getById(42L) twice, which results in two unnecessary DAO calls. Store the result in a variable to avoid redundant method invocations. The comment acknowledges this but the code should be fixed.

Suggested change
assertTrue(svc.getById(42L).isPresent());
assertEquals("Two Sum", svc.getById(42L).get().getTitle());
verify(dao, times(2)).getById(42L); // or call once then store the Optional
var result = svc.getById(42L);
assertTrue(result.isPresent());
assertEquals("Two Sum", result.get().getTitle());
verify(dao).getById(42L);

Copilot uses AI. Check for mistakes.
@ArchILLtect ArchILLtect linked an issue Nov 4, 2025 that may be closed by this pull request
@ArchILLtect ArchILLtect deleted the feat/week-9-exercise branch November 15, 2025 05:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api:external Work related to external/public API selection, integration, and resilience. area:service Business logic and orchestration (e.g., DrillService, ChallengeRunService). project:mvp Use for all issues/PRs that belong to the MVP release. Will auto-add labeled items to the board.

Projects

Development

Successfully merging this pull request may close these issues.

Week 9 Exercise Ready for Review

2 participants