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.

Sign up for GitHub

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

Jump to bottom

Tests are failing on Espresso in AddProfileActivityTest #3363

Open
rt4914 opened this issue Jun 23, 2021 · 7 comments
Open

Tests are failing on Espresso in AddProfileActivityTest #3363

rt4914 opened this issue Jun 23, 2021 · 7 comments
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@rt4914
Copy link
Contributor

rt4914 commented Jun 23, 2021 •
edited by adhiamboperes
Loading

Tests are failing on Espresso in AddProfileActivityTest

Screenshot 2021-06-23 at 4 38 02 PM

testAddProfileActivity_inputPin_configChange_downloadAccessSwitchIsOn() was removed in #5229. The behaviour was deemed not important to the UX.
The test failed when testing the checkbox should preserve its state after the screen rotated, which native checkbox doesn't have this ability and we agree that we don't need extra implementation to achieve this.

testAddProfileActivity_configChange_inputShortPin_create_pinLengthError() was improved in #5229, but it is still flaky. @XichengSpencer left some comments on this issue thread of his attempts to fix it, as well as a link to an external gist.

Developers should run the third test multiple times to deterrmine if it is still flaky.
@rt4914 rt4914 added Type: Improvement Priority: Essential This work item must be completed for its milestone. labels Jun 23, 2021
@rt4914 rt4914 added this to the Beta milestone Jun 23, 2021
This was referenced Jun 23, 2021
Merged
Closed
@FareesHussain
Copy link
Contributor

FareesHussain commented Jul 8, 2021 •
edited
Loading

@rt4914 It looks like a espresso scrolling issue,
I think we need to introduce nestedScrollTo here.

On testing it manually, it was working fine
so I tried to use scrollTo() to the above view (to the targeted view) and it worked.

In case the nestedScrollTo() mentioned didn't work we can perform scrollTo() to the view above the targeted view.

@rt4914
Copy link
Contributor Author

rt4914 commented Jul 8, 2021

Tested this again on Pixel 3 XL and only testAddProfileActivity_configChange_inputShortPin_create_pinLengthError is failing now.

@rt4914 rt4914 mentioned this issue Jul 9, 2021
Merged
8 tasks
@Broppia Broppia added issue_type_bug Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Jul 29, 2022
@BenHenning BenHenning added Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer and removed Impact: Medium Moderate perceived user impact (non-blocking bugs and general improvements). labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta milestone Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_user_developer labels Mar 29, 2023
@adhiamboperes adhiamboperes added Work: Low Solution is clear and broken into good-first-issue-sized chunks. and removed Priority: Essential This work item must be completed for its milestone. labels Sep 22, 2023
@XichengSpencer
Copy link
Collaborator

XichengSpencer commented Sep 22, 2023 •
edited
Loading

Try to repro the failing test result without emulator. They all ran 3 times and had identical results.
Screenshot 2023-09-22 172511
Screenshot 2023-09-22 171358
Screenshot 2023-09-22 172356

Gist for failed test message: https://gist.github.com/XichengSpencer/c6f7b537ef783afc0ac56166f87ad1a4

@XichengSpencer
Copy link
Collaborator

Two emulator tests using api 29 pixel 3

Image

Image

@XichengSpencer
Copy link
Collaborator

After going through all the tests appear above one by one, I found two tests that were consistently failing while others passed. They are:

fun testAddProfileActivity_inputPin_configChange_downloadAccessSwitchIsOn()
This method failed at
onView(withId(R.id.add_profile_activity_allow_download_switch)).perform(scrollTo())

where it scroll to a switch component. But as the screenshot shows, even our XML preview on the left has the switch component, when the app is running on the right the switch is gone.

Image

fun testAddProfileActivity_clickInfo_configChange_infoPopupIsDisplayed()

oppia-android.AddProfileActivityTest.kt.oppia-android.app.2023-10-17.12-04-57.mp4

This one tests the pop-up dialog will stay after the screens rotate, which I checked the code and didn't see such implementation.
I tested on Pixel 3 API 29 and Pixel 4 API 30, both dialogs will go when the screen rotates so there is nothing wrong with the test, it just we don't have such implementation.

@XichengSpencer XichengSpencer mentioned this issue Nov 15, 2023
Merged
6 tasks
@XichengSpencer
Copy link
Collaborator

Here is the attempted code and discussion to fix testAddProfileActivity_configChange_inputShortPin_create_pinLengthError(). It fails to click the button to make the error message on the textinputlayout appears.

@XichengSpencer
Copy link
Collaborator

This is the a screen record of test fail

oppia-android.AddProfileActivityTest.kt.oppia-android.app.2023-11-15.18-46-22.mp4

@adhiamboperes adhiamboperes added good first issue This item is good for new contributors to make their pull request. and removed Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. labels Dec 7, 2023
adhiamboperes added a commit that referenced this issue Dec 7, 2023
@XichengSpencer @adhiamboperes
Fix part of #3363 AddProfileActivityTest some test didn't pass. (#5229)
c931ba4 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix part of #3363 
Remove
testAddProfileActivity_inputPin_configChange_downloadAccessSwitchIsOn()
as discussed with Ben and Adhiambo.

- The old test failed when testing the checkbox should persevere its
state after the screen rotated, which native checkbox doesn't have this
ability and we agree that we don't need extra implementation to achieve
this.

Negate download access logic in AddProfileActivityPresenter so it will
correctly set the listener.

Improve
testAddProfileActivity_configChange_inputShortPin_create_pinLengthError()
by adding the swipeup() action to make the create button fully visible
during the test but the test is still flaky

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
![Screenshot 2023-11-15
160038](https://github.com/oppia/oppia-android/assets/74568012/54670e41-9a41-4368-a9cb-0b47b83abe8b)

---------

Co-authored-by: Adhiambo Peres <[email protected]>
@adhiamboperes adhiamboperes removed the good first issue This item is good for new contributors to make their pull request. label Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Work: Low Solution is clear and broken into good-first-issue-sized chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Status: Todo
Milestone
No milestone
Development

No branches or pull requests
7 participants
@rt4914 @seanlip @BenHenning @FareesHussain @adhiamboperes @XichengSpencer @Broppia