Closed Bug 709829 Opened 8 years ago Closed 8 years ago

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

Categories

(SeaMonkey :: Bookmarks & History, defect, major)

defect
Not set
major

Tracking

(seamonkey2.7 wontfix)

VERIFIED FIXED
seamonkey2.8
Tracking Status
seamonkey2.7 --- wontfix

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Blocks 1 open bug)

Details

(Whiteboard: [perma-orange])

Attachments

(1 file)

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.
Severity: normal → major
Whiteboard: [perma-orange]
Summary: Port bug 673017 changeset 02ce78afb984 to SeaMonkey → Port bug 673017 changeset 02ce78afb984 ('places-connection-closing' removal) to SeaMonkey
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
Attachment #580950 - Flags: feedback-
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).
Attachment #580950 - Flags: feedback- → feedback?(jh)
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).
Attachment #580950 - Flags: feedback?(jh)
(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...)
Depends on: 565307
Blocks: 711937
(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 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.
Attachment #580950 - Flags: approval-comm-aurora?
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)
Attachment #580950 - Flags: review?(bugspam.Callek)
Attachment #580950 - Flags: review+
Attachment #580950 - Flags: approval-comm-aurora?
Attachment #580950 - Flags: approval-comm-aurora+
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
Attachment #580950 - Attachment description: (Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing' → (Av1) test_clearHistory_shutdown.js: Remove obsolete 'places-connection-closing' [Checked in: Comment 9]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [perma-orange] → [c-n: to comm‑aurora] [perma-orange]
Target Milestone: --- → seamonkey2.8
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
Status: RESOLVED → VERIFIED
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.
Attachment #580950 - Flags: approval-comm-beta?
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
Attachment #580950 - Flags: approval-comm-beta?
Attachment #580950 - Flags: approval-comm-beta+
Attachment #580950 - Flags: approval-comm-aurora+
(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
(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
Whiteboard: [c-n: to comm‑aurora] [perma-orange] → [c-n: to comm‑beta] [perma-orange]
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).
Attachment #580950 - Flags: approval-comm-beta+
Keywords: checkin-needed
Whiteboard: [c-n: to comm‑beta] [perma-orange] → [perma-orange]
Target Milestone: seamonkey2.8 → seamonkey2.9
Target Milestone: seamonkey2.9 → seamonkey2.8
No longer depends on: OrangeFactorCommApps
You need to log in before you can comment on or make changes to this bug.