602455 - With CommonJS global mozmill objects are not in module scope anymore

Status: VERIFIED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With the update to CommonJS I'm not able to access the mozmill object from within modules anymore. It's only available inside the tests itself. That's kinda bad because we have some instances where we call mozmill.controller.sleep without having to pass a controller instance into the helper function.

See the attachment which contains an example test and module. Once you remove the mozmill parameter from the setup function, the code will fail.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: test case

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

It can be solved by putting the mozmill object into the globals array. We should also check to which other objects modules should have access to.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

And one more thing. Would there be a way to set a module id? I would really like to replace the MODULE_NAME with module.id, i.e:

OLD: var MODULE_NAME = 'UtilsAPI';
New: module.id = 'utils';

But there is no global module object in the scope. See http://wiki.commonjs.org/wiki/Modules/1.1#Sample_Code

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Beside Mozmill it also affects elementslib.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v1

Proposed patch which should at least retain access to the global objects from within the modules.

Comment 6

[Heather Arthur [:harth]]

Comment on attachment 481476 [details] [diff] [review]
Patch v1

looks good

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Heather, could you also give me feedback for comment 3? That would be great.

Comment 8

[Heather Arthur [:harth]]

(In reply to comment #3)
> And one more thing. Would there be a way to set a module id? I would really
> like to replace the MODULE_NAME with module.id, i.e:
> 
> OLD: var MODULE_NAME = 'UtilsAPI';
> New: module.id = 'utils';
> 
> But there is no global module object in the scope. See
> http://wiki.commonjs.org/wiki/Modules/1.1#Sample_Code

Right now we're kind of at the mercy of the Jetpack SDK's module loader, though we could hack in better CommonJS support. Right now you can just name the file what you want the module name to be.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #8)
> Right now we're kind of at the mercy of the Jetpack SDK's module loader, though
> we could hack in better CommonJS support. Right now you can just name the file
> what you want the module name to be.

Alright. That would work.

Would it be possible to give this bug approval for Mozmill 1.5.1?

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed as:
http://github.com/mozautomation/mozmill/commit/6a93c3e6cd0c0a6cc6f08ce99f6238b90eab1521 (master)
http://github.com/mozautomation/mozmill/commit/23676c4fa3ce6001b15c7359d455b3c5a3b9aebc (hotfix-1.5.1)

Comment 11

[Aaron Train [:aaronmt]]

Verified fixed with 1.5.1rc1