Jwen/lyric save score #258
Conversation
|
Code Climate has analyzed commit be7c980 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 66.6% (80% is the threshold). This pull request will bring the total coverage in the repository to 82.0% (-0.2% change). View more on Code Climate. |
| super.setScores(gameManager) | ||
| } | ||
| } | ||
|
|
||
| /** helper functions to test private functions */ |
There was a problem hiding this comment.
| /** helper functions to test private functions */ | |
| /* helper functions to test private functions */ | |
It is a bit weird how the comment is done for the moment.
Either do a comment that describe that below functions are helper
or do a javadoc for each.
But a javadoc that does multiple function at a time does not seems following the conventions.
There was a problem hiding this comment.
I didn't write this comment nor this part, I just added one * more so it shows green...
I will leave this to the people who do refactor to change.
| } | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
| /** | |
| /* |
same as above, Javadoc is for a single function
There was a problem hiding this comment.
Maybe adding a javadoc for each could be great as their part of public interface, even if they are only for testing purpose.
There was a problem hiding this comment.
this is a nice comment, but not the focus of my PR (as I didn't use this part of the code). I think leaving the task to the people who do refactoring will be better.
There was a problem hiding this comment.
okay will do myself the change so. We will forget it otherwise
| @@ -285,6 +313,7 @@ class LyricsBelongGameActivityTest { | |||
| currentArtist = activity.getArtistName() | |||
| currentSong = activity.getSongName() | |||
| } | |||
|
|
|||
| onView(withId(R.id.lyricMatchButton)).check(matches(not(isDisplayed()))) | |||
| assertEquals(artistName, currentArtist) | |||
| assertEquals("Monday", currentSong) | |||
There was a problem hiding this comment.
(Whole file) Great job on adding mocks and removing Thread.sleep instances 👍
coverage is low due to firebase