Closed Bug 974465 Opened 6 years ago Closed 6 years ago

Remove asterisk imports in Marionette


(Testing :: Marionette, defect)

Not set


(Not tracked)



(Reporter: ato, Assigned: william.h.angell)


(Whiteboard: [good first bug][lang=py][mentor=automatedtester])


(1 file, 1 obsolete file)

Importing all symbols from a module is a bad idea and discouraged.  Instead import the required symbols or just the module.
Summary: Remove asterisk import in testing/marionette/client/marionette/ → Remove asterisk imports in Marionette
we need to update to remove the * import
Whiteboard: [good first bug][lang=py][mentor=automatedtester]
Hi, I wish to fix this bug. Please guide me through the process.
Hi Subhendu,

Follow the steps on to get firefox built.

Make the change as mentioned in comment 1. We need to be explicit with our imports and then follow for running the tests.

Hope that helps!
Thanks for the patch. I have pushed to our try servers which will run all the tests like they do in production. You can see the results from the links below.

Mobile try
Desktop Try
Please note that there are several other files in the Marionette client code that uses asterisk imports.  A quick grep gives me the following:

% grep -r -i --include \*.py "import \*" .
./ gestures import *
./ errors import *
./ runner import *
./ errors import *
./ errors import *
./runner/ base import *
./runner/ mixins import *
./runner/mixins/ endurance import *
./runner/mixins/ reporting import *
./runner/mixins/ b2g import *
./tests/unit/ single_finger_functions import *
./tests/unit/ single_finger_functions import *

Also the parenthesis construct lets you avoid redefining the "from foo import …" statement when you have really long imports that exceed the maximum allowed line length:

    from foo import (
I've replaced the "import *"s in all of the files mentioned above, as well as changing the long import statements to the suggested form.
Attachment #8413394 - Attachment is obsolete: true
Attachment #8413394 - Flags: review?(dburns)
Attachment #8413839 - Flags: review?(dburns)
Assignee: nobody → william.h.angell
Flags: needinfo?(dburns)
Flags: needinfo?(dburns)
It looks like a random failure.  I consulted with #ateam in IRC and someone scheduled another try:

I think it's okay now (if I'm reading this right, new Trys are appended to the first Try).

Please let me know.
Comment on attachment 8413839 [details] [diff] [review]
Patch for Bug 974465 to remove asterisk imports

Review of attachment 8413839 [details] [diff] [review]:

Attachment #8413839 - Flags: review?(dburns) → review+
trees closed will try push in a bit
Flags: needinfo?(dburns)
William, thanks and congrats on getting the patch landed!  It will appear on m-c soon and we can resolve this bug as fixed.
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.