Closed Bug 1127376 Opened 5 years ago Closed 5 years ago

PEP8-ify all of mochitest

Categories

(Testing :: Mochitest, defect)

defect
Not set

Tracking

(firefox38 fixed)

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: ahal, Assigned: ahal)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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.
Attached file MozReview Request: bz://1127376/ahal (obsolete) —
/r/3283 - Bug 1127376 - Flake8 passing in all mochitest python files

Pull down this commit:

hg pull review -r 65c0309ba17cbd3ae3c0dfb5290ccb65fa854fe1
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
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).
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
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+
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
https://hg.mozilla.org/mozilla-central/rev/32fe8089d155
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.