1528574 - TEST-UNEXPECTED-FAIL | comm/chat/protocols/irc/test/test_setMode.js

Status: RESOLVED FIXED

(Chat Core :: IRC, defect)

Description

[Jorg K (CEST = GMT+2)]

This appeared in a wave of recent bustage. Running the test locally I see:

0:00.89 pid:10948 JavaScript strict warning: file:///m:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/components/imConversations.js, line 78: ReferenceError: reference to undefined property "system"
0:00.89 pid:10948 JavaScript strict warning: file:///m:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/components/imConversations.js, line 74: ReferenceError: reference to undefined property "conversation"
0:00.89 pid:10948 JavaScript error: file:///m:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/components/imConversations.js, line 442: TypeError: aSubject.conversation is undefined

So this doesn't look healthy. Also check bug 1118515.

Maybe another job for Aceman.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: 1528574-switch-off-test.patch

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/01dfe512bc4d
switch off failing test test_setMode.js. rs=bustage-fix

Comment 3

[Florian Quèze [:florian]]

(In reply to Jorg K (GMT+1) from comment #0)

0:00.89 pid:10948 JavaScript error: file:///m:/mozilla-source/comm-central/obj-x86_64-pc-mingw32/dist/bin/components/imConversations.js, line 442: TypeError: aSubject.conversation is undefined

Smells like bug 1528907.

Comment 4

[Florian Quèze [:florian]]

I verified locally that my patch for bug 1528907 fixes this. Please backout attachment 9044518 [details] [diff] [review]. Thanks!

Note: it's unfortunate that chat in TB has so little test coverage, and that in this rare case where one of our tests caught a pretty severe bustage, the test got disabled as a "bustage fix".

Comment 5

[Jorg K (CEST = GMT+2)]

I can't see what's unfortunate about disabling the test. We have a few tests disabled, see https://mzl.la/2T5ZJz6. Would you expect the sheriff to navigate around all those failures on every push and try to detect new ones?

I disable all failing tests within 24 hours of filing the appropriate bug unless there is an obvious reason and I can fix them myself. Surely you don't expect a single person to follow all M-C changes.

This bug here was filed in the correct component (as far as I can see), and if anything was "unfortunate" here, it is that the bug didn't receive any attention until someone else filed another bug. Surely a test failure signifies a loss of functionality somewhere.

rs=bustage-fix is a general approval we use in cases like this. If you have a suggestion how to manage comm-central better, please let me know. Keeping the tree green on opt platforms is the only way to detect fresh bustage, even if tests have to be disabled to achieve that.

I will re-enable the test with my next push, thanks for letting me know.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/8bfea164811b
Backed out changeset 01dfe512bc4d to re-enable test after the failure cause was fixed in bug 1528907. a=backout DONTBUILD

Comment 7

[Jorg K (CEST = GMT+2)]

Fixed by bug 1528907, test re-enabled.

Comment 8

[Florian Quèze [:florian]]

(In reply to Jorg K (GMT+1) from comment #5)

rs=bustage-fix is a general approval we use in cases like this. If you have a suggestion how to manage comm-central better, please let me know. Keeping the tree green on opt platforms is the only way to detect fresh bustage, even if tests have to be disabled to achieve that.

Keeping the tree green makes sense in general when a test covering an edge case is broken. In this case chat was entirely busted (impossible to create a new account or to connect an existing one), which was a reason to keep nightlies disabled. When nightlies are disabled, I expect the tree to remain colorful and show the failures that need to be fixed before we can produce nightlies again.

Comment 9

[Jorg K (CEST = GMT+2)]

Sorry, I can't deliver: "I expect the tree to remain colorful and show the failures that need to be fixed before we can produce nightlies again". I would need to analyse each test failure to see whether it's "an edge case" or reflecting a major malfunction.

I also beg to differ. The treeherder is not an isssue tracker, BMO is!

Currently we have about 40 tests switched off in bug 1518823 which do reflect (minor) functional failures. I can't have these show up orange all the time. Also, you would even have to introduce of level of severity: Major failures stay, minor get switched off.

I think the preferable solution would be proper bug triaging, assigning of triage owners, and proper attention to test failures by the peers.

Comment 10

[Patrick Cloke [:clokep]]

This is no longer being skipped, see https://hg.mozilla.org/comm-central/rev/8bfea164811b