1118648 - MockHelper: `delete` the original property before overwriting it with mock and remove IndexedDB mock workaround

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[Tim Guan-tin Chien [:timdream] (please needinfo)]

See bug 1115275 for discussion and especially bug 1115275 comment 1 for the solution.

The lines to modify is here:
https://github.com/mozilla-b2g/gaia/blob/987c76c002a716a6787bc0576f0dd0f25fae25e4/shared/test/unit/mocks/mocks_helper.js#L56-L57

The workaround to remove is here:
https://github.com/mozilla-b2g/gaia/blob/d55d1d5f39bd338ab2d298823aab1121e3d73fef/apps/keyboard/test/unit/mock_indexeddb.js#L210-L215

We would need to update the files that use MockIndexedDB too.

Comment 1

[Giovanny Gongora [:gioyik]]

Hi, I would like to work on this issue. Seems to be not complicated. Could you (Tim) give me more info about what is needed to solve this bug?

Comment 2

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Hi Giovanny!

To take this bug first you would need to learn how to build Gaia and run Gaia unit tests, see
https://github.com/mozilla-b2g/gaia#unit-tests

To complete the changes suggested in comment 0 you need to understand how MockHelper works and make it work with MockIndexedDB. |grep| the code and find the call sites in the tests can help you on that greatly. 

One thing to note is that in the PromiseStorage tests I create one MockIndexedDB instance per test -- your change that makes the code call MockHelper must do that as well.

Thanks!

Comment 3

[Giovanny Gongora [:gioyik]]

Hi Tim,

I deleted the workaround in "apps/keyboard/test/unit/mock_indexeddb.js". But I have some questions about point 2. On "shared/test/unit/mocks/mocks_helper.js" you point some lines but I am not sure what or how I should change them.

Added to that I should replace MockIndexedDB with MockHelper in only keyboard app? Because I am seeing some cases in communications and verticalhome apps.

Comment 4

[GitHub Autolander Bot]

Attachment: [gaia] Gioyik:bug-1118648 > mozilla-b2g:master

Comment 5

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to Giovanny Gongora [:gioyik] from comment #3)
> Hi Tim,
> 
> I deleted the workaround in "apps/keyboard/test/unit/mock_indexeddb.js". But
> I have some questions about point 2. On
> "shared/test/unit/mocks/mocks_helper.js" you point some lines but I am not
> sure what or how I should change them.
> 
> Added to that I should replace MockIndexedDB with MockHelper in only
> keyboard app? Because I am seeing some cases in communications and
> verticalhome apps.

Yes, I understand it's a bit confusing because we write tests in our own ways in Gaia tree. 

So there are two mocked IndexedDB APIs, and both of them comes with utility function to replace the real functions instead of the real API, and in bug 1115275 we learned we could use the delete operator to dis-attach it.

The todos in this bug is to

1) remove the helper functions in both mocks.
2) fixes the tests that uses these mocks,
2.1) since the keyboard test does not use MockHelper, adding the delete operator in the suiteSetup and replace the API as usual would do.
2.2) for other tests that uses MockHelper, we would need to add delete operator in the replacing helper function so it could successfully replace |window.indexedDB| with mock.

Comment 6

[BMO Automation]

Firefox OS is not being worked on