1400101 - Permafailing testAppMenuPathways | Waiting for menu to open. since the upilft of Gecko 57 to Beta

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: rvandermeulen [at] mozilla.com

https://treeherder.mozilla.org/logviewer.html#?job_id=131174413&repo=mozilla-beta

https://queue.taskcluster.net/v1/task/DJDuV5_lQZCskV6PIQ0Zrg/runs/0/artifacts/public/logs/live_backing.log

Comment 1

[Ryan VanderMeulen [:RyanVM]]

[Tracking Requested - why for this release]: Permafailing test since the uplift.

Nevin, can you please take a look at this or redirect? It's permafailing at the moment :(

Comment 2

[Nevin Chen(Not active on Bugzilla)]

Hi Jing Wei
Are you working on this?

Comment 3

[Nevin Chen(Not active on Bugzilla)]

Please also help with bug 1383713

Comment 4

[Jing-wei Wu [:jwu]]

I think this issue has same root cause as bug 1398044. 

In `AppMenuComponent.java`, Robocop finds menu button directly from the root hierarchy [1], since the context menu in Activity Stream also uses R.id.menu as its ID, sometimes Robocop is confused and accesses wrong UI component.

A simple fix is instead of directly accessing menu button, we can get BrowserToolbar first then use it to find the correct menu button.

[1] https://dxr.mozilla.org/mozilla-central/source/mobile/android/tests/browser/robocop/src/org/mozilla/gecko/tests/components/AppMenuComponent.java#120

Comment 5

[Jing-wei Wu [:jwu]]

Attachment: Bug 1400101 - [robocop] Find toolbar first and use it to find menu button.

Sometimes Robocop is confused when are multiple `R.id.menu` UI components shown on screen,
one is the menu button on toolbar and otehrs are context menus on Activity Stream.

To access the menu on toolbar, a simple fix is access toolbar first and use it to find its child.

Review commit: https://reviewboard.mozilla.org/r/180172/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/180172/

Comment 6

[Nevin Chen(Not active on Bugzilla)]

Comment on attachment 8908508 [details]
Bug 1400101 - [robocop] Find toolbar first and use it to find menu button.

https://reviewboard.mozilla.org/r/180172/#review185352

r+ for the code. Do we also need to  rename the resource id used by Activity Stream? If it's urgent we can land it as is.

Comment 7

[Jan Henning [:JanH]]

Comment on attachment 8908508 [details]
Bug 1400101 - [robocop] Find toolbar first and use it to find menu button.

https://reviewboard.mozilla.org/r/180172/#review185364

If it works, I guess it's fine.
Do you want this for uplift only, or on trunk as well? For the latter, I'd still think we should sort out the IDs to be unique (bug 1398044).

Comment 8

[Jing-wei Wu [:jwu]]

(In reply to Jan Henning [:JanH] from comment #7)
> Comment on attachment 8908508 [details]
> Bug 1400101 - [robocop] Find toolbar first and use it to find menu button.
> 
> https://reviewboard.mozilla.org/r/180172/#review185364
> 
> If it works, I guess it's fine.
> Do you want this for uplift only, or on trunk as well? For the latter, I'd
> still think we should sort out the IDs to be unique (bug 1398044).

I prefer to keep it on trunk as well because it keeps our Robocop test a little more robust no matter we have unique ID or not.

Comment 9

[Jan Henning [:JanH]]

Fine with me.

Comment 11

[Jing-wei Wu [:jwu]]

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e9450c83f3b6&selectedJob=131261614

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08d09442ab4b
[robocop] Find toolbar first and use it to find menu button. r=JanH,nechen

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Thanks for the analysis and patch, jwu! One thing I'm not understanding at this point is how the Try simulations I was running prior the uplift didn't catch this issue (I had green robocop runs on the same rev that was later merged to Beta). I was wondering if you could maybe point me at the code that enabled Activity Stream on Fennec so I can maybe try to figure out if the uplift simulation patches need revisions for the future.

Comment 14

[Jing-wei Wu [:jwu]]

Hi Ryan,

I think Activity Stream is already forcibly enabled in Beta in bug 1384021. Actually we event don't allow user to disable it. :)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/08d09442ab4b