Marionette doesn't clean up its socket following a restart initiated inside the application

RESOLVED FIXED in Firefox 38

Status

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: chmanchester, Assigned: chmanchester)

Tracking

(Blocks 1 bug)

unspecified
mozilla39
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox38 fixed, firefox39 fixed)

Details

(Whiteboard: [qa-automation-blocked])

Attachments

(1 attachment, 1 obsolete attachment)

/r/5073 - Bug 1141679 - Ignore the server's response when quitting the application with marionette.

Pull down this commit:

hg pull review -r b1dce20b90468610467e9b3198eb45ba968a9baa
https://reviewboard.mozilla.org/r/5071/#review4093

::: testing/marionette/client/marionette/tests/unit/test_profile_management.py
(Diff revision 1)
> -        self.assertTrue(bool_value)
> +        self.assertRaisesRegexp(JavascriptException, "Error getting pref",

I'm not 100% sure why this changes with the patch, but this isn't really a desired behavior in the first place, so whether the user pref's value isn't saved or it's not recognized at all may not matter.
Assignee

Updated

4 years ago
Attachment #8575527 - Flags: review?(jgriffin)
Comment on attachment 8575527 [details]
MozReview Request: bz://1141679/chmanchester

/r/5073 - Bug 1141679 - Ignore the server's response when quitting the application with marionette.

Pull down this commit:

hg pull review -r b1dce20b90468610467e9b3198eb45ba968a9baa
https://reviewboard.mozilla.org/r/5071/#review4099

> I'm not 100% sure why this changes with the patch, but this isn't really a desired behavior in the first place, so whether the user pref's value isn't saved or it's not recognized at all may not matter.

The new behavior seems correct.  I'm not sure how to explain the old behavior the test was verifiying.
Comment on attachment 8575527 [details]
MozReview Request: bz://1141679/chmanchester

https://reviewboard.mozilla.org/r/5071/#review4101

Ship It!
Attachment #8575527 - Flags: review?(jgriffin) → review+
(In reply to Chris Manchester [:chmanchester] from comment #4)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04

This is failing on osx in ways I can't explain.
(In reply to Chris Manchester [:chmanchester] from comment #7)
> (In reply to Chris Manchester [:chmanchester] from comment #4)
> > https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04
> 
> This is failing on osx in ways I can't explain.

Surely a bad build. Here it is again: https://treeherder.mozilla.org/#/jobs?repo=try&revision=73edbe5e41c2
Assignee

Updated

4 years ago
Keywords: checkin-needed
(In reply to Chris Manchester [:chmanchester] from comment #8)
> (In reply to Chris Manchester [:chmanchester] from comment #7)
> > (In reply to Chris Manchester [:chmanchester] from comment #4)
> > > https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04
> > 
> > This is failing on osx in ways I can't explain.
> 
> Surely a bad build. Here it is again:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=73edbe5e41c2

Here's the contents of gecko.log from the second bad build (same as the first): 

19:32:14     INFO -  dyld: Symbol not found: _strndup
19:32:14     INFO -    Referenced from: /builds/slave/talos-slave/test/build/application/NightlyDebug.app/Contents/MacOS/libmozglue.dylib
19:32:14     INFO -    Expected in: flat namespace
19:32:14     INFO -   in /builds/slave/talos-slave/test/build/application/NightlyDebug.app/Contents/MacOS/libmozglue.dylib

I'm having a very hard time believing this patch caused that, but I guess I can't really ask for checkin without getting try to pass.
Keywords: checkin-needed
Thank you Chris for taking care of it that quickly. Lets hope we can figure out the remaining problem soon. Until then I will add our blocked whiteboard entry.
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
OS: Linux → All
Hardware: x86_64 → All
Whiteboard: [qa-automation-blocked]
FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
before I found this bug.  Feel free to dupe it if you will handle it here.
It would be nice if you could look into that soon, for obvious reasons :-)
(In reply to Mats Palmgren (:mats) from comment #11)
> FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
> before I found this bug.  Feel free to dupe it if you will handle it here.
> It would be nice if you could look into that soon, for obvious reasons :-)

I'm certainly going to try to figure out what's going on but that bug seems more on topic. This isn't my area of expertise so I really don't know how much I'm going to be able to offer on the subject.
Depends on: 1142006
(In reply to Chris Manchester [:chmanchester] from comment #12)
> (In reply to Mats Palmgren (:mats) from comment #11)
> > FYI, I filed bug 1142006 on the "dyld: Symbol not found: _strndup" problem
> > before I found this bug.  Feel free to dupe it if you will handle it here.
> > It would be nice if you could look into that soon, for obvious reasons :-)
> 
> I'm certainly going to try to figure out what's going on but that bug seems
> more on topic. This isn't my area of expertise so I really don't know how
> much I'm going to be able to offer on the subject.

I guess this is more likely bug 1141745. I'm just going to land this when inbound opens.
No longer depends on: 1142006
Assignee

Updated

4 years ago
Depends on: 1142344
Assignee

Updated

4 years ago
Depends on: 1142404
https://hg.mozilla.org/mozilla-central/rev/6039462e84b0
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
We kinda would like to have this support for Firefox 38 to be able to run our Firefox UI tests. Would it be possible to backport this patch?
Flags: needinfo?(dburns)
Flags: needinfo?(cmanchester)
The two dependent follow ups should be uplifted with this bug.
Flags: needinfo?(cmanchester)
Thanks Chris. I marked both as affected for 38.
Attachment #8575527 - Attachment is obsolete: true
Attachment #8619722 - Flags: review+
You need to log in before you can comment on or make changes to this bug.