Closed Bug 728647 Opened 8 years ago Closed 8 years ago

[OSX] browser_dataman_basics.js and browser_dataman_callviews.js fail, wrt IPv6

Categories

(SeaMonkey :: Passwords & Permissions, defect, P2, major)

All
macOS

Tracking

(seamonkey2.8- wontfix, seamonkey2.9 verified)

VERIFIED FIXED
seamonkey2.10
Tracking Status
seamonkey2.8 - wontfix
seamonkey2.9 --- verified

People

(Reporter: sgautherie, Assigned: kairo)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [perma-orange])

Attachments

(2 files)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329619612.1329623931.5998.gz
OS X 10.5 comm-central-trunk debug test mochitest-other on 2012/02/18 18:46:52
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329624302.1329629638.17173.gz
OS X 10.6 comm-central-trunk debug test mochitest-other on 2012/02/18 20:05:02
+
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1329628564.1329632893.23153.gz
OS X 10.5 comm-aurora debug test mochitest-other on 2012/02/18 21:16:04
+
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1329513480.1329517113.13259.gz
OS X 10.6 comm-beta debug test mochitest-other on 2012/02/17 13:18:00
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | IPv6 domain is selected - Got *, expected [::1]
[and many more]

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/dataman/tests/browser_dataman_callviews.js | Step 6: The correct domain is selected - Got [::1], expected *
[and a few more]
}
Sounds like either the sorting has changed or some default entries have.
Summary: browser_dataman_basics.js and browser_dataman_callviews.js fail, wrt IPv6 → [OSX] browser_dataman_basics.js and browser_dataman_callviews.js fail, wrt IPv6
On my Linux machine, all dataman tests pass, so this is either coming from somewhere else or it's OSX-specific.
Still, this one:

TEST-INFO | chrome://mochitests/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | run test #2 of 10 (test_forget_ipv6)
NEXT ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/dataman/tests/browser_dataman_basics.js | IPv6 domain is selected - Got *, expected [::1]

pretty much tells the story that the order in the domain pane is wrong. This only does a select for the second entry in the list, see http://mxr.mozilla.org/comm-central/source/suite/common/dataman/tests/browser_dataman_basics.js#150 and gets *, which should _always_ be the first item. Maybe I need to find some way to enforce that somewhat harder.
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #2)
> On my Linux machine, all dataman tests pass, so this is either coming from
> somewhere else or it's OSX-specific.

This bug _is_ filed as OSX-specific.
Attachment #600824 - Flags: review? → review?(iann_bugzilla)
Assignee: nobody → kairo
Comment on attachment 600824 [details] [diff] [review]
v1: try fixing by force-sorting "*" as first entry

(hrm, my comment wasn't submitted, I messed up a bit when attaching this, so delivering the comment now)

This is my try at fixing this problem by always force-sorting "*" to the top of the list. The more I think about this, the more I think we should have done that from the beginning anyhow.
Status: NEW → ASSIGNED
Keywords: helpwanted
Attachment #600824 - Flags: review?(iann_bugzilla) → review+
Thanks, pushed as http://hg.mozilla.org/comm-central/rev/042808980445 - I hope it really fixed the tests (as I can't test on Mac).
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1330818089.1330824354.559.gz
OS X 10.6 comm-central-trunk debug test mochitest-other on 2012/03/03 15:41:29
still has same failures :-/

Trying to guess:
Could it be that this.sort() needs to be called from gDomains.initialize(), like some of the other *.initialize() do?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #7)
> I can't test on Mac

Any chance to add some debugging code, like (somehow) taking a (few) snapshot?


(In reply to Serge Gautherie (:sgautherie) from comment #8)
> Could it be that this.sort() needs to be called from gDomains.initialize(),
> like some of the other *.initialize() do?

Hum, maybe irrelevant, but...
(In reply to Serge Gautherie (:sgautherie) from comment #9)
> (In reply to Robert Kaiser (:kairo@mozilla.com) from comment #7)
> > I can't test on Mac
> 
> Any chance to add some debugging code, like (somehow) taking a (few)
> snapshot?

I don't think that would be a good idea. Someone should try it on a Mac and tell us what (s)he sees.

> (In reply to Serge Gautherie (:sgautherie) from comment #8)
> > Could it be that this.sort() needs to be called from gDomains.initialize(),
> > like some of the other *.initialize() do?
> 
> Hum, maybe irrelevant, but...

It is irrelevant, sorting is called.
D'oh. Here's the patch that actually makes it do the correct thing. Ian, I won't tell anyone you gave the first one your review if you don't tell anyone I wrote it, OK? (If we had looked the original .localeCompare-containing line a bit more careful, we would have spotted all along what was wrong with that patch.)

I checked with injecting an empty ("") domain in the list, which localeCompare sorts before "*" even on Linux, and could verify that it was wrong before the patch and correct after the patch. I still won't add that case to the test as it's hacky and we have Mac to verify the sorting is correct anyhow.
Attachment #604548 - Flags: review?(iann_bugzilla)
Comment on attachment 604548 [details] [diff] [review]
Correct earlier patch to do what it should do

Obviously ;->
Attachment #604548 - Flags: feedback+
Attachment #604548 - Flags: review?(iann_bugzilla) → review+
OK, this should really do it now: http://hg.mozilla.org/comm-central/rev/e197041b4425 (thankfully still in time for 2.10, from what I see).
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1331684074.1331690325.18324.gz
OS X 10.6 comm-central-trunk debug test mochitest-other on 2012/03/13 17:14:34

V.Fixed
Status: RESOLVED → VERIFIED
Flags: in-testsuite-
Comment on attachment 604548 [details] [diff] [review]
Correct earlier patch to do what it should do

[Approval Request Comment]
For (the merge of) both patches.
Attachment #604548 - Flags: approval-comm-beta?
Comment on attachment 604548 [details] [diff] [review]
Correct earlier patch to do what it should do

I'll grant it, but I want Robert or IanN to decide explicitly that it is safe enough from their minds.
Attachment #604548 - Flags: approval-comm-beta? → approval-comm-beta+
I'm sure it's safe enough, but I wouldn't waste sleep over the test failures on beta, we should get some more green on trunk first.
That said, I've landed it still because I think not always sorting "*" to the top is a significant flaw and would be good to already have fixed in a 2.9 release.

http://hg.mozilla.org/releases/comm-beta/rev/9ac132949e97 (merged both patches to a single correct one)
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1331888181.1331892041.23235.gz
OS X 10.5 comm-beta debug test mochitest-other on 2012/03/16 01:56:21

seamonkey2.9: verified.


(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #17)

> I'm sure it's safe enough, but I wouldn't waste sleep over the test failures
> on beta, we should get some more green on trunk first.

(Makes me wonder what I have been doing for the past 3 months...)

> That said, I've landed it still because I think not always sorting "*" to
> the top is a significant flaw and would be good to already have fixed in a
> 2.9 release.

(Iow, this is not a test fix but an application "fix"...)
(In reply to Serge Gautherie (:sgautherie) from comment #18)
> (Makes me wonder what I have been doing for the past 3 months...)

A damn great job, AFAICT, given how much green there is already. :)

> (Iow, this is not a test fix but an application "fix"...)

Yes, found by the test (thanks for reporting) but ultimately a correctness fix in the code itself.
You need to log in before you can comment on or make changes to this bug.