Skip to content
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

Added unit test for SyncStatusFragment #2572

Conversation

anandwana001
Copy link
Collaborator

@anandwana001 anandwana001 commented Jul 24, 2024

Fixes #2571

Part 1 PR for this screen

@shobhitagarwal1612 PTAL?

Screenshot 2024-07-25 at 3 17 09 PM

@anandwana001 anandwana001 marked this pull request as ready for review July 25, 2024 09:47
@anandwana001 anandwana001 marked this pull request as draft July 26, 2024 04:01
@anandwana001 anandwana001 marked this pull request as ready for review July 30, 2024 11:19
@shobhitagarwal1612
Copy link
Member

/gcbrun

@shobhitagarwal1612
Copy link
Member

/gcbrun

Comment on lines 72 to 74
?: run {
return
}
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this?

Comment on lines 52 to 54
/**
* Confirming the Toolbar visibility.
*/
Copy link
Member

Choose a reason for hiding this comment

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

The test name is pretty obvious. We don't need extra comments here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for all or just this one?

* Confirming the Toolbar visibility.
*/
@Test
fun `MaterialToolbar should be Displayed`() {
Copy link
Member

Choose a reason for hiding this comment

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

Can we change it to Toolbar should be displayed?

* Confirming the SyncItem List visibility.
*/
@Test
fun `SyncItem List should Displayed`() = runWithTestDispatcher {
Copy link
Member

Choose a reason for hiding this comment

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

change to "Sync items should be displayed"

* Without Data, the list will not be visible on the screen.
*/
@Test
fun `SyncItem List should Not Displayed`() {
Copy link
Member

Choose a reason for hiding this comment

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

Same as previous comment.

surveyRepository.selectedSurveyId = SURVEY.id
advanceUntilIdle()

composeTestRule.onNodeWithTag("sync list").assertIsDisplayed()
Copy link
Member

Choose a reason for hiding this comment

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

Can we also verify the total count of items shown?

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, this needs to be done in next part of this pr, as this required a bit of extra work

@shobhitagarwal1612
Copy link
Member

/gcbrun

@shobhitagarwal1612 shobhitagarwal1612 merged commit b6c558e into google:master Aug 7, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Code health] Add Unit Test for SyncStatusFragment
2 participants