Code review comment for lp:~mars/launchpad/fix-js-unittests

Revision history for this message
Māris Fogels (mars) wrote :

On Wed, Dec 2, 2009 at 8:54 AM, Gavin Panella <email address hidden> wrote:
> Review: Approve
> Has Y.fail() been renamed as Y.error(), or is it a deprecated synonym? If neither, then I'd be inclined to leave it alone; "fail" more closely matches the terminology used in the Python testing world. Y.error() sounds like it should be used when test set-up fails, not when an assertion fails.

Yes, Y.fail() became Y.error(). Y.fail() is now only in the 'test' module, and has become the standard xUnit "fail()" call.

This caused a subtle bug: my tests were failing when they should have been raising errors. Turns out the code under test was calling Y.fail() itself, so it raised a failure exception instead of a real error.

>
> I'm very happy to see that the 'click' simulation has been fixed (re. test_me_too.js). Figuring that one out caused me a very sweary afternoon of pain.

It took me a while, too. I remembered a comment Bjorn made during the lazr-js sprint about using click because mousedown was unreliable. Turns out it was true!

>
> Thanks for doing this :)
>

My pleasure! Thank you for the review.

Mars

« Back to merge proposal