Supporting PhantomJS for running tests #167
Conversation
Had to remove the wait for close in `query` because PhantomJS seemed to break Moved JS that is shared between karma runner and browser runner into separate files Added some helpers for phantomjs
|
@brettz9 I've got PhantomJS tests running now as part of the build now, but as you can see from the last test run it failed. I can repo the failure locally and it's because these two expect(ev.oldVersion).to.equal(initialVersion);
expect(ev.newVersion).to.equal(null);I'd say there's something going amiss in the |
|
Ok so I'm thinking:
I'm leaning towards the former |
|
It's not a bug in our code; it's a bug in Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1220279 . It works fine in Chrome. Those checks are in |
|
|
||
| req.onblocked = function () { | ||
| console.log('db blocked', arguments); | ||
| }; | ||
| }); |
There was a problem hiding this comment.
Is there a reason you removed done in this block here? I think deleteDatabase might not always immediately complete its action which could be a problem if the next test depends on the database being deleted.
There was a problem hiding this comment.
for some reason this particular one was causing phantomjs to fail. Given that the tests generate a unique DB name each time (a guid) there's less a need to wait for it to finish.
There was a problem hiding this comment.
I'd think it should still work with done() in onerror or onblocked, but gotcha re: unique DB name...
There was a problem hiding this comment.
yeah I tried adding those calls but nothing was working
|
Regarding the FF bug - sure, but one of the things I tried to solve in db.js at the start was to normalise the quirks across browsers (back when there was the Now there's less quirks (except IE/Edge not supporting multi-key indexes) but something like this I'd like to normalise down where possible, so if I can patch it in db.js's core then that meets that goal. |
|
Oh, patching it within |
|
Check out 2e091af it has the patch I'm talking about in there. |
|
@MartijnR you might be interested in what's happening here. |
|
Yeah, that's cool. Maybe a comment would be nice with the bug link though in case this gets fixed in the future and therefore no longer any need to keep patching. |
Supporting PhantomJS for running tests
This should close #113