Closed
Bug 664932
Opened 13 years ago
Closed 13 years ago
Update unit tests must stop implementing nsIXMLHttpRequestEventTarget in script
Categories
(Toolkit :: Application Update, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: sicking, Assigned: sicking)
References
Details
Attachments
(1 file)
4.20 KB,
patch
|
robert.strong.bugs
:
review+
|
Details | Diff | Splinter Review |
Currently the updater unit tests implement a custom nsIXMLHttpRequest while running the tests. What's worse is that it also implements nsIXMLHttpRequestEventTarget which is getting marked as a 'builtinclass' in bug 661980 which means that it can't be implemented in script. We should soon be doing the same thing with nsIXMLHttpRequest since it should be extending nsIXMLHttpRequestEventTarget according to DOM specs. So we need to find another way of implementing these tests. A few options are: * Use the normal XHR implementation instead? * Add a new interface with the same properties as nsIXMLHttpRequest/nsIXMLHttpRequestEventTarget. (We'd also have to change the actual upload code to use nsISupports as interface argument to CreateInstance * Skip going through XPConnect somehow, which would remove the need for implementing any XPCOM interfaces at all.
Comment 1•13 years ago
|
||
I went with a custom nsIXMLHttpRequest implementation since I wasn't able to test all of the error conditions and it had the added benefit of it it being much faster. I can use the normal implementation but I don't know of a way to coerce nsIXMLHttpRequest to return these errors. I would feel better if the error conditions were tested by the nsIXMLHttpRequest test... I added a couple in bug 487571 but there are many more that I don't know of a way to add. Is that possible?
Assignee | ||
Comment 2•13 years ago
|
||
I went for the third bullet point from comment 0. The downside is that this does require a small modification to the shipped code, but it should hopefully be safe.
Assignee: nobody → jonas
Attachment #540192 -
Flags: review?(robert.bugzilla)
Assignee | ||
Comment 3•13 years ago
|
||
To answer your questions about more XHR tests. I honestly don't know what type of error situations we can simulate in for example mochitest. We do actually have a real networking team now though, so I bet they can help with figuring it out. Would be great if you could file a new bug (cc me and :smaug) and mention any specific things that you know of that needs testing.
Updated•13 years ago
|
Attachment #540192 -
Flags: review?(robert.bugzilla) → review+
Assignee | ||
Comment 4•13 years ago
|
||
Checked in! http://hg.mozilla.org/mozilla-central/rev/56854b8cf1a6 Thanks for the quick review!
Assignee | ||
Updated•13 years ago
|
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•