Closed Bug 1085562 Opened 7 years ago Closed 7 years ago

TEST-UNEXPECTED-FAIL | ../../../resources/logHelper.js | Error console says [stackFrame [Exception... "Failure arg 0 [nsITreeView.setTree]" nsresult: "0x80004005 (NS_ERROR_FAILURE)" location: "JS frame :: /builds/slave/test/build/tests/xpcshell/tests/mai

Categories

(MailNews Core :: Backend, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(thunderbird36 fixed)

RESOLVED FIXED
Thunderbird 36.0
Tracking Status
thunderbird36 --- fixed

People

(Reporter: jcranmer, Assigned: jcranmer)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Don't mock nsITreeBoxObject (obsolete) — Splinter Review
We don't necessarily need the tree box object... so let's tweak some of the tests to make this not necessary.
Attachment #8508141 - Flags: review?(kent)
Attachment #8508141 - Flags: review?(irving)
It helps to rerun all mailnews tests first.
Attachment #8508141 - Attachment is obsolete: true
Attachment #8508141 - Flags: review?(kent)
Attachment #8508141 - Flags: review?(irving)
Attachment #8508144 - Flags: review?(kent)
Attachment #8508144 - Flags: review?(irving)
Comment on attachment 8508144 [details] [diff] [review]
Don't mock nsITreeBoxObject

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

I totally agree with Joshua.
I had also tried to remove the mock, and I gave up because it's too hard to make all mozmill tests pass.
But at least xpcshell test, the nsITreeBoxObject mock is not necessary at all. 

This patch will make all xpcshell tests pass.

::: mailnews/base/src/nsMsgDBView.cpp
@@ +2559,5 @@
>    case nsMsgViewCommandType::collapseAll:
>      rv = CollapseAll();
>      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
>      SetViewFlags(m_viewFlags);
>      NS_ASSERTION(mTree, "no tree, see bug #114956");

This assertion should be also removed? This is the last assertion for mTree.
Comment on attachment 8508144 [details] [diff] [review]
Don't mock nsITreeBoxObject

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

r+=me but please incorporate hiro's suggestion to eliminate the extra assertion

::: mailnews/base/src/nsMsgDBView.cpp
@@ +2559,5 @@
>    case nsMsgViewCommandType::collapseAll:
>      rv = CollapseAll();
>      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
>      SetViewFlags(m_viewFlags);
>      NS_ASSERTION(mTree, "no tree, see bug #114956");

Yes I agree, those were debugging to find a crash a long time ago.
Attachment #8508144 - Flags: review?(kent)
Attachment #8508144 - Flags: review?(irving)
Attachment #8508144 - Flags: review+
(In reply to Kent James (:rkent) from comment #3)
> Comment on attachment 8508144 [details] [diff] [review]
> Don't mock nsITreeBoxObject
> 
> Review of attachment 8508144 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r+=me but please incorporate hiro's suggestion to eliminate the extra
> assertion
> 
> ::: mailnews/base/src/nsMsgDBView.cpp
> @@ +2559,5 @@
> >    case nsMsgViewCommandType::collapseAll:
> >      rv = CollapseAll();
> >      m_viewFlags &= ~nsMsgViewFlagsType::kExpandAll;
> >      SetViewFlags(m_viewFlags);
> >      NS_ASSERTION(mTree, "no tree, see bug #114956");
> 
> Yes I agree, those were debugging to find a crash a long time ago.

That comment was supposed to be added after hiro's comment, which is how they appeared in the review diff
https://hg.mozilla.org/comm-central/rev/ca3044b26a30
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 36.0
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.