"suite/common/tests/browser/browser_346337.js | Test timed out" : "WARNING: RestoreFromHistory failed" + "ASSERTION: aIndex is out of range: 'aIndex < mLength'"

VERIFIED FIXED in seamonkey2.10

Status

defect
--
major
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: sgautherie, Assigned: InvisibleSmiley)

Tracking

(Blocks 1 bug, {assertion})

Dependency tree / graph
Bug Flags:
in-testsuite +

SeaMonkey Tracking Flags

(seamonkey2.8 verified, seamonkey2.9 verified)

Details

(Whiteboard: [perma-orange], )

Attachments

(1 attachment)

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1328740940.1328745964.22504.gz
WINNT 5.2 comm-central-trunk debug test mochitest-other on 2012/02/08 14:42:20
{
...
TEST-PASS | chrome://mochitests/content/browser/suite/common/tests/browser/browser_346337.js | The value for "//input[@type='file'][2]" was correctly restored

WARNING: RestoreFromHistory failed: file e:/builds/slave/comm-cen-trunk-w32-dbg/build/mozilla/docshell/base/nsDocShell.cpp, line 6755
###!!! ASSERTION: aIndex is out of range: 'aIndex < mLength', file e:/builds/slave/comm-cen-trunk-w32-dbg/build/mozilla/docshell/shistory/src/nsSHistory.cpp, line 957

TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/tests/browser/browser_346337.js | Test timed out
}

PS: Reproduced locally while fixing bug 725529.

***

Then this test "leaks" into following ones, as in
{
TEST-UNEXPECTED-FAIL | chrome://mochitests/content/browser/suite/common/tests/browser/browser_522545.js | an unexpected uncaught JS exception reported through window.onerror - ok is not defined at chrome://mochitests/content/browser/suite/common/tests/browser/browser_346337.js:141
}
This is what I figured from the main difference between our test and FF's.

Note: I haven't checked the follow-up errors, but only running this test alone.
Assignee: nobody → jh
Status: NEW → ASSIGNED
Attachment #596340 - Flags: review?(sgautherie.bz)
Depends on: 726505
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

I confirm this fixes this bug locally :-)
But I have no idea why this test used some 'pageshow' (in SM) instead of 'load' (in FF) in the first place.

While looking at this patch, I noticed and filed bug 726505, which I would like to do first, just in case.
Attachment #596340 - Flags: review?(sgautherie.bz)
Attachment #596340 - Flags: review?(neil)
Attachment #596340 - Flags: feedback+
Depends on: 480109
Target Milestone: --- → seamonkey2.10
(In reply to Serge Gautherie (:sgautherie) from comment #2)
> While looking at this patch, I noticed and filed bug 726505, which I would
> like to do first, just in case.

This patch is still needed, even after fixing bug 726505.
No longer depends on: 726505
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

This test and the Firefox one look substantially different, even with the event change as given in the patch, so I can't tell how our test works at all, sorry.
Attachment #596340 - Flags: review?(neil)
Serge, I guess either you need to turn your f+ into an r+, or we need to find another reviewer (who?), or remove this test.
Perhaps Misak can remember why the tests differ so much?
(In reply to neil@parkwaycc.co.uk from comment #4)
> I can't tell how our test works at all, sorry.

Can you give a rs+, as you reviewed bug 480109?
(Or suggest another reviewer?)


(In reply to Jens Hatlak (:InvisibleSmiley) from comment #5)
> Serge, I guess either you need to turn your f+ into an r+

Well, I don't think/remember I'm allowed to give r+ (on anything).
Ian, correct me if I'm wrong.


(In reply to Philip Chee from comment #6)
> Perhaps Misak can remember why the tests differ so much?

Yeah, but it looks like Misak hasn't been active/responsive since last november 2011 meeting :-/
Attachment #596340 - Flags: feedback?(neil)
(In reply to Serge Gautherie (:sgautherie) from comment #7)
> (In reply to Jens Hatlak (:InvisibleSmiley) from comment #5)
> > Serge, I guess either you need to turn your f+ into an r+
> 
> Well, I don't think/remember I'm allowed to give r+ (on anything).

If that is true (it might be), I think this needs to be changed, considering that you probably have the most test experience and the FF guys even commit stuff with "a=testonly" and no (obvious) review at all. If we have failing tests and someone (e.g. you) can tell that a fix is correct, by all means that should suffice (IMHO). Writing new tests is probably another story, though, but that's not what I'm up to here.
Attachment #596340 - Flags: feedback?(neil) → feedback+
> If that is true (it might be), I think this needs to be changed, considering that you
> probably have the most test experience and the FF guys even commit stuff with
> "a=testonly" and no (obvious) review at all.

And I've seen Firefox test only changes go in without even a bug number. So given Serge's experience with tests I think he is already a de facto seamonkey-tests peer so he can give r+ if needed.
(In reply to Philip Chee from comment #9)
> So given Serge's experience with tests I think he is already a de facto
> seamonkey-tests peer so he can give r+ if needed.

Ian, if you think this needs a vote, please take this up with the Council, otherwise integrate into what you are working on. Thanks!
(In reply to Jens Hatlak (:InvisibleSmiley) from comment #10)
> (In reply to Philip Chee from comment #9)
> > So given Serge's experience with tests I think he is already a de facto
> > seamonkey-tests peer so he can give r+ if needed.
> 
> Ian, if you think this needs a vote, please take this up with the Council,
> otherwise integrate into what you are working on. Thanks!

This doesn't need a vote, I am happy for Serge's f+ = r+
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

Then, even if we don't (bother to) have a full understanding of this test code,
this patch fixes the failure and makes the test somewhat closer to Firefox code,
so f+ -> r+.
Attachment #596340 - Flags: feedback+ → review+
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

http://hg.mozilla.org/comm-central/rev/650205a133cd
Attachment #596340 - Attachment description: patch → patch [Checkin: Comment 13]
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
OS: Windows Server 2003 → All
Hardware: x86 → All
Resolution: --- → FIXED
Flags: in-testsuite+
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

[Approval Request Comment]
Test-only, wanted to clear the timeout and the rest.
Attachment #596340 - Flags: approval-comm-beta?
Attachment #596340 - Flags: approval-comm-aurora?
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1329271174.1329275384.15583.gz&fulltext=1
Linux comm-central-trunk debug test mochitest-other on 2012/02/14 17:59:34

V.Fixed, test + warning + assertion + etc!
Status: RESOLVED → VERIFIED
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

WoooHooo clearing timeouts means more time to spend on the machines actually doing stuff!

p.s.: I agree with
|So given Serge's experience with tests I think he is already a de facto seamonkey-tests peer so he can give r+ if needed.|
Attachment #596340 - Flags: approval-comm-beta?
Attachment #596340 - Flags: approval-comm-beta+
Attachment #596340 - Flags: approval-comm-aurora?
Attachment #596340 - Flags: approval-comm-aurora+
Comment on attachment 596340 [details] [diff] [review]
patch [Checkin: Comments 13, 18 and 19]

http://hg.mozilla.org/releases/comm-aurora/rev/4138254af9bf
Attachment #596340 - Attachment description: patch [Checkin: Comment 13] → patch [Checkin: Comments 13, 18 and 19]
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Aurora/1331173545.1331178886.17486.gz
Linux comm-aurora debug test mochitest-other on 2012/03/07 18:25:45

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey-Beta/1331186022.1331189779.1265.gz
OS X 10.5 comm-beta debug test mochitest-other on 2012/03/07 21:53:42

seamonkey2.9 and seamonkey2.8: verified.
You need to log in before you can comment on or make changes to this bug.