Closed Bug 422872 Opened 14 years ago Closed 14 years ago

fix test in test_contentAreaUtils.js for both suite and toolkit

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Callek, Unassigned)

References

Details

Attachments

(1 file)

Attached patch fixSplinter Review
No description provided.
Attachment #309345 - Flags: review?(sayrer)
Attachment #309345 - Flags: review?(sayrer) → review+
Checking in suite/common/tests/unit/test_contentAreaUtils.js;
/cvsroot/mozilla/suite/common/tests/unit/test_contentAreaUtils.js,v  <--  test_contentAreaUtils.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/content/tests/unit/test_contentAreaUtils.js;
/cvsroot/mozilla/toolkit/content/tests/unit/test_contentAreaUtils.js,v  <--  test_contentAreaUtils.js
new revision: 1.2; previous revision: 1.1
done
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Why was the change to toolkit/content/tests/unit/test_contentAreaUtils.js needed?
gavin, the attached patch actually was partially incorrect; suite had the same change as toolkit for this bug (Suite's version 1.1 matched that of toolkits except used the suite contentAreaUtils chrome url.

the problem was, the test was checking to ensure that we do actually throw an exception in the case provided, but |do_throw| was still in a try block, thus it would never actually fail in that case.

I noticed the issue shortly after checking in the test for suite, and figured fixing toolkits test at the same time would be a good thing.
after some further IRC discussion, we learned that my fix here was unnecessary. |do_throw| will always cause the test to fail, and will cause the test to exit.

Since these tests will still fail with my patch, it was also decided a backout really is not needed either. (just commenting incase anyone looks back at this bug)
You need to log in before you can comment on or make changes to this bug.