966563 - Add xpcshell tests for fixes in bug 966560

Status: RESOLVED WORKSFORME

(Firefox :: Sync, defect)

Description

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

What it says.

Comment 1

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

testing whether the onComplete method fired successfully in the RESTRequest is tricky.  If onComplete is not a function, it does not crash, since it's fired on next tick.  All we get is a 'JS Error' fprintf'd to stderr.  Maybe there's a way we can listen to stderr in xpcshell and see if there's a JS Error?

Comment 2

[:Gavin Sharp [email: [e-mail]]]

You can generally catch reported exceptions with an nsIConsoleService listener. But wouldn't it be easier to somehow test for "onComplete ran" rather than "calling onComplete didn't throw"?

Comment 3

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

This isn't really a reported exception - it's just written right to stderr.  Wouldn't that be inaudible to an nsIConsoleService listener?

But to your main point, agreed: It would be best to positively verify that onComplete ran.  I didn't at first go this way because my initial impulse was that I shouldn't modify the existing rest.js code.  Perhaps the best way to answer this question is through exploring with a patch.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to Jed Parsons (use needinfo, please) [:jedp, :jparsons] from comment #3)
> This isn't really a reported exception - it's just written right to stderr. 

Do you have a pointer?

Comment 5

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

I think the value of such tests would be low compared to the time required to perfect them.  I opened this bug during some tense days, when I was nervous about anything that didn't have a test.  From my perspective now, I think this is overkill.  Please reopen if you think I'm wrong.