Closed Bug 1397855 Opened 7 years ago Closed 7 years ago

Enable py2 and py3 linter on testing/xpcshell

Categories

(Testing :: XPCShell Harness, enhancement)

enhancement
Not set
normal

Tracking

(firefox57 fixed)

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: ahal, Assigned: stevea1, Mentored)

References

(Depends on 1 open bug)

Details

(Whiteboard: [good-first-bug])

Attachments

(1 file)

+++ This bug was initially created as a clone of Bug #1397853 +++

These linters can be enabled by removing 'testing/xpcshell' from the exclude section in tools/lint/py2.yml and tools/lint/py3.yml.

You can then see all the errors by running:
./mach lint -l py2 -l py3 testing/xpcshell

Each of the errors will need to be fixed before this can land.

To test changes, please be sure to run both:
./mach lint --outgoing
./mach python-test testing/xpcshell

to be sure nothing else broke. The latter requires a build, alternatively ask me to push this to try.
Assignee: nobody → stevea1
Status: NEW → ASSIGNED
Patch posted. I managed to eliminate all the Python 2 and 3 lint errors. After making a build, I was able to run:
./mach python-test testing/xpcshell

which returned:
 0:05.91 Return code from mach python-test: 0
Comment on attachment 8905749 [details]
Bug 1397855 - Enable py2 and py3 linter on testing/xpcshell.

https://reviewboard.mozilla.org/r/177552/#review183238

::: testing/xpcshell/remotexpcshelltests.py:173
(Diff revision 1)
>                          test_name=None):
>          if not self.device.dirExists(self.remoteMinidumpDir):
>              # The minidumps directory is automatically created when Fennec
>              # (first) starts, so its lack of presence is a hint that
>              # something went wrong.
> -            print "Automation Error: No crash directory (%s) found on remote device" % self.remoteMinidumpDir
> +            print ("Automation Error: No crash directory (%s) found on remote device" % self.remoteMinidumpDir)

Minor styling issue, there shouldn't be a space between a function call and its opening bracket.

If you're planning to work on bug 1397852 then feel free to close this issue (as it will show up as an error in the flake8 linter) and fix it there. Otherwise please push an updated commit with this fixed in all three places (hint: you can use `hg commit --amend`).
Attachment #8905749 - Flags: review?(ahalberstadt) → review+
I've corrected the 3 instances of the space after the 'print' function call in testing/xpcshell/remotexpcshelltests.py. I made a new build and ran:

./mach python-test testing/xpcshell
 0:04.41 Return code from mach python-test: 0
hg error in cmd: hg push -r tip ssh://hg.mozilla.org/integration/autoland: pushing to ssh://hg.mozilla.org/integration/autoland
searching for changes
remote: adding changesets
remote: adding manifests
remote: adding file changes
remote: added 1 changesets with 6 changes to 6 files
remote: 
remote: 
remote: ************************** ERROR ****************************
remote: Rev 39c1cddf676e needs "Bug N" or "No bug" in the commit message.
remote: Steve Armand <stevea1@mac.com>
remote: 1397855 - Enable py2 and py3 linter on testing/xpcshell. r=ahal
remote: 
remote: MozReview-Commit-ID: CsfIcI1ma7J
remote: *************************************************************
remote: 
remote: 
remote: transaction abort!
remote: rollback completed
remote: pretxnchangegroup.c_commitmessage hook failed
abort: push failed on remote
Thanks Steve! The commit message needs to have "Bug" at the start (sorry didn't notice in the review). Please push up a new commit with the amended message and I'll land it again.
Flags: needinfo?(stevea1)
My bad. I've updated the commit message to start with "Bug" so we should be all set now.
Flags: needinfo?(stevea1)
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ec61b7a902c2
Enable py2 and py3 linter on testing/xpcshell. r=ahal
Sorry, this had to be backed out for XPCshell bustage, at least on Android:

https://hg.mozilla.org/integration/autoland/rev/6cffef0e5ab98481df8e43647c4dd4b1372c17bd

Push which ran failing tests (only look at XPCshell): https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=5951cc2057a14fb414194127feeb3d82afda9205&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=130425873&repo=autoland

[task 2017-09-12T19:22:02.818830Z] 19:22:02     INFO -  pushing /builds/worker/workspace/build/tests/xpcshell/tests
[task 2017-09-12T19:22:43.739134Z] 19:22:43     INFO -  Traceback (most recent call last):
[task 2017-09-12T19:22:43.740653Z] 19:22:43     INFO -    File "/builds/worker/workspace/build/tests/xpcshell/remotexpcshelltests.py", line 615, in <module>
[task 2017-09-12T19:22:43.741232Z] 19:22:43     INFO -      main()
[task 2017-09-12T19:22:43.741339Z] 19:22:43     INFO -    File "/builds/worker/workspace/build/tests/xpcshell/remotexpcshelltests.py", line 603, in main
[task 2017-09-12T19:22:43.741940Z] 19:22:43     INFO -      xpcsh = XPCShellRemote(dm, options, log)
[task 2017-09-12T19:22:43.742007Z] 19:22:43     INFO -    File "/builds/worker/workspace/build/tests/xpcshell/remotexpcshelltests.py", line 287, in __init__
[task 2017-09-12T19:22:43.742052Z] 19:22:43     INFO -      self.setupUtilities()
[task 2017-09-12T19:22:43.742122Z] 19:22:43     INFO -    File "/builds/worker/workspace/build/tests/xpcshell/remotexpcshelltests.py", line 422, in setupUtilities
[task 2017-09-12T19:22:43.742171Z] 19:22:43     INFO -      print >> sys.stderr, "Pushing %s.." % fname
[task 2017-09-12T19:22:43.742230Z] 19:22:43     INFO -  TypeError: unsupported operand type(s) for >>: 'builtin_function_or_method' and 'file'
[task 2017-09-12T19:22:43.758369Z] 19:22:43    ERROR - Return code: 1

Please fix the issue and update the patch. Thank you.
Flags: needinfo?(stevea1)
My mistake for not including xpcshell on the try push. I would have thought this would be caught by the linter, but I guess not!

Steve you can fix this by turning it into a function and passing in file=sys.stderr. I'll do a try run that includes xpcshell when you have the new patch.
Ok, I've modified the 11 instances of 'print >>' to be print functions with file=sys.stderr instead.
Flags: needinfo?(stevea1)
Thanks, try run up here:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7d2f97829390

I'll also file a bug against the py2 linter that this doesn't get caught.
See Also: → 1399469
Try run looks fixed, I'll try landing again.
Pushed by ahalberstadt@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e5b9fcb0f622
Enable py2 and py3 linter on testing/xpcshell. r=ahal
https://hg.mozilla.org/mozilla-central/rev/e5b9fcb0f622
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Thanks for the patch Steve, much appreciated!

If you want, you could try bug 1399469 next. It might be a little tricky, but I think you'd be able to manage. Let me know if you're interested and I can give you a place to start. Otherwise, finishing up these bugs is also very helpful.
Blocks: 1411568
Depends on: 1525302
You need to log in before you can comment on or make changes to this bug.