664932 - Update unit tests must stop implementing nsIXMLHttpRequestEventTarget in script

Status: RESOLVED FIXED

(Toolkit :: Application Update, defect)

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

[Robert Strong (they/them - no direct email)]

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?

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: Patch to fix

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.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Checked in! http://hg.mozilla.org/mozilla-central/rev/56854b8cf1a6

Thanks for the quick review!