search

aaronpowell / db.js

db.js is a wrapper for IndexedDB to make it easier to work against
http://aaronpowell.github.com/db.js/
MIT License
818 stars 142 forks source link

Prototype PhantomJS as a test runner #113

Closed aaronpowell closed 8 years ago

aaronpowell commented 9 years ago

PhantomJS 2.0 supports indexedDB (from my initial reading). I'd like to see how the tests run in PhantomJS and whether that can be used for CI instead of SauceLabs.

aaronpowell commented 9 years ago

Hopefully this can fix #108

lygstate commented 9 years ago

Sounds good, may have a try, saucelabs is too slow.

lygstate commented 9 years ago

Now the CI works, except the Pull request.

aaronpowell commented 9 years ago

SauceLabs isn't slow, I just have to fix my account on there. And the PR didn't fix the build nor did it add PhantomJS support, that is blocked by https://github.com/travis-ci/travis-ci/issues/3225

lygstate commented 9 years ago

The PR fixed the build! It's just because only your pull request can running the SauceLabs unittest, I've already test locally!

lygstate commented 9 years ago

It' implemented by #108

aaronpowell commented 8 years ago

Sadly PhantomJS still doesn't support IndexedDB. I could rely on one of the IDB shims but that seems a little dodgy...

MartijnR commented 8 years ago

It does support it now (without shims). Oddly, version 0.10.2 works fine for me in PhantomJS 2.0.0. However, version 0.12.0 fails with an error:

NotFoundError: DOM IDBData/Users/martijnvanderijdt/Enketo/Code/enketo-express Exception 8: An operation failed because the requested data/Users/martijnvanderijdt/Enketo/Code/enketo-express object could not be found. (/var/folders/ng/dlcx_vv962g_kfzmttldhltr0000gn/T/82c49e6d590d0b39fabae986f211f0dd.browserify:2 <- node_modules/db.js/dist/db.min.js:1:0)

I'm not referring to the tests inside this repo, but a whole slew of my own tests of a layer built on top of db.js (so presumably it's the same).

Any thoughts what change in 0.12.0 might have caused this?

aaronpowell commented 8 years ago

Not that i can think of. Got a repo where I can see the tests?

MartijnR commented 8 years ago

These are the tests: https://github.com/kobotoolbox/enketo-express/blob/master/test/client/store.spec.js of this store.js file.

However, I can try to create a failing phantomjs test in a port of db.js for easier demonstration, when I have time.

aaronpowell commented 8 years ago

I've just created a karma branch for my testing here, and yep I can repo the same problem.

Damn :cry:

MartijnR commented 8 years ago

ah! Well, the good news is that they'll probably pass in v0.10. I tried for a while to identify changes by looking at the code, but didn't see anything suspicious. It's probably a very subtle change.

aaronpowell commented 8 years ago

Been digging more into this and I'm actually getting closer.

The error that we're seeing is happening on this line: https://github.com/aaronpowell/db.js/blob/master/src/db.js#511

What's happening is that the for-in loop is going over a DOMStringList and then the hasOwnProperty method is called. Because the DOMStringList is empty it only has a single property name, length, but there is a bug in the implementation of hasOwnProperty that says "oh hey, that's part of the object", when really, it shouldn't be.

It eventually gets through to line 511 which it tries to remove an object store named length which doesn't exist and it falls over.

I'm currently trying to introduce a workaround when running in Phantom for that. I'm down to being able to run a 32 of the 83 tests. Woo!

MartijnR commented 8 years ago

That's great news! :+1:

aaronpowell commented 8 years ago

I've just pushed a branch where the tests run under PhantomJS :grinning:

MartijnR commented 8 years ago

Woohooo!! :shipit:

aaronpowell commented 8 years ago

:tada: