scottohara / tvmanager

PWA for tracking recorded, watched & upcoming TV shows
MIT License
3 stars 0 forks source link

database-controller test suite introduces openDatabase global #50

Closed scottohara closed 6 years ago

scottohara commented 11 years ago

With the QUnit noglobals flag enabled, the test/controllers/database-controller suite is marked as introducing window.openDatabase as a global object.

At first this seems odd, since window.openDatabase is native code. The test suite simply redefines it in the module setup, eg.

this.originalOpenDatabase = window.openDatabase;
window.openDatabase = function() { ... };

...and restores the original in the module teardown, eg.

window.openDatabase = this.originalOpenDatabase

In other places where we've temporarily mocked properties of the global window object, we've avoided introducing globals by deleting the introduced property (eg. delete window.confirm;) rather than replacing it with the original version.

We can't do this in the database-controller, because of the fact that this all happens in setup/teardown. Since moving to require.js + the latest QUnit version, the internal queue in QUnit no longer needs be in a specific order. Previously it was the case that the QUnit queue looked like this: setup (1) -> test (1) -> teardown (1) -> setup (2) -> test (2) -> teardown (2) -> setup (3) -> test (3) -> teardown (3) ie. setup/teardown functions were run immediately before/after the test, with no chance of any overlaps between tests.

Now, whether due to asynchronous script loading by require.js, or a change to QUnit's internals, the queue might look like: setup (1) -> setup (2) -> test (1) -> teardown(1) -> setup (3) -> test (2) - > teardown(2) -> test (3) -> teardown (3)

If we did a delete window.openDatabase in the teardown function, it would mean that after teardown(1), the openDatabase property would be restored to the real version; and since setup (2) has already executed at that point, there's no opportunity to restore the patched version before test (2) executes. The only workaround would be to apply the openDatabase mocking to every single test (which kind of defeats the purpose of having setup/teardown functions).

Alternatively, issue #49 may assist, by not having to touch the global window object at all (instead passing a mock version of 'window' into the module under test).