949491 - DataStore not supported when running Gaia in Firefox

Status: RESOLVED WONTFIX

(Firefox OS Graveyard :: Gaia, defect)

Description

[Ben Francis [:benfrancis]]

I think we need to add some more magic to the Gaia build system to add prefs and permissions for DataStores specified in app manifests.

Currently DataStore won't work in Firefox.

As I understand it this means we also can't write automated tests that use DataStore because Travis runs unit tests against Firefox rather than B2G Desktop.

Comment 1

[Dale Harvey (:daleharvey)]

Attachment: https://github.com/mozilla-b2g/gaia/pull/14764

Comment 2

[Julien Wajsberg [:julienw]]

Comment on attachment 8348766 [details] [review]
https://github.com/mozilla-b2g/gaia/pull/14764

This is more Alexandre's field, redirecting :)

Comment 3

[Dale Harvey (:daleharvey)]

https://github.com/mozilla-b2g/gaia/commit/ff9075bd8f68b1729ef3bebd10d906829fbc8b60

Comment 4

[Dale Harvey (:daleharvey)]

This has regressed, currently we can test datastore with DESKTOP=1 but not DESKTOP=0, it seems that datastore only works if we install the screen reader extension which is a strange situation to be in.

kgrandon is taking a look at the failing tests (with DESKTOP=1), for the short term I think we need to revert the DESKTOP=0 change and disable the tests that causes to fail (+fix those tests)

Ultimately I think out unit test infrastructure is fundamentally broken, its 18 months in and we still cant run |make unit-tests| our tests mock all sorts of functionality that we are supposed to be testing, we are depending on arbitrarily built profiles in order for our tests to run which is just plain wrong, tests should be able to define the capabilities they require in order to run, we have a solid framework inside gecko with mochitests and I am hoping it will be possible to start writing gaia 'unit' tests with that.

(I quoted unit since they arent strict unit tests, but somewhere in between integration tests)

sound like a plan?

Comment 5

[Dale Harvey (:daleharvey)]

https://github.com/mozilla-b2g/gaia/pull/15081/files is the related PR thats attempting to test datastore

Comment 6

[Kevin Grandon :kgrandon]

So I really want to fix unit tests using a DESKTOP=1 build, and move towards unifying DESKTOP and DEBUG flags. (I don't think there needs to be two as they cause confusion). Even with this fixed though, it's still a very small improvement.

Mochitests would be an interesting experiment, but I'm not sure if it makes sense for us to focus on that right now. 

Regarding the commit with failing tests: I might suggest to either use mock datatore (sucks), or mark it with test.skip() for now (also sucks). We can open up a bug to re-enable them after bug 957923 is resolved.

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to Dale Harvey (:daleharvey) from comment #4)
> This has regressed, currently we can test datastore with DESKTOP=1 but not
> DESKTOP=0, it seems that datastore only works if we install the screen
> reader extension which is a strange situation to be in.
> 
> kgrandon is taking a look at the failing tests (with DESKTOP=1), for the
> short term I think we need to revert the DESKTOP=0 change and disable the
> tests that causes to fail (+fix those tests)
> 
> Ultimately I think out unit test infrastructure is fundamentally broken, its
> 18 months in and we still cant run |make unit-tests| our tests mock all
> sorts of functionality that we are supposed to be testing, we are depending
> on arbitrarily built profiles in order for our tests to run which is just
> plain wrong, tests should be able to define the capabilities they require in
> order to run, we have a solid framework inside gecko with mochitests and I
> am hoping it will be possible to start writing gaia 'unit' tests with that.
> 

Actually I disagree with most of it, but you know this since we already discussed of it and I don't know whether we finally can agree on something about this ;)

So here is my take:

* yes, a full "make test-agent-test" should be fixed, I'm fixing the test-agent one bug at a time. Especially, there is a bug about running each test file in its own sandbox, which (I hope) will make the tests behave in a more sane way. Also I'm very close to fix bug 892048 which is a major frustration for me, and maybe for you as well, and adds to the mess. I'm very close to fix tests-in-b2g-desktop as well.

* I still believe _unit_ tests should generally _not_ use the APIs and should mock them. This is necessary when you want to test the Gaia code for everything an API might do and return. That said, I'm open with using some of them (indexedDB comes to my mind, DataStore is also probably better to use directly).

* That's also the reason I think unit tests should run in a "as bare as possible" environment, and why they should run with DEBUG=1 DESKTOP=0. That said, they should not fail in DESKTOP=1 and we should rightly fix this. For this, I could be convinced though (especially that unit tests should be able to run both in B2G Desktop and in Firefox).

* Bug 885544 should also be fixed. Part of the mess is the environment variable names we chose.


> (I quoted unit since they arent strict unit tests, but somewhere in between
> integration tests)

I don't understand why you're saying this, and this is maybe the root of our disagreement.

For me, unit tests are _real_ unit tests, and that's why they should not use and don't need the real APIs.  Gecko already as mochitests for the API so we generally don't need to tests the APIs in our unit tests.

However Integration tests (JS and Python) are testing the whole thing and _do use_ the real APIs.

Therefore, can you please explain why you think Gaia unit tests are not real unit tests ?

Comment 8

[Julien Wajsberg [:julienw]]

Just to sweeten my comment a bit: the unit test framework is definitely used in different ways by the different apps. I also definitely think that we should use it an a unit test way, but, well, this is only a tool, so if some app developers want to use it differently, then we should enable this in a reliable way, and my opinion about how tests should be done doesn't count.

About mochitests: I think the framework we have now is far superior to mochitests, for how tests are written. But it's probably lacking for how tests are run. I don't know whether it's possible to use mochitests with the existing unit tests, maybe it is.

Also don't forget that the test-agent provides something very useful: run the related test when you save a file. This is awesome and I don't want to lose this ability.

Comment 9

[Dale Harvey (:daleharvey)]

Closing in favour of 949491

Comment 10

[Dale Harvey (:daleharvey)]

And sorry, I meant https://bugzilla.mozilla.org/show_bug.cgi?id=959841