Closed
Bug 547039
Opened 14 years ago
Closed 14 years ago
[SeaMonkey 2.1] xpcshell: test_bug542391.js fails since landing
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
VERIFIED
FIXED
mozilla1.9.3a5
People
(Reporter: sgautherie, Assigned: sgautherie)
References
(Blocks 1 open bug, )
Details
Attachments
(1 file, 1 obsolete file)
1.88 KB,
patch
|
mossop
:
review+
|
Details | Diff | Splinter Review |
Landing happened on http://tinderbox.mozilla.org/showbuilds.cgi?tree=SeaMonkey&maxdate=1265786768&hours=24&legend=0&norules=1 between http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1265756014.1265758041.29567.gz Linux comm-central-trunk debug test xpcshell on 2010/02/09 14:53:34 and http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1265775788.1265777756.26264.gz Linux comm-central-trunk debug test xpcshell on 2010/02/09 20:23:08 *** http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1266513354.1266514667.21677.gz&fulltext=1 Linux comm-central-trunk debug test xpcshell on 2010/02/18 09:15:54 { TEST-UNEXPECTED-FAIL | /builds/slave/comm-central-trunk-linux-debug-unittest-xpcshell/build/xpcshell/tests/test_extensionmanager/unit/test_bug542391.js | test failed (with xpcshell return code: 0), see following log: [...] TEST-PASS | test_extensionmanager/unit/test_bug542391.js | [check_state_v3 : 158] false == false TEST-UNEXPECTED-FAIL | test_extensionmanager/unit/test_bug542391.js | 8 == 3 - See following stack: JS frame :: xpcshell/head.js :: do_throw :: line 202 JS frame :: xpcshell/head.js :: do_check_eq :: line 232 JS frame :: test_extensionmanager/unit/test_bug542391.js :: run_test_1 :: line 201 JS frame :: test_extensionmanager/unit/test_bug542391.js :: run_test :: line 191 JS frame :: xpcshell/head.js :: _execute_test :: line 130 JS frame :: -e :: <TOP_LEVEL> :: line 1 } *** Bug 542391 comment 17: { Dave Townsend (:Mossop) 2010-02-04 20:40:38 PST This adds unit tests for both the Firefox and Fennec cases. } Should that test be disabled (or completed) for other apps then?
Comment 1•14 years ago
|
||
Probably needs work to support running in apps that have extensions included.
Comment 2•14 years ago
|
||
*cough* toolkit@mozilla.org *cough*
Comment 3•14 years ago
|
||
(In reply to comment #2) > *cough* toolkit@mozilla.org *cough* Yes, that is the problem I think, the extensions in Seamonkey are compatible with toolkit so they also work in the xpcshell tests.
Assignee | ||
Comment 4•14 years ago
|
||
MacOSX too: { http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1266802005.1266803746.19182.gz OS X 10.5 comm-central-trunk debug test xpcshell on 2010/02/21 17:26:45 }
OS: Linux → All
Hardware: x86 → All
Assignee | ||
Comment 5•14 years ago
|
||
Fix/Workaround wanted: this is perma-orange on SM21.
Assignee | ||
Comment 6•14 years ago
|
||
(In reply to comment #5) Would it be acceptable to use checks like do_check_true(WindowWatcher.arguments.length >= 3); in the meantime?
Assignee | ||
Comment 7•14 years ago
|
||
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #434794 -
Flags: review?(robert.bugzilla)
Comment 8•14 years ago
|
||
Comment on attachment 434794 [details] [diff] [review] (Av1) Account for SeaMonkey included add-ons Serge, Dave is doing a rewrite of the Extension Manager atm so I'm pushing the review over to him since it may adversely impact the work he is doing. Also, most of us are extremely busy finishing up several projects and taking time to review changes like this may need to wait until the other work has been completed.
Attachment #434794 -
Flags: review?(robert.bugzilla) → review?(dtownsend)
Comment 9•14 years ago
|
||
Comment on attachment 434794 [details] [diff] [review] (Av1) Account for SeaMonkey included add-ons I don't want us manually altering tests everytime an application changes the extensions they ship with. As Rob says these tests are all changing in the near future anyway, probably better to wait until then and implement a more general solution for this common problem.
Attachment #434794 -
Flags: review?(dtownsend) → review-
Assignee | ||
Comment 10•14 years ago
|
||
(In reply to comment #9) > I don't want us manually altering tests everytime an application changes the Does it actually happen (that often)? > extensions they ship with. As Rob says these tests are all changing in the near Then, what's wrong in getting it to pass in the meantime? > future anyway, probably better to wait until then and implement a more general > solution for this common problem. Any such fix is/will_be welcome.
Comment 11•14 years ago
|
||
(In reply to comment #10) > (In reply to comment #9) > > > I don't want us manually altering tests everytime an application changes the > > Does it actually happen (that often)? With all the apps that are out there I'd imagine it would be often enough to be a nuisance. > > extensions they ship with. As Rob says these tests are all changing in the near > > Then, what's wrong in getting it to pass in the meantime? My understanding is you aren't developing against 1.9.3 currently so I don't believe it is worth the time to look at this further now when we'll only have to look at it differently next week.
Assignee | ||
Comment 12•14 years ago
|
||
(In reply to comment #11) > With all the apps that are out there I'd imagine it would be often enough to be > a nuisance. Not sure which apps you're talking about: only SeaMonkey tinderboxes are failing afaik. > My understanding is you aren't developing against 1.9.3 currently On the contrary, SeaMonkey (2.1) officially decided to skip m-1.9.2.
Comment 13•14 years ago
|
||
(In reply to comment #12) > (In reply to comment #11) > > > With all the apps that are out there I'd imagine it would be often enough to be > > a nuisance. > > Not sure which apps you're talking about: only SeaMonkey tinderboxes are > failing afaik. I imagine that any application that ships with extensions in the application directory is failing right now. I don't want to play whack-a-mole. > > My understanding is you aren't developing against 1.9.3 currently > > On the contrary, SeaMonkey (2.1) officially decided to skip m-1.9.2. If you're desperate then I'd rather just disable the test completely until the rewrite branch lands in a few days where I suspect this issue is already fixed.
Comment 14•14 years ago
|
||
Could this test be either shortened out or our pre-shipped extensions be masked out until the EM rework comes in and changes are needed anyhow? After that, I'd like to get us to fix this in a suitable way so that the problems don't crop up all over again. It probably makes sense to actually have a good mechanism for those preinstalled extensions then in the new system - preferably a mechanism that even has tests of its own. IMHO, we should re-purpose bug 553890 for doing that and make it dependent on the EM rework.
Comment 15•14 years ago
|
||
(In reply to comment #14) > Could this test be either shortened out or our pre-shipped extensions be masked > out until the EM rework comes in and changes are needed anyhow? As I said I'd rather just disable the test. File the one line patch to do it and I'll r+ it. > After that, I'd like to get us to fix this in a suitable way so that the > problems don't crop up all over again. It probably makes sense to actually have > a good mechanism for those preinstalled extensions then in the new system - > preferably a mechanism that even has tests of its own. IMHO, we should > re-purpose bug 553890 for doing that and make it dependent on the EM rework. As I said I believe that this is already taken care of on the rewrite branch, I don't really have the time to test it against SeaMonkey right now though.
Comment 16•14 years ago
|
||
(In reply to comment #15) > As I said I'd rather just disable the test. File the one line patch to do it > and I'll r+ it. Unfortunately, it's not a one-liner, as we're including the whole unit/ directory in one command in the Makefile.in :( That's why I spoke about shortening out that test if it's run on SeaMonkey - for now. If it would be as simple as the usual removal of the fitting line in Makefile.in, I would just have attached the patch... > As I said I believe that this is already taken care of on the rewrite branch, I > don't really have the time to test it against SeaMonkey right now though. It's very good to hear that this is already taken care of, I'm looking forward to that. Can you tell us over in bug 553890 what we need to do on our side to work well with what's coming up there?
Comment 17•14 years ago
|
||
(In reply to comment #16) > Can you tell us over in bug 553890 what we need to do on our side to > work well with what's coming up there? Make that bug 555615, the other one was pulled away under my feet.
Assignee | ||
Comment 18•14 years ago
|
||
(In reply to comment #13) > until the > rewrite branch lands in a few days where I suspect this issue is already fixed. Weren't these few more days actually many days ago?! The failure is still there: what is (not) happening?
Comment 19•14 years ago
|
||
(In reply to comment #18) > (In reply to comment #13) > > until the > > rewrite branch lands in a few days where I suspect this issue is already fixed. > > Weren't these few more days actually many days ago?! > The failure is still there: what is (not) happening? Reviews are taking longer than expected. You are welcome to file a patch to disable the test as I suggested. I single |return| at the start of run_test would do it.
Assignee | ||
Comment 20•14 years ago
|
||
I don't understand why you want to disable the whole test when 2 checks only are failing...
Attachment #434794 -
Attachment is obsolete: true
Attachment #440130 -
Flags: review?(dtownsend)
Comment 21•14 years ago
|
||
Comment on attachment 440130 [details] [diff] [review] (Av2) (Temporarily) Disable 2 checks [Checkin: Comment 22] If that works then fine
Attachment #440130 -
Flags: review?(dtownsend) → review+
Assignee | ||
Updated•14 years ago
|
Attachment #440130 -
Attachment description: (Av2) (Temporarily) Disable 2 checks → (Av2) (Temporarily) Disable 2 checks
[Checkin: Comment 22]
Assignee | ||
Comment 22•14 years ago
|
||
Comment on attachment 440130 [details] [diff] [review] (Av2) (Temporarily) Disable 2 checks [Checkin: Comment 22] http://hg.mozilla.org/mozilla-central/rev/09540c132187
Assignee | ||
Updated•14 years ago
|
Assignee | ||
Comment 23•14 years ago
|
||
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1271768995.1271772770.23531.gz WINNT 5.2 comm-central-trunk debug test xpcshell on 2010/04/20 06:09:55 V.Fixed
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•