IndexedDBShim icon indicating copy to clipboard operation
IndexedDBShim copied to clipboard

Order results correctly when using getAll() and getAllKeys()

Open dfahlander opened this issue 7 years ago • 12 comments

This will hopefully resolve the ordering issue listed in https://github.com/dfahlander/Dexie.js/issues/709

https://github.com/dfahlander/Dexie.js/issues/701

as referred from https://github.com/axemclion/IndexedDBShim/issues/325.

dfahlander avatar May 30 '18 14:05 dfahlander

Seems the travis tests failed. Was not able to test locally either. What I did for testing was building the shim and run the dexie tests on it. Still, there are failing tests, but many went from failed to OK. Am I supposed to commit the files in dist together with the source? Would appreciate any help in contributing to IndexedDBShim.

dfahlander avatar May 30 '18 14:05 dfahlander

This is great, thanks! Can you point me to the particular Dexie tests failing for this case if it is something that could, without too much trouble, map to a vanilla IDB test?

brettz9 avatar May 31 '18 05:05 brettz9

And I believe the Travis tests may always fail on the forks...

brettz9 avatar May 31 '18 05:05 brettz9

@brettz9 I think both Dexie and IndexedDBShim would benefit from integrating the dexie test suite with IndexedDBShim applied - both in browser and on node. This is what @aral is up to. I plan to make a test config in dexie that runs all tests with the shim applied shim applied (one config for using the shim in browser and one for using it on node).

Since there seems to be several tests that fails with the shim, I believe it's not worth the effort to rewrite them to use vanilla indexeddb testing. This could also become a more sustainable road forward, if it will be easy to the the shim in dexie tests and get regression tests in all future versions.

I think me and @aral is on this right now, and we'll try finding and fixing issues in best-effort. Let's see what comes out from that, as a first step. I'll let you know if we need more help.

dfahlander avatar May 31 '18 08:05 dfahlander

After the last commit, another 2 assertions make it through in the dexie tests. The last commit was that IDBIndex.getAll() should not just ORDER BY the queried key, but also on primary key.

dfahlander avatar May 31 '18 10:05 dfahlander

With your PR I'm now getting failures on 'idbindex_getAll' and 'idbindex_getAllKeys', two W3C web-platform-tests tests that we were not getting failures on previously.

Btw, I just updated master with a fix to Grunt for testing and to the testing Docs.

The tests you want to run are npm run w3c, but you must first follow the installation instructions at https://github.com/w3c/web-platform-tests/ (wasn't too difficult).

brettz9 avatar Jun 01 '18 01:06 brettz9

@brettz9 thanks for guiding in how to run the tests. I'm trying to setup the test environment to debug it.

dfahlander avatar Jun 01 '18 08:06 dfahlander

I've got no luck running the w3c tests. Installed the WPT according to their instructions. Then, the instructions in docs/TESTING did not guide me in how to proceed from there exactly, but as I had installed web-platform-tests in its own location, i symlinked IndexedDBShim/web-platform-tests into that location. Then, I tried both npm run w3c-add-wrap followed by npm run w3c (according to your instructions), but no luck so ran npm run w3c-wrap but still no luck:

missing?:http://localhost:9999/node_modules/babel-polyfill/dist/polyfill.js
missing?:http://localhost:9999/dist/indexeddbshim-noninvasive.js
evalmachine.<anonymous>:3526
    setGlobalVars(null, {
    ^

ReferenceError: setGlobalVars is not defined

dfahlander avatar Jun 01 '18 09:06 dfahlander

Sorry, I can't figure out why I sometimes don't seem to get notifications from Github issues...

The relevant section of the testing page is no. 4 of https://github.com/axemclion/IndexedDBShim/blob/master/docs/TESTING.md#node-testing

After web-platform-tests are running (npm run web-platform-tests), in a separate process run npm run w3c.

The wrap tests are so you can test IndexedDBShim in the browser using their tests.

brettz9 avatar Jun 05 '18 09:06 brettz9

The page at https://github.com/axemclion/IndexedDBShim/blob/master/docs/TESTING.md now has additional info on the now headless testing one can do through the web-platform-tests library (their new recommended way of doing browser testing): ./wpt run --headless chrome IndexedDB.

However, if you'd rather avoid setting up the testing environment, you could just extract the JavaScript within the relevant tests at https://github.com/web-platform-tests/wpt/blob/master/IndexedDB/idbindex_getAll.html and https://github.com/web-platform-tests/wpt/blob/master/IndexedDB/idbindex_getAllKeys.html

brettz9 avatar Dec 10 '18 14:12 brettz9

Though I forgot those had some dependencies you'd have to grab too...

brettz9 avatar Dec 10 '18 14:12 brettz9

In master, I have now installed npm-run-all to allow parallel processes, so it should be easier now if you might be able to revisit this (running npm run w3c).

brettz9 avatar Mar 17 '20 10:03 brettz9