Closed Bug 1400101 Opened 2 years ago Closed 2 years ago

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

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 + verified

People

(Reporter: intermittent-bug-filer, Assigned: jwu)

References

Details

(Whiteboard: [FNC][SPT57.3][INT])

Attachments

(1 file)

[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 :(
Flags: needinfo?(cnevinchen)
Hi Jing Wei
Are you working on this?
Flags: needinfo?(cnevinchen) → needinfo?(topwu.tw)
Please also help with bug 1383713
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
Flags: needinfo?(topwu.tw)
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.
Attachment #8908508 - Flags: review?(cnevinchen) → review+
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).
Attachment #8908508 - Flags: review?(jh+bugzilla) → review+
(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.
Fine with me.
Duplicate of this bug: 1398031
Pushed by topwu.tw@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/08d09442ab4b
[robocop] Find toolbar first and use it to find menu button. r=JanH,nechen
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.
Assignee: nobody → topwu.tw
Flags: needinfo?(topwu.tw)
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. :)
Flags: needinfo?(topwu.tw)
https://hg.mozilla.org/mozilla-central/rev/08d09442ab4b
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Status: RESOLVED → VERIFIED
Whiteboard: [FNC][SPT57.3][INT]
You need to log in before you can comment on or make changes to this bug.