Last Comment Bug 759311 - Assertions in update tests have to be more verbose
: Assertions in update tests have to be more verbose
Status: RESOLVED FIXED
[lib][qa-]
:
Product: Mozilla QA
Classification: Other
Component: Mozmill Tests (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Henrik Skupin (:whimboo)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-05-29 05:56 PDT by Henrik Skupin (:whimboo)
Modified: 2012-08-14 15:04 PDT (History)
3 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---
unaffected
fixed
fixed
fixed
fixed
wontfix


Attachments
Patch v1 (11.29 KB, patch)
2012-05-29 06:03 PDT, Henrik Skupin (:whimboo)
dave.hunt: review+
Details | Diff | Splinter Review

Description Henrik Skupin (:whimboo) 2012-05-29 05:56:40 PDT
There are a couple of instances in the current test and lib code which make use of the old controller.assert() method. All of those have to be converted to use the assertions module. Otherwise it's kinda hard to figure out the reason for all the latest update test failures for localized builds.
Comment 1 Henrik Skupin (:whimboo) 2012-05-29 06:03:39 PDT
Created attachment 627931 [details] [diff] [review]
Patch v1
Comment 2 Dave Hunt (:davehunt) 2012-05-29 06:30:08 PDT
Comment on attachment 627931 [details] [diff] [review]
Patch v1

Looks good. I just have one general question regarding the following line:

expect.equal(button.getNode().hidden, false, "The update button is always visible even after an update.");

An alternative would be to use:

expect.ok(!button.getNode().hidden, "The update button is always visible even after an update.");

When checking something is false would we always use the former, or would there be times we'd use the latter?
Comment 3 Henrik Skupin (:whimboo) 2012-05-29 06:31:54 PDT
(In reply to Dave Hunt (:davehunt) from comment #2)
> An alternative would be to use:
> 
> expect.ok(!button.getNode().hidden, "The update button is always visible
> even after an update.");

Sorry, that's the right way to do it. Simply missed it to use ok(). Thanks for the catch.
Comment 4 Henrik Skupin (:whimboo) 2012-05-29 06:33:37 PDT
Landed as:
http://hg.mozilla.org/qa/mozmill-tests/rev/57f0c818570e

Once it has been proven to work, I will backport the patch to older branches.

Note You need to log in before you can comment on or make changes to this bug.