-
-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get all unit tests passing #178
Comments
Updated stats as of today's commits:
|
Updated stats as of today's commits:
|
@BigstickCarpet Thanks for the awesome work !! |
Wow, I really like this! Thanks for the awesome work! |
@BigstickCarpet Correct me if i'm wrong but Windows Phone 8.1 doesn't have IndexedDb or WebSQL support for files that are loaded from the local file system, am I right? So how can it be that 93 tests are passing? Are you using a SQLite database? And if so, is it on github? Thanks for the great work, keep it up :) |
@nimrodxx - IE Mobile on Windows Phone 8.1 natively supports IndexedDB. But in a Cordova app, it requires the IndexedDB plugin, which uses IndexedDBShim (this project) and the WebSQL plugin internally |
@BigstickCarpet Yeah, I already thought you would came up with that solution. However as pointed out in the issues of the WebSQL plugin. It performs very bad and it is completely sync where the IndexedDb and WebSQL interfaces are async. So that leaves a difference in the interface and most important the user experience. But wait, @DickvdBrink and I managed to get an async variant of the WebSQL plugin to work. Hopefully this fixes some unit tests. We are persuading our company to opensource it. Ill keep you in touch. |
@BigstickCarpet, if I recall correctly the WebSQL plug-in returns true\false (or 1\0 cant really remember) while a string was expected for storeProps. The code I'm talking about is: Causing this code to fail: https://github.com/axemclion/IndexedDBShim/blob/be681b1d61101bcf08a4c6ac4bf0d466d94f0e1c/src/IDBObjectStore.js#L157 Hopefully I can check this soon (or maybe @nimrodxx can do it because we work on the same project :)) |
@DickvdBrink & @nimrodxx - Thank you guys for helping knock out a few bugs, especially with the WebSQL plug-in, which I am not very familiar with. I just discovered a new issue with the WebSQL plug-in today. It's not throwing errors when primary key constraints are violated. Sounds like this bug, except that bug was supposedly fixed back in 2012. So I'm not sure what's up. Anyway... here are the new numbers, as of today's commits:
|
@BigstickCarpet how do you run your test on Windows Phone 8.1? Because your test app (https://github.com/BigstickCarpet/cordova-test-app) opens the default browser on our device and all tests pass in that browser. |
@nimrodxx - Yeah, unfortunately Windows Phone 8.1 doesn't allow remote code execution inside a Cordova app, so my Cordova Test App only really works for iOS and Android. To test on Windows Phone 8.1, I just created a Cordova app, added the WebSQL plugin, and then copied the tests directory into the |
Here are the new numbers, as of today's commits:
|
DONE!!!All 269 Mocha unit tests and 30 QUnit tests are now passing on every desktop and mobile platform that I have available to test: There are some caveats though:
|
Wow this is really nice!! Again a big thank you! :) |
Hopefully they'll let you publish it. Open-source only works when it goes both ways. I wish more companies understood that. |
@BigstickCarpet @DickvdBrink Just received the green light to opensource our async WebSQL plugin. We are planning to release it asap. I'll keep you posted. |
👍 That's great news! We will almost certainly end up using it at my company, since we target Windows Phone as our primary platform. Will you be publishing it as a separate WebSql plugin? Or are you planning to submit a PR and get your fixes merged into the MSOpenTech plugin? |
Well I think that a separate WebSql plugin is the way to go, since we have changed almost every line of code. But I will have to discuss that with @DickvdBrink |
@BigstickCarpet Hey we just released the WebSQL plugin. We didn't have time to test it with the new version of the shim but we will pick it up soon. |
Badass! Looking at it right now. Might have it integrated into my company's mobile apps by the end of the day. Thanks for contributing it! |
This is totally awesome to see this issue fixed. Really great job, thank you very much. |
I'm not sure if the plugin works right now because I remember making some changes in the shim to make it work. We might need to incorporate them in this shim. But it is a start :) |
Yep. Working on that now. It'll require some changes to the shim and to your plug-in. I'll submit a PR |
@nimrodxx Have you guys published the plugin on npm ? Note that Cordova plugins are moving to NPM. |
@axemclion no we haven't because we need to do some more testing and check what additional changes are required on the shim (we have a slightly modified version). After more testing/documentation it will be published on npm with the required keywords :) I'm on the cordova-list so I know about the changes for npm. edit noticed the fork with the package.json! Nice :) |
@axemclion @DickvdBrink - As we discussed a couple weeks ago, I've added a bunch of new unit tests (221 of them!) to the project. These tests are in a separate directory from the QUnit tests and are not run as part of the CI process, since almost all of them fail when run with IndexedDBShim. The goal is to always have a working CI process with passing tests, so the QUnit tests are the official test suite. As we fix bugs in IndexedDBShim and get the Mocha tests passing, we can convert them to QUnit and add them to the official test suite.
I've tested on every browser, platform, and device that I have available, and here are the results:
Browsers with Native IndexedDB
Browsers without Native IndexedDB (using IndexedDBShim)
So, now to get all those tests passing! :)
The text was updated successfully, but these errors were encountered: