Skip to content
This repository has been archived by the owner on Jan 31, 2024. It is now read-only.

createProfileActivity: mock Authenticator #269

Merged
merged 3 commits into from
May 9, 2022

Conversation

kamilababayeva
Copy link
Collaborator

@kamilababayeva kamilababayeva commented May 9, 2022

This PR mocks authenticator in the createProfileActivity.

@kamilababayeva kamilababayeva self-assigned this May 9, 2022
@MaximeZmt MaximeZmt self-requested a review May 9, 2022 09:39
@@ -69,9 +86,9 @@ class CreateProfileActivityTest {

val intent = Intent(ApplicationProvider.getApplicationContext(), CreateProfileActivity::class.java)
intent.putExtra("email", mockEmail)
intent.putExtra("isUnitTest", true)
Copy link
Owner

Choose a reason for hiding this comment

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

Nooooo bye my friend isUnitTest, you had a great life 😂😂😂😂 !!!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ahahahahahhahaa bye-bye and never come back please

Comment on lines +87 to +96
/**
* Getter for the current user ID
*/
fun getCurrUID(): String {
var uid = ""
if (isLoggedIn()) {
uid = FirebaseAuth.getInstance().currentUser!!.uid
}
return uid
}
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
/**
* Getter for the current user ID
*/
fun getCurrUID(): String {
var uid = ""
if (isLoggedIn()) {
uid = FirebaseAuth.getInstance().currentUser!!.uid
}
return uid
}

See 40 lines above !!!
getCurrentUID() exactly the same 😂

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes, but when you mock, you cant access one from the above

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will mock others as well, and remove those from companion object. sounds good?

Copy link
Owner

Choose a reason for hiding this comment

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

Ohh okay I see, yes maybe removing the companion object may be a good idea because I find it weird otherwise to have two times the same code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is... I decided to remove one by one, so it easier to check!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will remove these function on the next pr since there a lot of dependencies. My next pr (where these repetetive functions are removed) is almost done!

Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

Nice work! just left some comment above 😀

@codeclimate
Copy link

codeclimate bot commented May 9, 2022

Code Climate has analyzed commit 317c9fc and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 28.5% (80% is the threshold).

This pull request will bring the total coverage in the repository to 82.2% (0.1% change).

View more on Code Climate.

@jiabaow jiabaow self-requested a review May 9, 2022 10:59
Copy link
Owner

@MaximeZmt MaximeZmt left a comment

Choose a reason for hiding this comment

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

As you are doing the commented update in the new PR:
#272
Approved!!

Copy link
Collaborator

@jiabaow jiabaow left a comment

Choose a reason for hiding this comment

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

LGTM

@kamilababayeva kamilababayeva merged commit 5f75482 into main May 9, 2022
@kamilababayeva kamilababayeva deleted the kamila/mock_create_profile branch May 9, 2022 11:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants