Skip to content

Fixes #2828 - Changes from pep8 to pycodestyle #2829

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
name: run tests
command: |
. venv/bin/activate
pep8 --ignore=E402 webcompat/ tests/ config/secrets.py.example
pycodestyle --ignore=E402,W504 webcompat/ tests/ config/secrets.py.example
npm run lint
npm run build
nosetests
Expand Down
6 changes: 3 additions & 3 deletions config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@

import requests

from environment import * # nopep8
from secrets import * # nopep8
from environment import * # noqa
from secrets import * # noqa

MILESTONE_ERROR = """It failed with {msg}!
We will read from data/milestones.json.
Expand All @@ -29,7 +29,7 @@
in config/secrets.py and try again. Good luck!
"""
MILESTONE_UNMATCHING = """A milestone is missing or has been added: {names}"""
MILESTONE_UNMATCHING_ERROR = """Check the milestones names on your Github repository and try again.
MILESTONE_UNMATCHING_ERROR = """Check the milestones names on your Github repository and try again.
This error was probably caused by a typo.
Your milestones.json was erased and a backup copy was created at {path}.
"""
Expand Down
2 changes: 1 addition & 1 deletion config/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ Flask-WTF==0.14.2
GitHub-Flask==3.2.0
mock==2.0.0
nose==1.3.7
pep8==1.7.1
Pillow==5.4.1
pycodestyle==2.5.0
requests==2.21.0
ua-parser==0.8
WTForms==2.2.1
10 changes: 5 additions & 5 deletions docs/pr-coding-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -91,19 +91,19 @@ For product and design contributions, check out the [Design Repo](https://github
Try to take care to follow existing conventions. Some of these are defined in an [.editorconfig](https://github.com/webcompat/webcompat.com/blob/master/.editorconfig) file. You can download the plugin for your editor here http://editorconfig.org/#download.

### Python
As we are still very early in the project, we do not yet have that many conventions for naming, routes, APIs. If in doubt, ask us or open an issue. All Python code should pass [pep8](http://pep8.readthedocs.org/en/1.4.6/intro.html).
As we are still very early in the project, we do not yet have that many conventions for naming, routes, APIs. If in doubt, ask us or open an issue. All Python code should pass [pycodestyle](http://pycodestyle.pycqa.org/en/latest/intro.html).

You can check this by installing the pep8 module.
You can check this by installing the pycodestyle module. This is usually installed through the requirements file in the project. If you wish to install it yourself on your computer, you can do.

sudo pip install pep8
pip install --user pycodestyle

Once at the root of the project you can run it with

pep8 --show-source --show-pep8 .
pycodestyle --ignore=E402,W504 webcompat/ tests/ config/secrets.py.example

That will show you the list of errors and their explanations. Another tool, we have used for checking consistency of the code is `flake8` + `hacking`. [Hacking](https://github.com/openstack-dev/hacking) is a set of [OpenStack guidelines](http://docs.openstack.org/developer/hacking/) which is used by the community for the stability of their projects. You will see that there's nothing really hard about it.

sudo pip install hacking
pip install --user hacking

will install the relevant flake8 and hacking modules. In the same fashion, if you do

Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_api_urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
'rv:31.0) Gecko/20100101 Firefox/31.0'),
'HTTP_ACCEPT': 'application/json'}

STATUSES = {u'sitewait': {'color': '', 'state': 'open', 'id': 5, 'order': 5}, u'worksforme': {'color': '', 'state': 'closed', 'id': 11, 'order': 7}, u'non-compat': {'color': '', 'state': 'closed', 'id': 12, 'order': 5}, u'needsdiagnosis': {'color': '', 'state': 'open', 'id': 2, 'order': 2}, u'contactready': {'color': '', 'state': 'open', 'id': 4, 'order': 4}, u'wontfix': {'color': '', 'state': 'closed', 'id': 6, 'order': 6}, u'needscontact': {'color': '', 'state': 'open', 'id': 3, 'order': 3}, u'invalid': {'color': '', 'state': 'closed', 'id': 8, 'order': 4}, u'needstriage': {'color': '', 'state': 'open', 'id': 1, 'order': 1}, u'duplicate': {'color': '', 'state': 'closed', 'id': 10, 'order': 1}, u'fixed': {'color': '', 'state': 'closed', 'id': 9, 'order': 2}, u'incomplete': {'color': '', 'state': 'closed', 'id': 7, 'order': 3}} # nopep8
STATUSES = {u'sitewait': {'color': '', 'state': 'open', 'id': 5, 'order': 5}, u'worksforme': {'color': '', 'state': 'closed', 'id': 11, 'order': 7}, u'non-compat': {'color': '', 'state': 'closed', 'id': 12, 'order': 5}, u'needsdiagnosis': {'color': '', 'state': 'open', 'id': 2, 'order': 2}, u'contactready': {'color': '', 'state': 'open', 'id': 4, 'order': 4}, u'wontfix': {'color': '', 'state': 'closed', 'id': 6, 'order': 6}, u'needscontact': {'color': '', 'state': 'open', 'id': 3, 'order': 3}, u'invalid': {'color': '', 'state': 'closed', 'id': 8, 'order': 4}, u'needstriage': {'color': '', 'state': 'open', 'id': 1, 'order': 1}, u'duplicate': {'color': '', 'state': 'closed', 'id': 10, 'order': 1}, u'fixed': {'color': '', 'state': 'closed', 'id': 9, 'order': 2}, u'incomplete': {'color': '', 'state': 'closed', 'id': 7, 'order': 3}} # noqa


def mock_api_response(response_config={}):
Expand Down Expand Up @@ -63,7 +63,7 @@ def test_api_issues_out_of_range(self):
with patch('webcompat.helpers.proxy_request') as github_data:
github_data.return_value = mock_api_response({
'status_code': 404,
'content': '[{"message":"Not Found","documentation_url":"https://developer.github.com/v3"}]' # nopep8
'content': '[{"message":"Not Found","documentation_url":"https://developer.github.com/v3"}]' # noqa
})
rv = self.app.get('/api/issues/1', environ_base=headers)
json_body = json.loads(rv.data)
Expand Down Expand Up @@ -110,7 +110,7 @@ def test_api_comments_link_header_auth(self):
'status_code': 200,
'content': '[]',
'headers': {
'Link': '<https://api.github.com/repositories/17839063/issues/398/comments?page=2>; rel="next", <https://api.github.com/repositories/17839063/issues/398/comments?page=4>; rel="last"', # nopep8
'Link': '<https://api.github.com/repositories/17839063/issues/398/comments?page=2>; rel="next", <https://api.github.com/repositories/17839063/issues/398/comments?page=4>; rel="last"', # noqa
},
}),
mock_api_response({'status_code': 200, 'content': '[]'})
Expand Down
1 change: 1 addition & 0 deletions tests/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,6 @@ def test_update_status_config(self):
actual = update_status_config(milestones_json)
self.assertEqual(actual, expected)


if __name__ == '__main__':
unittest.main()
3 changes: 2 additions & 1 deletion tests/unit/test_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

# Add webcompat module to import path
sys.path.append(os.path.realpath(os.pardir))
import webcompat # nopep8
import webcompat # noqa

# Any request that depends on parsing HTTP Headers (basically anything
# on the index route, will need to include the following: environ_base=headers
Expand Down Expand Up @@ -87,5 +87,6 @@ def test_browser_labels(self):
labels = ['status-foo', 'blah', 'browser-', 'browser-firefox']
self.assertListEqual(['firefox'], browser_labels(labels))


if __name__ == '__main__':
unittest.main()
22 changes: 11 additions & 11 deletions tests/unit/test_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from webcompat import form
from webcompat import helpers

FIREFOX_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0' # nopep8
FIREFOX_UA = 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:48.0) Gecko/20100101 Firefox/48.0' # noqa


class TestForm(unittest.TestCase):
Expand Down Expand Up @@ -144,7 +144,7 @@ def test_get_metadata(self):
metadata_keys = ['browser', 'ua_header', 'reported_with',
'extra_labels']
actual = form.get_metadata(metadata_keys, form_object)
expected = u'<!-- @browser: Firefox 59.0 -->\n<!-- @ua_header: Mozilla/5.0...Firefox 59.0 -->\n<!-- @reported_with: desktop-reporter -->\n<!-- @extra_labels: type-webrender-enabled -->\n' # nopep8
expected = u'<!-- @browser: Firefox 59.0 -->\n<!-- @ua_header: Mozilla/5.0...Firefox 59.0 -->\n<!-- @reported_with: desktop-reporter -->\n<!-- @extra_labels: type-webrender-enabled -->\n' # noqa
self.assertEqual(actual, expected)

def test_get_metadata_browser_as_extra(self):
Expand All @@ -158,7 +158,7 @@ def test_get_metadata_browser_as_extra(self):
metadata_keys = ['browser', 'ua_header', 'reported_with',
'extra_labels']
actual = form.get_metadata(metadata_keys, form_object)
expected = u'<!-- @browser: Firefox 59.0 -->\n<!-- @ua_header: Mozilla/5.0...Firefox 59.0 -->\n<!-- @reported_with: desktop-reporter -->\n<!-- @extra_labels: browser-focus-geckoview -->\n' # nopep8
expected = u'<!-- @browser: Firefox 59.0 -->\n<!-- @ua_header: Mozilla/5.0...Firefox 59.0 -->\n<!-- @reported_with: desktop-reporter -->\n<!-- @extra_labels: browser-focus-geckoview -->\n' # noqa
self.assertEqual(actual, expected)

def test_normalize_metadata(self):
Expand All @@ -181,18 +181,18 @@ def test_build_formdata(self):
# even if the data are empty
form_object = {'foo': 'bar'}
actual = form.build_formdata(form_object)
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: None\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': 'None - unknown'} # nopep8
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: None\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': 'None - unknown'} # noqa
self.assertIs(type(actual), dict)
self.assertEqual(actual, expected)
# testing for double URL Schemes.
form_object = {'url': 'http://https://example.com/'}
actual = form.build_formdata(form_object)
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: https://example.com/\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': 'example.com - unknown'} # nopep8
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: https://example.com/\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': 'example.com - unknown'} # noqa
self.assertEqual(actual, expected)
# testing with unicode strings.
form_object = {'url': u'愛'}
actual = form.build_formdata(form_object)
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: http://\u611b\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': u'\u611b - unknown'} # nopep8
expected = {'body': u'<!-- @browser: None -->\n<!-- @ua_header: None -->\n<!-- @reported_with: None -->\n\n**URL**: http://\u611b\n\n**Browser / Version**: None\n**Operating System**: None\n**Tested Another Browser**: Unknown\n\n**Problem type**: Unknown\n**Description**: None\n**Steps to Reproduce**:\nNone\n\n\n\n_From [webcompat.com](https://webcompat.com/) with \u2764\ufe0f_', 'title': u'\u611b - unknown'} # noqa
self.assertEqual(actual, expected)

def test_get_details(self):
Expand All @@ -209,26 +209,26 @@ def test_build_details(self):
# Test for receiving JSON object as a string
actual_json_arg = form.build_details(json.dumps(
{'a': 'b', 'c': False}))
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>' # nopep8
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>' # noqa
self.assertEqual(actual_json_arg, expected_json_arg)
# Test for receiving a JSON value which is not an object
actual_json_arg = form.build_details('null')
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>None</li>\n</ul>\n\n</details>' # nopep8
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>None</li>\n</ul>\n\n</details>' # noqa
self.assertEqual(actual_json_arg, expected_json_arg)
# Test for receiving a string
actual_string_arg = form.build_details(u'cool')
expected_string_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>cool</li>\n</ul>\n\n</details>' # nopep8
expected_string_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>cool</li>\n</ul>\n\n</details>' # noqa
self.assertEqual(actual_string_arg, expected_string_arg)

def test_build_details_with_console_logs(self):
"""Expected HTML is returned for a json object with console logs."""
actual_json_arg = form.build_details(json.dumps(
{'a': 'b', 'c': False, 'consoleLog': ['console.log(hi)']}))
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n<p>Console Messages:</p>\n<pre>\n[u\'console.log(hi)\']\n</pre>\n</details>' # nopep8
expected_json_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n<p>Console Messages:</p>\n<pre>\n[u\'console.log(hi)\']\n</pre>\n</details>' # noqa
self.assertEqual(actual_json_arg, expected_json_arg)
actual_empty_log_arg = form.build_details(json.dumps(
{'a': 'b', 'c': False, 'consoleLog': ''}))
expected_empty_log_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>' # nopep8
expected_empty_log_arg = '<details>\n<summary>Browser Configuration</summary>\n<ul>\n <li>a: b</li><li>c: false</li>\n</ul>\n\n</details>' # noqa
self.assertEqual(actual_empty_log_arg, expected_empty_log_arg)

def test_get_console_section(self):
Expand Down
Loading