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)

defect
Not set
major

Tracking

()

VERIFIED FIXED
mozilla1.9.3a5

People

(Reporter: sgautherie, Assigned: sgautherie)

References

(Blocks 1 open bug, )

Details

Attachments

(1 file, 1 obsolete file)

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?
Probably needs work to support running in apps that have extensions included.
*cough* toolkit@mozilla.org *cough*
(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.
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
Fix/Workaround wanted: this is perma-orange on SM21.
status2.0: --- → ?
Depends on: NoC192SM
(In reply to comment #5)

Would it be acceptable to use checks like
do_check_true(WindowWatcher.arguments.length >= 3);
in the meantime?
Assignee: nobody → sgautherie.bz
Status: NEW → ASSIGNED
Attachment #434794 - Flags: review?(robert.bugzilla)
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)
Depends on: 553890
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-
(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.
(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.
(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.
(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.
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.
(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.
(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?
(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.
(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?
(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.
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 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+
Attachment #440130 - Attachment description: (Av2) (Temporarily) Disable 2 checks → (Av2) (Temporarily) Disable 2 checks [Checkin: Comment 22]
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
status2.0: ? → ---
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Depends on: 461973
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.

Attachment

General

Created:
Updated:
Size: