If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

PEP8-ify all of mochitest

RESOLVED FIXED in Firefox 38

Status

Testing
Mochitest
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ahal, Assigned: ahal)

Tracking

(Blocks: 1 bug)

unspecified
mozilla38
Points:
---

Firefox Tracking Flags

(firefox38 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Basically I've caved and can't stand looking at mochitest anymore :p.

I already have a patch that gives four space indent to runtests.py. I'll do the same for the other .py files. Maybe I'll call it a day once that's done, but I might as well fix other violations while I'm at it. Will use flake8.
(Assignee)

Comment 1

3 years ago
To switch to 4 space indents, I first did this:
http://stackoverflow.com/a/16892086

Then used flake8 to fix space related violations and finally a manual scan to make sure everything looks to be in the right place. I'm fairly confident the indentation levels were kept the same, but will do lots of local and try testing to be safe.
rs=me. We should do this everywhere.
(Assignee)

Comment 3

3 years ago
Created attachment 8558599 [details]
MozReview Request: bz://1127376/ahal

/r/3283 - Bug 1127376 - Flake8 passing in all mochitest python files

Pull down this commit:

hg pull review -r 65c0309ba17cbd3ae3c0dfb5290ccb65fa854fe1
(Assignee)

Comment 4

3 years ago
There's no possible way anyone can do a thorough review on that and keep their sanity. So here's a full mochitest try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2abb06fd1b57

Comment 5

3 years ago
Not sure if you've tried https://pypi.python.org/pypi/autopep8/ but it seems pretty effective at my initial attempts using it (note you'll need to add duplicate config entries under [pep8] matching what you have under [flake8] if you use a global config).
(Assignee)

Comment 6

3 years ago
Heh, yeah I found that after I had already done it all semi-manually, sigh. Rather than playing spot the indentation change in my patch [1], I think I'll just go with autopep8. The try run with that is looking a lot better:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b60ad7bf432

[1] I had fixed the perma failures in above try run, but then ran into oranges on about half the chunks.. so must have inadvertently changed something somewhere
(Assignee)

Comment 7

3 years ago
Created attachment 8560461 [details] [diff] [review]
Auto-generated patch using autopep8

This isn't a review for the patch, so much as for the process used to generate the patch. This bitrots like crazy, and I think it is easier to just run autopep8 again, than it is to try and rebase it, so the final patch that lands might be slightly different than this.

To replicate this patch:

    $ pip install --upgrade autopep8
    $ autopep8 -i -a -a -r testing/mochitest --exclude 'testing/mochitest/pywebsocket/*'

See https://github.com/hhatto/autopep8#usage for details on arguments. The previous try run seems to be mostly passing. I'll do another try run as close as possible to landing since I've already needed to re-generate this.
Attachment #8558599 - Attachment is obsolete: true
Attachment #8560461 - Flags: review?(ted)
Attachment #8560461 - Flags: review?(ted) → review+
(Assignee)

Comment 8

3 years ago
Here's a more recent try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=199ea2329e64

I had to re-generate the patch since then, but this patch bitrots faster than the try results come back, so at some point I just have to push:
https://hg.mozilla.org/integration/mozilla-inbound/rev/32fe8089d155
Blocks: 1109785
https://hg.mozilla.org/mozilla-central/rev/32fe8089d155
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox38: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.