993992 - require/requireApp function should be sync, comparing to *_test.js and setup.js

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::TestAgent, defect)

Description

[John Hu [:johnhu][:johu][:醬糊小弟]]

If we use "require" function in normal *_test.js, the object is too slow to load and the object is undefined. But if we move the "require" function in setup.js, the object is loaded before *_test.js.

The MocksHelper is undefined in the following case:

in abcd_test.js:

require('/shared/test/unit/mocks/mocks_helper.js', function() {
   console.log('mocks_helper is loaded);
});

console.log('using MocksHelper');
new MocksHelper([]);

The MocksHelper is defined in following working case:

in setup.js

require('/shared/test/unit/mocks/mocks_helper.js', function() {
   console.log('mocks_helper is loaded);
});

in abcd_test.js

console.log('using MocksHelper');
new MocksHelper([]);

Comment 1

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

I don't want require/requireApp to be sync.

Actually, I want quite the contrary, see bug 943163.

Why don't you want to put the mocks helper in setup.js ?

Actually, I think the mocks helper should always be automatically added by the test-agent (like we do for sinon.js).

Comment 2

[John Hu [:johnhu][:johu][:醬糊小弟]]

Well, I just want to say the behavior of require/requireApp is different between they are in *_test.js and setup.js.

I had asked Ricky about this. He told me that the require/requireApp should be sync.

(In reply to Julien Wajsberg [:julienw] from comment #1)
> Why don't you want to put the mocks helper in setup.js ?
I met similar issue not only mocks helper. That's an example for easy understanding. In our new app, we have few utils objects whose mock-up object should be loaded.

> 
> Actually, I think the mocks helper should always be automatically added by
> the test-agent (like we do for sinon.js).

For mocks helper case, I agree with this one. We should load it at the start-up of test agent.

Comment 3

[John Hu [:johnhu][:johu][:醬糊小弟]]

BTW, I know we may use "callback function" at require and requireApp. But it looks a little bit ugly.

Comment 4

[Ricky Chien [:rickychien] (inactive)]

Test Agent's require/requireApp use async=false to load scripts.

https://github.com/mozilla-b2g/js-test-agent/blob/master/lib/test-agent/loader.js#L177

Actually, it's different between inserting script with async=false into plain HTML and dynamically create script with async=false in JavaScript. By being dynamically created, they’re executed outside of document parsing, so execution isn’t blocked while they’re downloading.

For this reason, we could not guarantee you can access object immediately after require/requireApp.

Now, our workaround is to put MockHelper or similar stuffs into setup.js to make sure we can access it immediately.

Comment 5

[John Hu [:johnhu][:johu][:醬糊小弟]]

It's reasonable. But may we promise that all JS are loaded before the callback at suite()?

Comment 6

[Ricky Chien [:rickychien] (inactive)]

Yeah, I agree. We could modify all require/requireApp pattern to match promise feature and make sure we can access any objects right away after finishing script download.

What do you think Julien?

Comment 7

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

(In reply to John Hu [:johnhu][:johu][:醬糊小弟] from comment #5)
> It's reasonable. But may we promise that all JS are loaded before the
> callback at suite()?

Yes, all require'd js files are loaded when the callback in suite is called.

setup.js works the same, but maybe by chance ;)


(In reply to Ricky Chien [:rickychien] from comment #6)
> Yeah, I agree. We could modify all require/requireApp pattern to match
> promise feature and make sure we can access any objects right away after
> finishing script download.
> 
> What do you think Julien?

I tried to use the require's callback and I never really could make it work reliably.

The main issue was that if some files were already loaded in another test, the callback was called right away, but otherwise it was called in order thanks to how the browser processes dynamically inserted scripts. That said, maybe it's different now that we have a new sandbox for each file.

I don't see the improvement of converting the callback we barely use to promises.

I think your time would be better used by adding the mocks_helper and load_body_html_helper in the default configuration for our test-agent :)

Comment 8

[Julien Wajsberg [:julienw] (PTO -> Aug 14)]

Let me precise, because I just saw that what I am saying is not completely correct.

The "suite" callback is called before the scripts are loaded, but setup/suiteSetup/tests are always called once they're loaded.

Behind the doors, they start when the 'load' event is sent.