709829 - Port bug 673017 changeset 02ce78afb984 ('places-connection-closing' removal) to SeaMonkey

Status: VERIFIED FIXED

(SeaMonkey :: Bookmarks & History, defect)

Description

[Serge Gautherie (:sgautherie)]

2011-09-21
https://hg.mozilla.org/mozilla-central/rev/02ce78afb984
"Misc places fixes."

*****

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1323553945.1323558718.13019.gz
WINNT 5.2 comm-central-trunk debug test xpcshell on 2011/12/10 13:52:25  
{
TEST-UNEXPECTED-FAIL | e:/builds/slave/test/build/xpcshell/tests/suite/common/places/tests/unit/test_clearHistory_shutdown.js | places-connection-closing == places-connection-closed - See following stack:
}

'places-connection-closing' doesn't exist anymore: resync' SeaMonkey test.

Comment 1

[Serge Gautherie (:sgautherie)]

Attachment: (Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing' [Checked in: Comment 9]

Comment 2

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

Without the patch I get about the same as you do, but with it it fails, too:

TEST-UNEXPECTED-FAIL | e:/mozilla-src/seamonkey-central/mozilla/_tests/xpcshell/suite/common/places/tests/unit/test_clearHistory_shutdown.js | places-expiration-finished == places-connection-closed - See following stack:
JS frame :: e:\mozilla-src\comm-central\mozilla\testing\xpcshell\head.js :: do_throw :: line 453
JS frame :: e:\mozilla-src\comm-central\mozilla\testing\xpcshell\head.js :: _do_check_eq :: line 547
JS frame :: e:\mozilla-src\comm-central\mozilla\testing\xpcshell\head.js :: do_check_eq :: line 568
JS frame :: e:/mozilla-src/seamonkey-central/mozilla/_tests/xpcshell/suite/common/places/tests/unit/test_clearHistory_shutdown.js :: observe :: line 75

Comment 3

[Serge Gautherie (:sgautherie)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #2)
> Without the patch I get about the same as you do, but with it it fails, too:
> 
> TEST-UNEXPECTED-FAIL |
> e:/mozilla-src/seamonkey-central/mozilla/_tests/xpcshell/suite/common/places/
> tests/unit/test_clearHistory_shutdown.js | places-expiration-finished ==
> places-connection-closed - See following stack:

You are right. I can reproduce that error locally:
{
[...]
Received notification: places-connection-closed

TEST-UNEXPECTED-FAIL | [...]/test_clearHistory_shutdown.js | places-expiration-finished == places-connection-closed - See following stack:
[...]
}

I have no idea why 'places-expiration-finished' notification is not received.
http://mxr.mozilla.org/comm-central/search?string=places-expiration-finished&case=on

Nonetheless, afaict, my (port) patch is right/needed (on its own).

Comment 4

[Jens Hatlak (:InvisibleSmiley)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

> Nonetheless, afaict, my (port) patch is right/needed (on its own).

Probably; it does indeed remove one test that fails. But it doesn't make this whole testsuite pass, which I thought was the actual goal. I'll just leave it to Callek then whether he wants to go with the port or require a complete fix (which, granted, would require more analysis so may as well deserve its own bug).

Comment 5

[Serge Gautherie (:sgautherie)]

(In reply to Jens Hatlak (:InvisibleSmiley) from comment #4)

> But it doesn't make
> this whole testsuite pass, which I thought was the actual goal. I'll just

Wanted goal, for sure. But not the goal I (had) initially set for this bug.

> leave it to Callek then whether he wants to go with the port or require a
> complete fix (which, granted, would require more analysis so may as well
> deserve its own bug).

http://brasstacks.mozilla.com/topfails/test/SeaMonkey?name=xpcshell/tests/test_suite_places/unit/test_clearHistory_shutdown.js
(older)

http://brasstacks.mozilla.com/topfails/test/SeaMonkey?name=xpcshell/tests/suite/common/places/tests/unit/test_clearHistory_shutdown.js
Last time it started to fail: "2011-04-26 17:13 SeaMonkey -1   [0c9c4a20b02a]"
Last failure: "2011-09-21 09:00 SeaMonkey Linux   [1c7e6fcdc4fa]"
Newer/Current failures are obviously missing :-(


(before 'places-connection-closing' changeset)
[Mozilla/5.0 (Windows NT 5.0; rv:9.0a1) Gecko/20110920 Firefox/9.0a1 SeaMonkey/2.6a1] (nightly, 2011-09-20-00-30-01-comm-central-trunk)

5 notifications received :-)
Then
{
Looking for uncleared stuff.

TEST-UNEXPECTED-FAIL | [...]/test_clearHistory_shutdown.js | true == false - See following stack:
<TOP_LEVEL> :: line 94
observe :: line 92
}

Code is:
{
(92)      URIS.forEach(function(aUrl) {
        stmt.params.page_url = aUrl;
(94)        do_check_false(stmt.executeStep());
        stmt.reset();
      });
}


(after 'places-connection-closing' changeset)
[Mozilla/5.0 (Windows NT 5.0; rv:9.0a1) Gecko/20110922 Firefox/9.0a1 SeaMonkey/2.6a1] (nightly, 2011-09-22-00-30-05-comm-central-trunk)

Has this bug, as expected.
After applying patch Av1, back to previous error, as expected :-)


'places-expiration-finished' "regression" happened (later) in the last 3 months.
(To be continued...)

Comment 6

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #5)
> 'places-expiration-finished' "regression" happened (later) in the last 3
> months.
> (To be continued...)

I filed bug 711937.

Comment 7

[Serge Gautherie (:sgautherie)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

"approval-comm-aurora=?":
Will be wanted for SM 2.7 wrt bug 711937. No risk.

Comment 8

[Justin Wood (:Callek)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

Review of attachment 580950 [details] [diff] [review]:
-----------------------------------------------------------------

This looks fine, I do see lots of other changes in https://hg.mozilla.org/integration/mozilla-inbound/rev/02ce78afb984 though, relating to SQL in the tests, do we not have similar tests on our end, or is that to be a separate patch? (if separate patch, new bug please if it will be written/posted after today)

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

http://hg.mozilla.org/comm-central/rev/3f9d9e6e5c0c


(In reply to Justin Wood (:Callek) from comment #8)
> do we not have similar tests on our end

No, those tests were not ported to SeaMonkey:
http://mxr.mozilla.org/comm-central/search?string=.executeAsync%28%29&case=1

Comment 10

[Serge Gautherie (:sgautherie)]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1324367288.1324369912.18402.gz
OS X 10.6 comm-central-trunk debug test xpcshell on 2011/12/19 23:48:08

V.Fixed

Comment 11

[Serge Gautherie (:sgautherie)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

"approval-comm-beta=?":
Will be wanted for SM 2.7 wrt bug 711937. No risk.

Comment 12

[Justin Wood (:Callek)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

missed aurora, ok for beta

Comment 13

[Justin Wood (:Callek)]

(In reply to Justin Wood (:Callek) from comment #12)
> Comment on attachment 580950 [details] [diff] [review]
> (Av1) test_clearHistory_shutdown.js: Remove obsolete
> 'places-connection-closing'
> [Checked in: Comment 9]
> 
> missed aurora, ok for beta

Just to be clear, comm-beta is closed for now, wait until TB opens to push

Comment 14

[Serge Gautherie (:sgautherie)]

(In reply to Serge Gautherie (:sgautherie) from comment #9)
> http://hg.mozilla.org/comm-central/rev/3f9d9e6e5c0c

http://hg.mozilla.org/releases/comm-aurora/rev/3f9d9e6e5c0c
central->aurora AURORA_BASE_20111220 merge

Comment 15

[Serge Gautherie (:sgautherie)]

Comment on attachment 580950 [details] [diff] [review]
(Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing'
[Checked in: Comment 9]

Removing (test-only) approval, as bug 711937 proved to be test-only (too).