-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
So in this PR, I refactored authentication and user profile tests to use mock data. for this, I set up Hilt an automatic dependency injector and mockk a mocking library for Kotlin. The test coverage is a bit low because we're now using mock data instead of connecting to the actual 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.
Once you remove the unnecessary commented code, will approve. Great work otherwise!
Deleted unnecessary code |
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.
Good cleanup and good work!
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.
👍 nice improvement for the codebase
Code Climate has analyzed commit 551cfad and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 67.7% (80% is the threshold). This pull request will bring the total coverage in the repository to 92.4%. View more on Code Climate. |
|
||
|
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.
It will be great if you can add some documentation to this class and its non-trivial functions
.child(name) | ||
.setValue(input.text.toString()) | ||
}) | ||
usersRepo.updateField("-Myfy9TlCUTWYRxVLBsQ", input.text.toString() ,name) |
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.
usersRepo.updateField("-Myfy9TlCUTWYRxVLBsQ", input.text.toString() ,name) | |
usersRepo.updateField("-Myfy9TlCUTWYRxVLBsQ", input.text.toString(), name) | |
@@ -29,11 +44,42 @@ import kotlin.random.Random | |||
* See [testing documentation](http://d.android.com/tools/testing). | |||
*/ | |||
@RunWith(AndroidJUnit4::class) | |||
@HiltAndroidTest | |||
class AuthenticationActivityTest { | |||
private val sleepTime: Long = 5000 |
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.
sleepTime can be deleted if it's no longer used
import org.junit.Test | ||
import org.junit.runner.RunWith | ||
|
||
|
||
@RunWith(AndroidJUnit4::class) | ||
@HiltAndroidTest | ||
class ProfileActivityTest { | ||
|
||
private val sleepTime: Long = 4000 |
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.
same for this
class ProfileActivityTest { | ||
|
||
private val sleepTime: Long = 4000 | ||
|
||
@get:Rule(order = 0) |
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.
Do we still need the order = 0 if it's the only rule?
No description provided.