Skip to content
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

Feature to save and load a pattern in IndexedDB #35

Closed
kikuomax opened this issue Jan 30, 2020 · 16 comments · Fixed by #47
Closed

Feature to save and load a pattern in IndexedDB #35

kikuomax opened this issue Jan 30, 2020 · 16 comments · Fixed by #47
Assignees

Comments

@kikuomax
Copy link
Owner

kikuomax commented Jan 30, 2020

Where to save?

Here I want to provide an offline feature first.
So I will give IndexedDB a shot.

IndexedDB Structure

Database

AmidzDatabase

Object Store

pattern

  • keyPath: 'name'
  • value: Object
    • name: String
    • rows: Array.Object
      • columns: Array.Object
        • symbolId: String

A value object has the same structure as the patternData state of the pattern Vuex store.

@kikuomax kikuomax self-assigned this Jan 30, 2020
@kikuomax kikuomax mentioned this issue Jan 30, 2020
4 tasks
@kikuomax
Copy link
Owner Author

kikuomax commented Feb 5, 2020

This issue is not that simple.

@kikuomax kikuomax changed the title Interface to save a pattern Feature to save a pattern in IndexedDB Feb 6, 2020
@kikuomax
Copy link
Owner Author

kikuomax commented Feb 6, 2020

There is a simple wrapper for IndexedDB.

@kikuomax
Copy link
Owner Author

kikuomax commented Feb 6, 2020

As I guessed, I have to carefully handle IndexedDB when it is combined with Cypress.
cypress-io/cypress#1208

@kikuomax
Copy link
Owner Author

kikuomax commented Feb 10, 2020

Is it really the responsibility for the pattern store to save and load the current pattern.
An edit history (#45 and #46) will also be saved and loaded tied with the current pattern in the future.

@kikuomax kikuomax changed the title Feature to save a pattern in IndexedDB Feature to save and load a pattern in IndexedDB Feb 10, 2020
@kikuomax
Copy link
Owner Author

Is it really the responsibility for the pattern store to save and load the current pattern.
An edit history (#45 and #46) will also be saved and loaded tied with the current pattern in the future.

Who is responsible for managing an edit history by the way?

@kikuomax
Copy link
Owner Author

It looks the following code works.

beforeEach(function () {
  cy.visit('/')
  window.indexedDB.deleteDatabase('AmidzDatabase')
  cy.reload()
})

But is it guaranteed that window.indexedDB represents the IndexedDB of my application?
Or should I do something like below?

beforeEach(function () {
  cy.visit('/')
  cy.window()
    .then(window => window.indexedDB.deleteDatabase('AmidzDatabase'))
  cy.reload()
})

@kikuomax
Copy link
Owner Author

kikuomax commented Feb 10, 2020

cy.window is described in this file.

It is up to what is inside state('window').
https://github.com/cypress-io/cypress/blob/7efd9d8ab0db34424b2854a2ee3d1f1bf463ffee/packages/driver/src/cy/commands/window.coffee#L86

It is configured by the function setWindowDocumentProps.

@kikuomax
Copy link
Owner Author

kikuomax commented Feb 10, 2020

Tests occasionally fail unless I append cy.wait after cy.reload.
This should be because loading the AmidzDatabase needs some time.
It currently waits for 100ms but I do not know an appropriate duration.

@kikuomax
Copy link
Owner Author

By the way, I noticed that cy.visit yields a window.
Now beforeEach looks like the following,

beforeEach(function () {
  cy.visit('/')
    .then(window => {
      expect(window.indexedDB).to.exist
      window.indexedDB.deleteDatabase('AmidzDatabase')
    })
  cy.reload()
  cy.wait(100)
})

kikuomax added a commit that referenced this issue Feb 11, 2020
- A pattern is experimentally loaded from the IndexedDB.
  The root and `pattern` stores require a Promise that will be resolved
  into an IDBDatabase object.
- An IDBDatabase is opened in `src/index.js`.
- The `editor-container` component triggers a `loadCurrentPattern`
  action of the `pattern` store.

issue #35
kikuomax added a commit that referenced this issue Feb 11, 2020
- The `pattern` store has a new action `saveCurrentPattern` that saves
  the current pattern to the IndexedDB.
- The `pattern-editor` component saves the current pattern by calling
  the `saveCurrentPattern` action every time the current pattern is
  edited.

issue #35
kikuomax added a commit that referenced this issue Feb 11, 2020
- Since the last commit the current pattern is saved in the IndexedDB as
  an `AmidzDatabase` database. So E2E tests became unstable because the
  application remembers the last pattern. To fix this, the
  `AmidzDatabase` database is deleted at the beginning of every test.
  The default pattern stored in the `AmidzDatabase` is now same as the
  default value of the `patternData` state of the `pattern` store.

issue #35
kikuomax added a commit that referenced this issue Feb 11, 2020
- The default pattern is given by a single blank cell.
- Tests occasionally fail unless `cy.wait` is appended after
  `cy.reload`. It currently waits for 100ms but I do not know an
  appropriate duration.
- `cy.window` calls are removed since `cy.visit` yields a window.
  The database deletion code is moved to the `then` function following
  `cy.visit`.

issue #35
kikuomax added a commit that referenced this issue Feb 11, 2020
kikuomax added a commit that referenced this issue Feb 11, 2020
@kikuomax
Copy link
Owner Author

kikuomax commented Feb 11, 2020

I am still facing irreproducible tests failure especially when they are executed with cypress:run.

kikuomax added a commit that referenced this issue Feb 11, 2020
- Ugly workaround.

issue #35
@kikuomax
Copy link
Owner Author

How about to wait for the pattern loaded from IndexedDB before rendering the editor component.

@kikuomax
Copy link
Owner Author

How long does cy.reload wait?

@kikuomax
Copy link
Owner Author

How long does cy.reload wait?

It looks cy.reload waits for a load event.

kikuomax added a commit that referenced this issue Feb 11, 2020
- A loading spinner is shown until the pattern that was edited last time
  is loaded from the database. When the pattern is loaded, the loading
  spinner is hidden and a class `is-ready` is added to the editor
  container component. The `is-ready` class facilitates E2E tests to
  know when the pattern is actually loaded.

issue #35
kikuomax added a commit that referenced this issue Feb 11, 2020
kikuomax added a commit that referenced this issue Feb 11, 2020
- The loading spinner is checked disappeared at the beginning of each
  test case. This should make the test results more stable.

issue #35
issue #47
@kikuomax
Copy link
Owner Author

Why don't you flush the database at afterEach?

@kikuomax
Copy link
Owner Author

You missed a critical thing versionchange.
You have to close the database when a versionchange event is fired.

That is why your deleteDatabase does not fire onsuccess.

https://stackoverflow.com/questions/35137234/why-does-calling-indexeddb-deletedatabase-prevent-me-from-making-any-further-tra

@kikuomax
Copy link
Owner Author

Why don't you flush the database at afterEach?

By the way, this would be better because you do not need to call cy.reload.

kikuomax added a commit that referenced this issue Feb 11, 2020
- A `versionchange` event was not handled by `AmidzDatabase`, that is
  fired when a database is going to be deleted by `deleteDatabase`.
  `deleteDatabase` does not fire a `success` event until the database
  is closed. This is why tests timed out if a `success` event was
  expected. So I did not wait for `deleteDatabase` to fire a `success`
  event at that time. This can be a cause of E2E test instability.

  Now `AmidzDatabase` is closed when a `versionchange` event is fired.

  And by moving `deleteDatabase` call to `afterEach`, `cy.reload` is no
  longer necessary.

issue #35
issue #47
kikuomax added a commit that referenced this issue Feb 11, 2020
- `AmidzDatabase` is closed when `deleteDatabase` is triggered.

issue #35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant