Closed Bug 1118515 Opened 8 years ago Closed 3 years ago

TEST-UNEXPECTED-FAIL | chat/protocols/irc/test/test_setMode.js | xpcshell return code: 0

Categories

(Chat Core :: IRC, defect)

Product:
Chat Core
Component:
IRC
Platform:
x86_64
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: hiro, Unassigned)

Details

(Keywords: intermittent-failure)

Attachments

(1 file, 1 obsolete file)

log_exception.patch
1016 bytes, patch
hiro
: review+
Details | Diff | Splinter Review 
04:59:17 INFO - chat/protocols/irc/test/test_setMode.js | Starting test_topicSettable
04:59:17 INFO - (xpcshell/head.js) | test test_topicSettable pending (2)
04:59:17 INFO - TEST-PASS | chat/protocols/irc/test/test_setMode.js | test_topicSettable - [test_topicSettable : 29] false == false
04:59:17 INFO - TEST-PASS | chat/protocols/irc/test/test_setMode.js | test_topicSettable - [test_topicSettable : 34] true == true
04:59:17 INFO - TypeError: this.ERROR is not a function at resource:///modules/jsProtoHelper.jsm:479
04:59:17 INFO - GenericConversationPrototype.notifyObservers@resource:///modules/jsProtoHelper.jsm:479:9
04:59:17 INFO - GenericMessagePrototype.conversation@resource:///modules/jsProtoHelper.jsm:413:5
04:59:17 INFO - GenericConversationPrototype.writeMessage@resource:///modules/jsProtoHelper.jsm:501:6
04:59:17 INFO - GenericConvChatPrototype.writeMessage@resource:///modules/jsProtoHelper.jsm:626:5
04:59:17 INFO - ircChannel.prototype.setMode@/builds/slave/talos-slave/test/build/tests/xpcshell/tests/chat/protocols/irc/test/test_setMode.js -> resource:///components/irc.js:516:1
04:59:17 INFO - test_topicSettable@/builds/slave/talos-slave/test/build/tests/xpcshell/tests/chat/protocols/irc/test/test_setMode.js:37:3
04:59:17 INFO - _run_next_test@/builds/slave/talos-slave/test/build/tests/xpcshell/head.js:1354:11
04:59:17 INFO - do_execute_soon/<.run@/builds/slave/talos-slave/test/build/tests/xpcshell/head.js:616:9
04:59:17 INFO - _do_main@/builds/slave/talos-slave/test/build/tests/xpcshell/head.js:184:5
04:59:17 INFO - _execute_test@/builds/slave/talos-slave/test/build/tests/xpcshell/head.js:476:5
04:59:17 INFO - @-e:1:1
04:59:17 INFO - exiting test
See Also: → 955446
Can you expand why you think that's related?
(In reply to Patrick Cloke [:clokep] from comment #5)
> Can you expand why you think that's related?

From the failure log

 04:59:17 INFO - GenericConversationPrototype.notifyObservers@resource:///modules/jsProtoHelper.jsm:479:9

jsProtoHelper.jsm
474   notifyObservers: function(aSubject, aTopic, aData) {
475     for each (let observer in this._observers) {
476       try {
477         observer.observe(aSubject, aTopic, aData);
478       } catch(e) {
479         this.ERROR(e);
480       }
481     }
482   },

Actually observer.observe throws an exception, I am guessing the exception is the same as reported in bug 1118515.

Of course, you know, it's a problem that FakeAccount in test_setMode.js does not have ERROR method, but it is not the reason of this intermittent failure.
(In reply to Hiroyuki Ikezoe (:hiro) from comment #6)
> (In reply to Patrick Cloke [:clokep] from comment #5)
> > Can you expand why you think that's related?
> 
> From the failure log
> 
>  04:59:17 INFO -
> GenericConversationPrototype.notifyObservers@resource:///modules/
> jsProtoHelper.jsm:479:9
> 
> jsProtoHelper.jsm
> 474   notifyObservers: function(aSubject, aTopic, aData) {
> 475     for each (let observer in this._observers) {
> 476       try {
> 477         observer.observe(aSubject, aTopic, aData);
> 478       } catch(e) {
> 479         this.ERROR(e);
> 480       }
> 481     }
> 482   },
> 
> Actually observer.observe throws an exception, I am guessing the exception
> is the same as reported in bug 1118515.

bug 955446, I meant.
I might be wrong... there are other possibility throwing exception in observe methods in chat/.
Attached patch log_exception.patch (obsolete) — — Splinter Review 
This patch is not a fix for this intermittent failure.

To get the exception in log, we need to implement ERROR method there.
Attachment #8544961 - Flags: review?(clokep)
Comment on attachment 8544961 [details] [diff] [review]
log_exception.patch

Review of attachment 8544961 [details] [diff] [review]:
-----------------------------------------------------------------

::: chat/protocols/irc/test/test_setMode.js
@@ +14,5 @@
>    __proto__: irc.ircAccount.prototype,
> +  setWhois: (n, f) => true,
> +  ERROR: function(e) {
> +    do_throw(e);
> +  }

Can't this just be ERROR: do_throw?
Attachment #8544961 - Flags: review?(clokep) → review+
Attached patch log_exception.patch — — Splinter Review 
(In reply to Patrick Cloke [:clokep] from comment #10)
> Comment on attachment 8544961 [details] [diff] [review]
> log_exception.patch
> 
> Review of attachment 8544961 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: chat/protocols/irc/test/test_setMode.js
> @@ +14,5 @@
> >    __proto__: irc.ircAccount.prototype,
> > +  setWhois: (n, f) => true,
> > +  ERROR: function(e) {
> > +    do_throw(e);
> > +  }
> 
> Can't this just be ERROR: do_throw?

yes!
Attachment #8544961 - Attachment is obsolete: true
Attachment #8545206 - Flags: review+
Keywords: checkin-needed, leave-open
See Also: 955446 →

I believe this is now passing fine (see bug 1528574).

Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.