-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test Coverage #8
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just an advice for upcoming task, using for loop is good, but using lambda functions make your code more readable. Ex: forEach()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for PR. Few observations and suggestions.
@@ -100,7 +100,7 @@ public List<StudiosDTO> getAllStudiosByVisibility(boolean visible) { | |||
public List<StudiosDTO> getAllStudiosByCommunity(int community) { | |||
List<StudiosDTO> studiosDto = new ArrayList<>(); | |||
if (community == 1) { | |||
studioRepository.findByCommunity(community).forEach(studio -> studiosDto.add(getStudioMapper().studiosToStudiosDto(studio))); | |||
studioRepository.findByIsCommunity(community).forEach(studio -> studiosDto.add(getStudioMapper().studiosToStudiosDto(studio))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Observational: Returning the list within the if block can remove the need for else block.
@@ -0,0 +1,4 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this enum required?
|
||
|
||
@DataJpaTest(properties="spring.cloud.config.enabled=false") | ||
@TestPropertySource("classpath:test.properties") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use application.properties in the test/resources. The idea is to follow similar pattern across the projects.
|
||
@DataJpaTest(properties="spring.cloud.config.enabled=false") | ||
@TestPropertySource("classpath:test.properties") | ||
@EntityScan("com.gamedoora.model.*") // Scanning the model(s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this to the Application class.
@@ -0,0 +1,15 @@ | |||
# H2 Database |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add these properties to application properties in test/resources
Fixes # Test Coverage for Studio Repository #6
Changes
Remaining