Closed Bug 1396217 Opened 2 years ago Closed 2 years ago

make testing/talos directory pass the new python linting and py3 compatibility checks

Categories

(Testing :: Talos, defect)

defect
Not set

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jmaher, Assigned: igoldan)

Details

(Whiteboard: [PI:September])

Attachments

(1 file)

here is a log file:
https://public-artifacts.taskcluster.net/AZLftXZrQjyu_XN8UnM-CA/0/public/logs/live_backing.log

and the raw errors:
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/INSTALL.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/setup.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager_linux.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager_linux.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager_mac.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager_mac.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmanager_win32.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmdline.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/cmdline.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/config.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/config.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ffsetup.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ffsetup.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ffsetup.py:168:25 | invalid syntax (is-parseable)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/filter.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/gecko_profile.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/mainthreadio.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/mainthreadio.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/mitmproxy/alternate-server-replay.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/mitmproxy/alternate-server-replay.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/mitmproxy/mitmproxy.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/output.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/profiling.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/symFileManager.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/symLogging.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/symLogging.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/symbolication.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/profiler/symbolicationRequest.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/results.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/results.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/run_tests.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/run_tests.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/scripts/report.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/talos_process.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/talosconfig.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/talosconfig.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/test.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ttest.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/ttest.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/utils.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/whitelist.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/whitelist.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/__init__.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/etlparser.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/etlparser.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/parse_xperf.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/parse_xperf.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/start_xperf.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/start_xperf.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/xtalos.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos/xtalos/xtalos.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos_from_code.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/talos_from_code.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_browser_output.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_filter.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_results.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_talosconfig.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_talosconfig.py:1:1 | Missing from __future__ import print_function (require print_function)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_urlsplit.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_utils.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)
TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/testing/talos/tests/test_xrestop.py:1:1 | Missing from __future__ import absolute_import (require absolute_import)


I am not sure how to run this locally, but if you push to try it automatically runs:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=c00daa7170d90038a18a9dbf3fb412346583e838
:igoldan, is this something you would like to fix up?
Flags: needinfo?(ionut.goldan)
I forgot to mention, in order to test this out, there are files in-tree:
* tools/lint/py3.yml
* tools/lint/py2.yml

these have exclusions for many directories including testing/talos, I removed those entries and then pushed to try.
Sure, I will take a look. Thanks for the tips!
Flags: needinfo?(ionut.goldan)
Assignee: nobody → ionut.goldan
The errors above are from py2 linting, which we can address. But making Talos Python3.X compatible is totally another issue.
If we want that, we need another dedicated bug.

Another thing I'm not sure about: py2 linting checks that each module includes one of the 2 __future__ import statements; but there are versions of Python which include these by default and so don't require them explicitly. These can be seen here:
https://docs.python.org/2.7/library/__future__.html#module-__future__

I don't think we need these imports on all of Talos' modules, as they're included by default on the version we're using (2.7). Rather tweak the check_compat_py2 function to be aware where these imports are optional.
Flags: needinfo?(jmaher)
Flags: needinfo?(ahalberstadt)
I will let :ahal comment on the checks that are enabled for the pylint jobs.  The main goal here is to pass the checks and if we determine that we have incorrect checks we should change that.
Flags: needinfo?(jmaher)
Thanks, Joel. I submitted a sample with the tweaks I mentioned.
Comment on attachment 8904970 [details]
Bug 1396217 - resolve py2 and py3 lint errors

redirecting to :ahal
Attachment #8904970 - Flags: review?(jmaher) → review?(ahalberstadt)
Comment on attachment 8904970 [details]
Bug 1396217 - resolve py2 and py3 lint errors

https://reviewboard.mozilla.org/r/176804/#review181774

In python 2.7 these features are optional, but we want to make them mandatory. This will make it easier to transistion to python 3 at a later date. P.s the mach bootstrapper already checks that the minimum python version used is 2.7. So we are guaranteed to be using 2.7 (or python 3) here.
Attachment #8904970 - Flags: review?(ahalberstadt) → review-
(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #4)
> Another thing I'm not sure about: py2 linting checks that each module
> includes one of the 2 __future__ import statements; but there are versions
> of Python which include these by default and so don't require them
> explicitly. These can be seen here:
> https://docs.python.org/2.7/library/__future__.html#module-__future__

I think by "optional" they mean that you can "optionally enable it by including it at the top of the file". For example, in python 2.7, print_function is definitely not enabled by default :).
Flags: needinfo?(ahalberstadt)
I also wanted to mention that adding this to the py3 linter doesn't mean it needs to work in python 3. It's just needs to parse without syntax errors. Usually this is just stuff like turning print statements into functions and isn't too hard to get working.
Comment on attachment 8904970 [details]
Bug 1396217 - resolve py2 and py3 lint errors

https://reviewboard.mozilla.org/r/176804/#review182814
Attachment #8904970 - Flags: review?(jmaher) → review+
We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again.

hg error in cmd: hg rebase -s 25a1ce92e842 -d 4c9a3b09cfc1: rebasing 418879:25a1ce92e842 "Bug 1396217 - resolve py2 and py3 lint errors r=jmaher" (tip pylint_fix3)
merging testing/talos/talos/config.py
merging testing/talos/talos/mainthreadio.py
merging testing/talos/talos/whitelist.py
warning: conflicts while merging testing/talos/talos/mainthreadio.py! (edit, then use 'hg resolve --mark')
warning: conflicts while merging testing/talos/talos/whitelist.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
:igoldan, if you update this we can push it again.
Flags: needinfo?(ionut.goldan)
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/acb232650faf
resolve py2 and py3 lint errors r=jmaher
Thanks for landing this. I looked over the Try results:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=4cf7bd0aa6d495021fbe4b1ae86d1299c30916b3&selectedJob=130052303

As you can see, I issued lots of retriggers, most of which are green. Some of the Talos tests failed, then succeeded on next retriggers. I assume it's because of environment issues.
Flags: needinfo?(ionut.goldan)
https://hg.mozilla.org/mozilla-central/rev/acb232650faf
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.