Haciendo que omegaUp siga pep8 y pylint (#1635)

Este cambio agrega linters para que sigamos pep8 y pylint. No más
bikesheds para código en Python!

.lint.config.json

@@ -5,7 +5,8 @@
 				"^frontend/.*\\.(php|css|js|sql|tpl|py|vue)$"
 			],
 			"blacklist": [
-				".*third_party.*"
+				".*third_party.*",
+				".*\\.py$"
 			]
 		},
 		"php": {
@@ -35,6 +36,16 @@
 			"whitelist": [
 				"frontend/www/.*\\.vue$"
 			]
+		},
+		"python": {
+			"pep8_config": "setup.cfg",
+			"whitelist": [
+				".*\\.py$"
+			],
+			"blacklist": [
+				"bin/karel_mdo_convert.py",
+				"frontend/tests/ui/integration-test.py"
+			]
 		}
 	}
 }

.pylintrc

@@ -0,0 +1,18 @@
+[MESSAGES CONTROL]
+
+# We are locally disabling something because we don't want to hear about it.
+#  I0011 = Locally disabling
+#
+# Too few public methods is bad for context managers.
+#  R0903 = Too few public methods
+disable=I0011,R0903
+
+[BASIC]
+
+# We allow variables to be two characters.
+attr-rgx=[a-z_][a-z0-9_]{1,64}$
+function-rgx=[a-z_][a-z0-9_]{1,64}$
+variable-rgx=[a-z_][a-z0-9_]{1,64}$
+
+# We also allow 's' (for strings) and 'f' (for files)
+good-names=i,j,k,ex,Run,_,s,f

.travis.yml

@@ -23,12 +23,13 @@ addons:
     sources:
       - ubuntu-toolchain-r-test
       - llvm-toolchain-precise-3.7
+      - deadsnakes
     packages:
       - clang-format-3.7
       - libmysqlclient-dev
       - nginx
       - python-pip
-      - python3
+      - python3.5
       - python3-pip
       - realpath
   sauce_connect:

frontend/tests/controllers/QualityNominationTest.php

@@ -791,7 +791,7 @@ public function testAggregateFeedback() {
         // Ensure all suggestions are written to the database before invoking
         // the external script.
         self::commit();
-        shell_exec('python3 ' . escapeshellarg(OMEGAUP_ROOT) . '/../stuff/cron/aggregate-feedback.py' .
+        shell_exec('python3 ' . escapeshellarg(OMEGAUP_ROOT) . '/../stuff/cron/aggregate_feedback.py' .
                  ' --host ' . escapeshellarg(OMEGAUP_DB_HOST) .
                  ' --user ' . escapeshellarg(OMEGAUP_DB_USER) .
                  ' --database ' . escapeshellarg(OMEGAUP_DB_NAME) .

frontend/tests/ui/conftest.py

@@ -6,12 +6,12 @@
 import contextlib
 import json
 import os.path
-import pytest
-import re
 import sys
 import time
 import urllib
 
+import pytest
+
 from selenium import webdriver
 from selenium.common.exceptions import WebDriverException
 from selenium.webdriver.support.wait import WebDriverWait
@@ -22,6 +22,7 @@
 _SUCCESS = True
 _WINDOW_SIZE = (1920, 1080)
 
+
 class Driver(object):
     '''Wraps the state needed to run a test.'''
 
@@ -45,7 +46,7 @@ def assert_script(self, script):
         '''Asserts that evaluating the JavaScript |script| returns true.'''
 
         assert self.browser.execute_script('return !!(%s);' % script), \
-               'Evaluation of `%s` returned false' % script
+            'Evaluation of `%s` returned false' % script
 
     def assert_script_equal(self, script, value):
         '''Asserts that evaluating the JavaScript |script| returns true.'''
@@ -106,39 +107,44 @@ def login(self, username, password):
         self.wait.until(lambda _: self.browser.current_url == home_page_url)
         self.wait_for_page_loaded()
 
+
 @pytest.hookimpl(hookwrapper=True)
 def pytest_pyfunc_call(pyfuncitem):
     '''Takes a screenshot and grabs console logs on test failures.'''
 
-    global _SUCCESS
+    global _SUCCESS  # pylint: disable=global-statement
 
     outcome = yield
 
     if not outcome.excinfo:
         return
     _SUCCESS = False
-    if not 'driver' in pyfuncitem.funcargs:
+    if 'driver' not in pyfuncitem.funcargs:
         return
     if _CI:
-        # When running in CI, we have movies, screenshots and logs in Sauce Labs.
+        # When running in CI, we have movies, screenshots and logs in
+        # Sauce Labs.
         return
     try:
-        driver = pyfuncitem.funcargs['driver']
+        current_driver = pyfuncitem.funcargs['driver']
         try:
-            logs = driver.browser.get_log('browser')
-        except:
+            logs = current_driver.browser.get_log('browser')
+        except:  # pylint: disable=bare-except
             # geckodriver does not support getting logs:
             # https://github.com/mozilla/geckodriver/issues/284
             logs = []
         results_dir = os.path.join(_DIRNAME, 'results')
         os.makedirs(results_dir, exist_ok=True)
-        driver.browser.get_screenshot_as_file(
+        current_driver.browser.get_screenshot_as_file(
             os.path.join(results_dir, 'webdriver_%s.png' % pyfuncitem.name))
-        with open(os.path.join(results_dir, 'webdriver_%s.log' % pyfuncitem.name), 'w') as f:
-            json.dump(logs, f, indent=2)
-    except Exception as ex:
+        logpath = os.path.join(results_dir,
+                               'webdriver_%s.log' % pyfuncitem.name)
+        with open(logpath, 'w') as logfile:
+            json.dump(logs, logfile, indent=2)
+    except Exception as ex:  # pylint: disable=broad-except
         print(ex)
 
+
 def pytest_addoption(parser):
     '''Allow configuration of test invocation.'''
 
@@ -150,31 +156,32 @@ def pytest_addoption(parser):
     parser.addoption('--disable-headless', action='store_false',
                      dest='headless', help='Show the browser window')
 
+
 def pytest_generate_tests(metafunc):
     '''Parameterize the tests with the browsers.'''
 
     if not metafunc.config.option.browsers:
         metafunc.config.option.browsers = ['chrome', 'firefox']
 
     if 'driver' in metafunc.fixturenames:
-        metafunc.parametrize('browser', metafunc.config.option.browsers,
+        metafunc.parametrize('browser_name', metafunc.config.option.browsers,
                              scope='session')
 
-@pytest.yield_fixture(scope='session')
-def driver(request, browser):
-    '''Run tests using the selenium webdriver.'''
+
+def _get_browser(request, browser_name):
+    '''Gets a browser object from the request parameters.'''
 
     if _CI:
         capabilities = {
             'tunnel-identifier': os.environ['TRAVIS_JOB_NUMBER'],
             'name': 'Travis CI run %s[%s]' % (
-                os.environ.get('TRAVIS_BUILD_NUMBER', ''), browser),
+                os.environ.get('TRAVIS_BUILD_NUMBER', ''), browser_name),
             'build': os.environ.get('TRAVIS_BUILD_NUMBER', ''),
             'tags': [os.environ.get('TRAVIS_PYTHON_VERSION', '3'), 'CI'],
         }
         # Add browser configuration
         capabilities.update({
-            'browserName': browser,
+            'browserName': browser_name,
             'version': 'latest',
             'platform': 'Windows 10',
             'screenResolution': '%dx%d' % _WINDOW_SIZE,
@@ -183,27 +190,36 @@ def driver(request, browser):
             os.environ.get('SAUCE_USERNAME', 'lhchavez'),
             os.environ['SAUCE_ACCESS_KEY']
         )
-        browser = webdriver.Remote(desired_capabilities=capabilities,
-                                   command_executor=hub_url)
-    else:
-        if browser == 'chrome':
-            options = webdriver.ChromeOptions()
-            options.binary_location = '/usr/bin/google-chrome'
-            options.add_experimental_option('prefs', {'intl.accept_languages': 'en_US'})
-            options.add_argument('--lang=en-US')
-            if request.config.option.headless:
-                options.add_argument('--headless')
-            browser = webdriver.Chrome(chrome_options=options)
-        else:
-            firefox_capabilities = webdriver.common.desired_capabilities.DesiredCapabilities.FIREFOX
-            firefox_capabilities['marionette'] = True
-            options = webdriver.firefox.options.Options()
-            if request.config.option.headless:
-                options.add_argument('-headless')
-            browser = webdriver.Firefox(capabilities=firefox_capabilities,
-                                        firefox_options=options)
-        browser.set_window_size(*_WINDOW_SIZE)
+        return webdriver.Remote(desired_capabilities=capabilities,
+                                command_executor=hub_url)
+    if browser_name == 'chrome':
+        chrome_options = webdriver.ChromeOptions()
+        chrome_options.binary_location = '/usr/bin/google-chrome'
+        chrome_options.add_experimental_option(
+            'prefs', {'intl.accept_languages': 'en_US'})
+        chrome_options.add_argument('--lang=en-US')
+        if request.config.option.headless:
+            chrome_options.add_argument('--headless')
+        chrome_browser = webdriver.Chrome(chrome_options=chrome_options)
+        chrome_browser.set_window_size(*_WINDOW_SIZE)
+        return chrome_browser
+    # pylint: disable=line-too-long
+    firefox_capabilities = webdriver.common.desired_capabilities.DesiredCapabilities.FIREFOX  # NOQA
+    firefox_capabilities['marionette'] = True
+    firefox_options = webdriver.firefox.options.Options()
+    if request.config.option.headless:
+        firefox_options.add_argument('-headless')
+    firefox_browser = webdriver.Firefox(
+        capabilities=firefox_capabilities, firefox_options=firefox_options)
+    firefox_browser.set_window_size(*_WINDOW_SIZE)
+    return firefox_browser
+
+
+@pytest.yield_fixture(scope='session')
+def driver(request, browser_name):
+    '''Run tests using the selenium webdriver.'''
 
+    browser = _get_browser(request, browser_name)
     browser.implicitly_wait(_DEFAULT_TIMEOUT)
     wait = WebDriverWait(browser, _DEFAULT_TIMEOUT,
                          poll_frequency=0.1)
@@ -213,10 +229,11 @@ def driver(request, browser):
     finally:
         if _CI:
             print(('\n\nYou can see the report at '
-                   'https://saucelabs.com/beta/tests/%s/commands') % browser.session_id,
-                  file=sys.stderr)
+                   'https://saucelabs.com/beta/tests/%s/commands') %
+                  browser.session_id, file=sys.stderr)
             try:
-                browser.execute_script("sauce:job-result=%s" % str(_SUCCESS).lower())
+                browser.execute_script("sauce:job-result=%s" %
+                                       str(_SUCCESS).lower())
             except WebDriverException:
                 # Test is done. Just ignore the error.
                 pass

frontend/tests/ui/test_smoke.py

@@ -10,6 +10,7 @@
 
 _OMEGAUP_ROOT = os.path.normpath(os.path.join(__file__, '../../../..'))
 
+
 def test_create_user(driver):
     '''Tests basic functionality.'''
 
@@ -38,6 +39,7 @@ def test_create_user(driver):
     with driver.login(username, password):
         pass
 
+
 def test_login(driver):
     '''Tests login with a normal and an admin user.'''
 
@@ -47,6 +49,7 @@ def test_login(driver):
     with driver.login_admin():
         pass
 
+
 def test_create_problem(driver):
     '''Tests creating a public problem and retrieving it.'''
 
@@ -57,7 +60,8 @@ def test_create_problem(driver):
         driver.wait.until(
             EC.element_to_be_clickable(
                 (By.XPATH,
-                 '//li[@id = "nav-problems"]//a[@href = "/problem/new/"]'))).click()
+                 ('//li[@id = "nav-problems"]'
+                  '//a[@href = "/problem/new/"]')))).click()
 
         driver.browser.find_element_by_name('title').send_keys(problem_alias)
         # Alias should be set automatically
@@ -80,7 +84,8 @@ def test_create_problem(driver):
         driver.wait.until(
             EC.element_to_be_clickable(
                 (By.XPATH,
-                 '//li[@id = "nav-problems"]//a[@href = "/problem/"]'))).click()
+                 ('//li[@id = "nav-problems"]'
+                  '//a[@href = "/problem/"]')))).click()
 
         search_box_element = driver.browser.find_element_by_id(
             'problem-search-box')

setup.cfg

@@ -0,0 +1,2 @@
+[pep8]
+ignore = E125,E129