Closed Bug 1087038 Opened 10 years ago Closed 10 years ago

Gaia is closed, perma failures after last m-c -> b-i merge

Categories

(Remote Protocol :: Marionette, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: kgrandon, Assigned: timdream)

References

Details

(Keywords: regression)

Attachments

(1 file, 2 obsolete files)

We need bug 960072 so this doesn't happen to us in the future.
See Also: → 960072
Possibly caused by bug 1084412.
Blocks: 1084412
It looks like bug 1084412 changes findElement which we use under the hood in waitForElement: https://github.com/mozilla-b2g/marionette-helper/blob/b205ef7041ee7148e6aa2200667b052911178f49/index.js#L135
I can reproduce this error locally after updating B2G Desktop:

https://treeherder.mozilla.org/ui/logviewer.html#?job_id=676527&repo=b2g-inbound

There seems to be a legit regression on the marionette server side -- I can't bypass the error in anyway in waitForElement on the client side, and when the error throws I can clearly see the element is on screen.

I am building B2G Desktop to see if I can debug Gecko (instead of the client).
I can confirm that backing out bug 1084412 can make the test passes locally, however I am now suspect that patch is legit and it simply correct the false positive of our tests somehow.

Going to insert more throw into Gecko and see what's wrong.
The cause of the failure on comment 4 (and comment 4 along) is indeed due to logic error in bug 1084412. The check in https://hg.mozilla.org/mozilla-central/rev/56e2d9aff637#l2.13 will fail for HTMLFormElement because it comes with a length property === 0. I have a patch ready.
Assignee: nobody → timdream
Status: NEW → ASSIGNED
Attached patch Patch v1.0 (obsolete) — Splinter Review
Attachment #8509249 - Flags: review?(dburns)
Attachment #8509249 - Flags: feedback?(ahalberstadt)
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=1efd6b1b78d5

This is definitely not the complete set involving this patch, but I don't know what to run since bug 1084412 didn't come with a try run...
Component: Gaia → Marionette
Product: Firefox OS → Testing
Attachment #8509249 - Flags: review?(dburns) → review+
Attached patch Patch for commit (obsolete) — Splinter Review
Attachment #8509249 - Attachment is obsolete: true
Attachment #8509249 - Flags: feedback?(ahalberstadt)
(In reply to Carsten Book [:Tomcat] from comment #10)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/04a18f29f774

This needs to be backed out ... there is a |throw| for debug that I didn't remove :-/
Flags: needinfo?(cbook)
Keywords: checkin-needed
Attachment #8509375 - Attachment is obsolete: true
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #11)
> (In reply to Carsten Book [:Tomcat] from comment #10)
> > https://hg.mozilla.org/integration/mozilla-inbound/rev/04a18f29f774
> 
> This needs to be backed out ... there is a |throw| for debug that I didn't
> remove :-/

this got backouted in remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/4243e118f58d
Flags: needinfo?(cbook)
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #12)
> Created attachment 8509427 [details] [diff] [review]
> Real patch for commit

landed
remote:   https://hg.mozilla.org/integration/mozilla-inbound/rev/2b1c2663f92a
Here is another try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8a0b20c634a1
Hopefully we'll get the test results this time.
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #15)
> https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-
> inbound&revision=2b1c2663f92a

If this is the real landing then we are still in trouble here. Looks like we still have quite a few Gij failures.
It seems we are *still* getting failures here. Since the tree is closed, I'm going to recommend that we backout this patch and the patches in bug 1084412. I hate that we have to do this, but FirefoxOS has many deadlines, and currently hundreds of people are blocked from landing patches.

Tim/Andrew/David - Is this approach ok with you? If we are ok backing out, we can definitely find gaia people to help update our tests/libraries where necessary.
Flags: needinfo?(timdream)
Flags: needinfo?(dburns)
Flags: needinfo?(ahalberstadt)
Sure, let's back it out.
Flags: needinfo?(ahalberstadt)
I backed it out here:
https://hg.mozilla.org/mozilla-central/rev/305d24174ace

I'd ask a sheriff for help getting this to propagate to b-i
Flags: needinfo?(timdream)
Flags: needinfo?(dburns)
(In reply to Andrew Halberstadt [:ahal] from comment #21)
> I backed it out here:
> https://hg.mozilla.org/mozilla-central/rev/305d24174ace
> 
> I'd ask a sheriff for help getting this to propagate to b-i

Thanks for the understanding Andrew. Apologies for this - will make getting our tree un-hidden a top priority. Do we need to backout the patch in this bug as well? https://hg.mozilla.org/integration/mozilla-inbound/rev/2b1c2663f92a
Yes please Kevin. Maybe it's been backed out with the other back out though (because it was a fix for the previous patch) ?
Nope, ahal's backout was directly to m-c where comment 15 hadn't been merged to yet. I've backed it out from inbound now and will merge everything around when things are green enough for me to do so.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0f76f410b03b
The next issue to look at is another remaining Gij failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=50810640&tree=Mozilla-Central

I think the changelog that introduces this is the very next merge: https://tbpl.mozilla.org/?showall=1&rev=367d8d88c2cb
It's hard to say due to the other failures, but I suspect that the fullscreen issue was introduced in this changeset:

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&showall=1&jobname=b2g_ubuntu64_vm%20mozilla-inbound%20opt%20test%20gaia-js-integration&rev=2cc8826abef2

Most likely culprit: bug 1082652
Blocks: 1082652
Flags: needinfo?(etienne)
https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=6066a2a0766f

There are still failures after both bug 1084412 and this bug reverted from m-i.

I have just hit retrigger on Gij-3 here ...
Depends on: 1087831
We reaches green after backing out bug 1082652. Let's call this fixed.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(etienne)
Resolution: --- → FIXED
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) (please ni?) from comment #28)
> We reaches green after backing out bug 1082652. Let's call this fixed.

https://tbpl.mozilla.org/?tree=B2g-Inbound&jobname=b2g_ubuntu64_vm%20b2g-inbound%20opt%20test%20gaia-js-integration&rev=fc8e45f09f78
See Also: → 1091367
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: