New testing framework (FLUID-4850 and FLUID-4886) - and a QUnit bug

[Antranig Basman]

All of the changes to the testing framework for Infusion have now been pushed to trunk - thanks to yzen and 
michelled for great review.

As described in the outline JIRA, the main changes are to move the jqUnit API over to a constructor-free 
form that more clearly mirrors the real semantic of the underlying QUnit API:

http://issues.fluidproject.org/browse/FLUID-4886

Every person who writes new test cases will now have to use the following form (all existing test cases in 
Infusion's image have already been update)

In the past, where you would write

var myTestCase = new jqUnit.TestCase("My Test Cases");

myTestCase.test("My Test Case", function () ....


you will now write


jqUnit.module("My Module");

jqUnit.test("My Test Case", function () .....


All our existing testing utilities have been rationalised, so for the most part now you should ONLY ever be 
using methods on the jqUnit namespace when writing your tests - the global definitions with which QUnit 
polluted the global namespace have been removed. This includes directives like jqUnit.expect, jqUnit.start, 
jqUnit.stop, etc. The definitions which were previous exposed as part of fluid.testUtils.* are now also on 
jqUnit.* - e.g. jqUnit.assertLeftHand, jqUnit.assertNode, etc. Note that the namespaced QUnit global is 
still exposed for the rare cases when you need to drop into raw QUnit. These is one important case for this 
which will now be described.

Comparing Deep Equality in jqUnit/QUnit
=======================================

The question of how to compare deep object trees for equality is somewhat perplexed in QUnit. The default 
algorithm which is used is quite powerful, but has a few features which IMO are inappropriate, especially in 
environments such as Node.js where stability of built-in constructors can't  be relied upon - for example, 
every NPM "context" will have its own instances of Array, Object, etc. and so checks like "x instanceof 
Array" can't be relied upon to be effective - this is exactly the same issue that we face when transporting 
JS object between iframe boundaries in the browser. This is the reason for our specialised utilities 
"fluid.isArrayable" etc. which rely on schemes which have already been devised for this purpose in jQuery.

In general in Fluid, we are much more interested in JSON-based structural equality of trees, rather than 
verifying some kind of "exact prototypical correspondence". The QUnit team recently responded to this 
requirement with discussion on https://github.com/jquery/qunit/issues/279 - recent version of QUnit expose a 
new method called "propEqual" which operates structural comparison of this type. One of the updates that are 
in the recent merge are to forward all calls from jqUnit.assertDeepEq onto this new QUnit.propEqual method 
rather than QUnit.deepEqual as before.

This means that in almost all cases, including in Node.js and cross-iframe situations, the semantics of 
comparison are just what we would want and there is no need to think about the choice of jqUnit.assertDeepEq 
which carries on working as before. However, there are some issues with the QUnit implementation which mean 
that in some special cases you will need to tread carefully. These issues all, in one way or another, 
related to comparing structures which may have circular linking within them.

For a start, the implementation of QUnit.propEqual is much less sophisticated than QUnit.deepEqual - in 
fact, it amounts to operating a fluid.copy on both arguments given to QUnit.propEqual and then passing them 
over to QUnit.deepEqual. In particular, in the case these arguments have circular linkage, this will cause a 
stack overflow and bomb the test - and in Firefox at least, this stack overflow will bomb the entire browser 
too.

Normally supplying a circularly linked structure to assertDeepEq represents an error in the first place - 
but there are a few cases when arguably it doesn't. For example, many DOM nodes hold such circularly linked 
structures - especially on Safari. TableOfContents.js held a test case which wanted to compare one array of 
DOM nodes for equality with another, and when passed to the new jQUnit.assertDeepEq -> QUnit.propEquals, 
this will bomb the test. This is one of the cases when the appropriate test is to drop back into raw QUnit 
and issue a QUnit.deepEqual test - I updated this test to that form.

However worse, there is also a BUG. It turned out that part of the "clever" old implementation of 
QUnit.deepEqual was some code dedicated to specifically handling the cases of circular linkage. It is 
because such code is missing in the new QUnit.propEquals driver that it is now possible for circular linkage 
to bomb the test case. However, this deepEqual code, whilst it assured that the tests would not bomb, did 
not succeed in providing a consistent semantic for comparison - this resulted in that ultimate horror in a 
test framework, the possibility for "false positives". In our call yesterday we talked over the worry of 
potential false positives - that is, tests which pass due to an error in the testing framework which should 
not pass - and it turns out there is just such a thing in QUnit. In the case where the only difference 
between two JSON structures consists of a circular linkage, QUnit.deepEqual will incorrectly compare them as 
the same. To some extent, the new default behaviour of bombing out the test case (and maybe even the 
browser) could be described as "helpful" since at least it signals there is a problem. I have reported the 
issue upstream at qunit as

https://github.com/jquery/qunit/issues/397

This advice boils down to - in cases where there might be circular linkage in the structures you are 
comparing (mostly, legitimately I can only think of DOM nodes), do consider dropping back into raw 
QUnit.deepEqual - BUT beware in this case that the circular linkage itself might lead to false positives in 
your tests. In all cases, think of other ways of expressing your tests that don't require comparing such 
structures and just use jqUnit.assertDeepEq instead. IF you find your tests bombing out, try testing them 
out in Chrome/Safari rather than Firefox, which have much more forgiving behaviours in the case of a stack 
overflow.