dumbmatter / fakeIndexedDB

A pure JS in-memory implementation of the IndexedDB API
Apache License 2.0
562 stars 69 forks source link

Unable to cover openCursor onSuccess scenario while testing #60

Closed BrinsterAman closed 3 years ago

BrinsterAman commented 3 years ago

We are using fake-indexedDB for our app. We are unable to cover the openCursor().onSuccess scenario, though we are writing test case the same way it is displayed in your npmjs.com page. Can you please suggest how to cover these lines. We are using React testing library and Jest.

dumbmatter commented 3 years ago

Can you provide a test case showing the problem?

BrinsterAman commented 3 years ago

Yes sure,

tx.objectStore("test-key").index("contextLabel").openCursor().onsuccess = function (event) { var cursor = event.target.result; if (cursor) { console.log("From cursor:", cursor.value); cursor.continue(); } }; this is what I am using in my test case, as suggested by your npmjs page. But it is still not covering the lines.

dumbmatter commented 3 years ago

Thanks... but how about a complete script I can actually run? I ask because I am skeptical such a core feature is just completely broken. There are a ton of tests and a lot of people using fake-indexeddb. So it's likely some subtle issue specific to precisely how you're using it. Same for #59

icorbrey commented 3 years ago

I'm getting a similar error using the default React testing library (i.e. react-scripts test on a CRA app). When we initialize a cursor using FDBFactory it does not seem to run cursors at all. See following:

// In tests before execution
indexedDB = new FDBFactory();

// In implementation
async function Migration(transaction) {

    const albumCursor = await transaction
        .objectStore('album')
        .index('albumId')
        .openCursor();

    console.log(albumCursor); // FDBCursorWithValue { ... }

    if (albumCursor) {
        albumCursor.onsuccess = function (event) {
            const cursor = event.target.result;
            if (cursor) {
                console.log('Got a value!'); // Never prints
            }
            else {
                console.log('Finishing up!'); // Never prints
            }
        }
    }
}

I would expect some sort of error to occur if we had set this up wrong but nothing happens.

dumbmatter commented 3 years ago

@icorbrey can you provide a self-contained example that I can run to see the problem? Without that it'd be a lot of guesswork for me to try to figure out what you're doing that is triggering a bug. There's already a lot of automated tests covering the basic functionality of cursors, so if there is a bug, I imagine it is some subtle interaction related to the specific way you're using it.

icorbrey commented 3 years ago

Sure. Please note that idb() is simply getting a reference to our IndexedDB instance, you'll need to graft an existing IDB instance on. Additionally, album and photo are both tables in our schema.

describe('The fake-indexeddb cursor', () => {
    beforeEach(() => {
        indexedDB = new FDBFactory();
    });

    it('runs', async () => {
        const db = await idb();
        const cursor = db
            .transaction(['album', 'photo'], 'readwrite')
            .objectStore('album')
            .index('albumId')
            .openCursor();

        let didRun = false;
        cursor.onsuccess = function () {
            didRun = true;
            console.log('Ran!');
        };

        expect(didRun).toBeTruthy();
    });
});
dumbmatter commented 3 years ago

That test will always fail because onsuccess is executed asynchronously.

I added a unit test containing a working version of this code in commit c15b096d81e4c1fbb4ee4cc02065f10a49ee6340

You can run it with yarn run build && yarn run test-mocha. It does indeed work. Closing this issue for now, I don't think there is a bug here, but feel free to provide more information if anyone is still having trouble.

icorbrey commented 3 years ago

Of course it runs asynchronously, but the issue is that it doesn't run. In our tests, on success is never actually called

dumbmatter commented 3 years ago

Okay, but like I said above, I tried filling in the blanks in your code in commit c15b096 and it works fine. Maybe there is something else going on that you did not include in your partial code? Not much more I can say without a complete example.

icorbrey commented 3 years ago

Valid. I'll do some more internal testing and see if I can isolate the behavior

icorbrey commented 3 years ago

Quick update! I've been trying to debug this issue assuming that we're using the native IndexedDB interface, while lamenting that we're not using the idb package internally. However, I dug deeper and discovered that yes, we are in fact using the idb package! Once I figured that out I was able to properly implement this in about 15 minutes lol.

TL;DR: Don't use oncomplete when you're supposed to be using an async/await interface.

Charith47 commented 2 years ago

Quick update! I've been trying to debug this issue assuming that we're using the native IndexedDB interface, while lamenting that we're not using the idb package internally. However, I dug deeper and discovered that yes, we are in fact using the idb package! Once I figured that out I was able to properly implement this in about 15 minutes lol.

TL;DR: Don't use oncomplete when you're supposed to be using an async/await interface.

Same thing happened to me when using idb package. I used below method instead of onsuccess event and it worked.

        const db = await dbPromise(); // opens db
        const tx = db.transaction(storeName, 'readonly');
        const store = tx.objectStore(storeName);
        let cursor = await store.openCursor();

        while (cursor) {
                        // logic
            cursor = await cursor.continue();
        }