Closed Bug 974465 Opened 6 years ago Closed 6 years ago
Remove asterisk imports in Marionette
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/marionette_test.py → Remove asterisk imports in Marionette
we need to update http://dxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/marionette.py#17 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 https://developer.mozilla.org/en-US/docs/Simple_Firefox_build to get firefox built. Make the change as mentioned in comment 1. We need to be explicit with our imports and then follow https://developer.mozilla.org/en-US/docs/Mozilla/QA/Marionette/Running_Tests 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 https://tbpl.mozilla.org/?tree=Try&rev=3f27942e502b Desktop Try https://tbpl.mozilla.org/?tree=Try&rev=f2c3d8f7f1e7
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 \*" . ./__init__.py:from gestures import * ./__init__.py:from errors import * ./__init__.py:from runner import * ./marionette.py:from errors import * ./marionette_test.py:from errors import * ./runner/__init__.py:from base import * ./runner/__init__.py:from mixins import * ./runner/mixins/__init__.py:from endurance import * ./runner/mixins/__init__.py:from reporting import * ./runner/mixins/__init__.py:from b2g import * ./tests/unit/test_single_finger.py:from single_finger_functions import * ./tests/unit/test_single_finger_desktop.py:from 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 ( foo bar )
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.
try pushes https://tbpl.mozilla.org/?tree=Try&rev=dfe64ba80354 https://tbpl.mozilla.org/?tree=Try&rev=4915fb44f7a2
Assignee: nobody → william.h.angell
It looks like a random failure. I consulted with #ateam in IRC and someone scheduled another try: https://tbpl.mozilla.org/?tree=Try&rev=dfe64ba80354 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]: ----------------------------------------------------------------- lgtm
Attachment #8413839 - Flags: review?(dburns) → review+
trees closed will try push in a bit
landed in https://hg.mozilla.org/integration/mozilla-inbound/rev/0d3f2e53c30e William, thanks for doing this! If you want to try do another bug have a look at https://bugzilla.mozilla.org/buglist.cgi?resolution=---&status_whiteboard_type=allwordssubstr&query_format=advanced&status_whiteboard=[good%20first%20bug]&bug_status=UNCONFIRMED&bug_status=NEW&bug_status=ASSIGNED&bug_status=REOPENED&component=Marionette&product=Testing&list_id=10003041
William, thanks and congrats on getting the patch landed! It will appear on m-c soon and we can resolve this bug as fixed.
Status: NEW → RESOLVED
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.