If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in Firefox 38

Status

Testing
Marionette
RESOLVED FIXED
3 years ago
2 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])

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Henrik describes the issue here: https://bugzilla.mozilla.org/show_bug.cgi?id=1130220#c4
(Assignee)

Comment 1

3 years ago
Created 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
(Assignee)

Comment 2

3 years ago
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

3 years ago
Attachment #8575527 - Flags: review?(jgriffin)
(Assignee)

Comment 3

3 years ago
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
(Assignee)

Comment 4

3 years ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=00b0f128ed04
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+
(Assignee)

Comment 7

3 years ago
(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.
(Assignee)

Comment 8

3 years ago
(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

3 years ago
Keywords: checkin-needed
(Assignee)

Comment 9

3 years ago
(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 :-)
(Assignee)

Comment 12

3 years ago
(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
(Assignee)

Comment 13

3 years ago
(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)

Comment 14

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/6039462e84b0
(Assignee)

Updated

3 years ago
Depends on: 1142344
(Assignee)

Updated

3 years ago
Depends on: 1142404
https://hg.mozilla.org/mozilla-central/rev/6039462e84b0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox39: --- → fixed
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?
status-firefox38: --- → affected
Flags: needinfo?(dburns)
Flags: needinfo?(cmanchester)
(Assignee)

Comment 17

3 years ago
The two dependent follow ups should be uplifted with this bug.
Flags: needinfo?(cmanchester)
Thanks Chris. I marked both as affected for 38.
https://hg.mozilla.org/releases/mozilla-aurora/rev/75069e9797d9
Flags: needinfo?(dburns)
status-firefox38: affected → fixed
(Assignee)

Comment 20

2 years ago
Comment on attachment 8575527 [details]
MozReview Request: bz://1141679/chmanchester
Attachment #8575527 - Attachment is obsolete: true
Attachment #8619722 - Flags: review+
(Assignee)

Comment 21

2 years ago
Created attachment 8619722 [details]
MozReview Request: Bug 1141679 - Ignore the server's response when quitting the application with marionette.
You need to log in before you can comment on or make changes to this bug.