Wordpress-android: Testing: Improve the logout action in UI tests

Created on 12 May 2020  路  7Comments  路  Source: wordpress-mobile/WordPress-Android

In our UI tests, logout started failing after https://github.com/wordpress-mobile/WordPress-Android/pull/11838 was merged. We did a quick fix to the UI tests in https://github.com/wordpress-mobile/WordPress-Android/pull/11850 to get logout to work so tests can pass. (See the discussion on that PR for more details about the matching failure and discussed fixes.)

We should revisit that quick fix and improve the logout action to make sure we're using withEffectiveVisibility correctly (and only where it's needed).

Testing [Type] Enhancement

Most helpful comment

I had a little more time to look at this, in particular:

I wasn鈥檛 sure if the issue was gone somehow or if I was just failing to correctly reproduce it

The test failures started after #11838 was merged, and I see some changes since then that very likely fixed the issue that was caused those failures. The logout issue was only with the tests themselves (not for end users) so let's go ahead and revert the changes made in #11850 since they are no longer needed for the tests to pass.

We can do that in an upcoming Groundskeeping rotation.

All 7 comments

Also, a note about debugging this from a Slack conversation with @oguzkocer:

I suggest commenting out the waitForElementToBeDisplayed parts because they are masking the errors. You'll get much better logs of what's happening. The tests were complaining about me_item matching to multiple views even though what it prints out only has one mention of it.

After taking a quick look at those tests, it seems to me that it would be better to update them all at once, after we finish the unified login project, as many views and flows are going to be affected by it anyway, so we would be avoiding some rework.

@rachelmcr Let me know if this makes sense to you.

Sure thing! Since the tests are working as-is, I don't think making these improvements is urgent. It makes sense to me to wait and do a more holistic update all at once.

FYI: I recently looked into this again while working on #12445 and I actually could not reproduce the logout issue you were seeing before, even after totally reverting the changes made by #11850, so I wasn鈥檛 sure if the issue was gone somehow or if I was just failing to correctly reproduce it, which made me unable to accurately verify if withEffectiveVisibility is still needed.

Thanks for the update @renanferrari! Unfortunately I don't have the bandwidth to follow up on this right now. I was able to reproduce the issue at the time if I remember correctly, but regardless, whatever I was able to figure out I documented in the PRs, so I'd look into those for info.

If you need any help with this, I'd be happy to assist once I am back from afk next sprint.

Thanks for the update @renanferrari! Unfortunately I don't have the bandwidth to follow up on this right now.

No problem. I talked to @rachelmcr via Slack before leaving the comment above and we agreed this is not urgent.

I was able to reproduce the issue at the time if I remember correctly, but regardless, whatever I was able to figure out I documented in the PRs, so I'd look into those for info.

11850 isn't linked to any issue and I couldn't find any steps to reproduce the logout problem (searched on GitHub and Slack).

If you need any help with this, I'd be happy to assist once I am back from afk next sprint.

Again, this is not urgent, but pointing to any info that could help determine if the logout issue is gone would definitely help a lot whoever works on this going forward.

I had a little more time to look at this, in particular:

I wasn鈥檛 sure if the issue was gone somehow or if I was just failing to correctly reproduce it

The test failures started after #11838 was merged, and I see some changes since then that very likely fixed the issue that was caused those failures. The logout issue was only with the tests themselves (not for end users) so let's go ahead and revert the changes made in #11850 since they are no longer needed for the tests to pass.

We can do that in an upcoming Groundskeeping rotation.

Was this page helpful?
0 / 5 - 0 ratings