Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixing some violations to assuage pylint and get a bit of overhead

Browse Source
This commit is contained in:
bmedx
2018-01-18 16:11:38 -05:00
parent 1d45c85135
commit 40951a28cf
17 changed files with 42 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
pavelib/assets.py
View File

@@ -287,7 +287,7 @@ def debounce(seconds=1):
@wraps(func)
def wrapper(*args, **kwargs): # pylint: disable=missing-docstring
def call(): # pylint: disable=missing-docstring
def call():
func(*args, **kwargs)
func.timer = None
if func.timer:
@@ -347,8 +347,9 @@ class SassWatcher(PatternMatchingEventHandler):
paths.extend(glob.glob(dirname))
else:
paths.append(dirname)
for dirname in paths:
observer.schedule(self, dirname, recursive=True)
for obs_dirname in paths:
observer.schedule(self, obs_dirname, recursive=True)
@debounce()
def on_any_event(self, event):

2
pavelib/i18n.py
View File

@@ -315,7 +315,7 @@ def find_release_resources():
if len(resources) == 2:
return resources
if len(resources) == 0:
if not resources:
raise ValueError("You need two release-* resources defined to use this command.")
else:
msg = "Strange Transifex config! Found these release-* resources:\n" + "\n".join(resources)

4
pavelib/paver_tests/test_assets.py
View File

@@ -14,7 +14,7 @@ from ..utils.envs import Env
from .utils import PaverTestCase
ROOT_PATH = path(os.path.dirname(os.path.dirname(os.path.dirname(__file__))))
TEST_THEME_DIR = ROOT_PATH / "common/test/test-theme" # pylint: disable=invalid-name
TEST_THEME_DIR = ROOT_PATH / "common/test/test-theme"
class TestPaverWatchAssetTasks(TestCase):
@@ -162,7 +162,7 @@ class TestCollectAssets(PaverTestCase):
for i, sys in enumerate(systems):
msg = self.task_messages[i]
self.assertTrue(msg.startswith('python manage.py {}'.format(sys)))
self.assertIn(' collectstatic '.format(Env.DEVSTACK_SETTINGS), msg)
self.assertIn(' collectstatic ', msg)
self.assertIn('--settings={}'.format(Env.DEVSTACK_SETTINGS), msg)
self.assertTrue(msg.endswith(' {}'.format(log_location)))

8
pavelib/paver_tests/test_database.py
View File

@@ -110,7 +110,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase):
fingerprint_file.write(self.expected_fingerprint)
with patch.object(db_utils, 'get_file_from_s3', wraps=db_utils.get_file_from_s3) as _mock_get_file:
database.update_local_bokchoy_db_from_s3()
database.update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter
# Make sure that the local cache files are used - NOT downloaded from s3
self.assertFalse(_mock_get_file.called)
calls = [
@@ -149,7 +149,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase):
fingerprint_file.write(local_fingerprint)
with patch.object(db_utils, 'get_file_from_s3', wraps=db_utils.get_file_from_s3) as _mock_get_file:
database.update_local_bokchoy_db_from_s3()
database.update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter
# Make sure that the fingerprint file is downloaded from s3
_mock_get_file.assert_called_once_with(
'moto_test_bucket', self.fingerprint_filename, db_utils.CACHE_FOLDER
@@ -181,7 +181,7 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase):
with open(db_utils.FINGERPRINT_FILEPATH, 'w') as fingerprint_file:
fingerprint_file.write(local_fingerprint)
database.update_local_bokchoy_db_from_s3()
database.update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter
calls = [
call('{}/scripts/reset-test-db.sh --calculate_migrations'.format(Env.REPO_ROOT)),
call('{}/scripts/reset-test-db.sh --rebuild_cache'.format(Env.REPO_ROOT))
@@ -208,5 +208,5 @@ class TestPaverDatabaseTasks(MockS3Mixin, TestCase):
with open(db_utils.FINGERPRINT_FILEPATH, 'w') as fingerprint_file:
fingerprint_file.write(local_fingerprint)
database.update_local_bokchoy_db_from_s3()
database.update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter
self.assertTrue(self.bucket.get_key(self.fingerprint_filename))

4
pavelib/paver_tests/test_paver_quality.py
View File

@@ -12,10 +12,10 @@ import paver.tasks
from ddt import ddt, file_data, data, unpack
from mock import MagicMock, mock_open, patch
from path import Path as path
from paver.easy import BuildFailure
from paver.easy import BuildFailure # pylint: disable=ungrouped-imports
import pavelib.quality
from pavelib.paver_tests.utils import fail_on_eslint, fail_on_pylint
from pavelib.paver_tests.utils import fail_on_eslint
@ddt

9
pavelib/paver_tests/test_servers.py
View File

@@ -203,6 +203,7 @@ class TestPaverServerTasks(PaverTestCase):
]
)
# pylint: disable=too-many-statements
def verify_server_task(self, task_name, options, contracts_default=False):
"""
Verify the output of a server task.
@@ -247,7 +248,9 @@ class TestPaverServerTasks(PaverTestCase):
expected_messages.append(u"xmodule_assets common/static/xmodule")
expected_messages.append(u"install npm_assets")
expected_messages.append(EXPECTED_COFFEE_COMMAND.format(platform_root=self.platform_root))
expected_messages.extend([c.format(settings=expected_asset_settings) for c in EXPECTED_PRINT_SETTINGS_COMMAND])
expected_messages.extend(
[c.format(settings=expected_asset_settings) for c in EXPECTED_PRINT_SETTINGS_COMMAND]
)
expected_messages.append(EXPECTED_WEBPACK_COMMAND.format(
node_env="production" if expected_asset_settings != Env.DEVSTACK_SETTINGS else "development",
static_root_lms=None,
@@ -291,7 +294,9 @@ class TestPaverServerTasks(PaverTestCase):
expected_messages.append(u"xmodule_assets common/static/xmodule")
expected_messages.append(u"install npm_assets")
expected_messages.append(EXPECTED_COFFEE_COMMAND.format(platform_root=self.platform_root))
expected_messages.extend([c.format(settings=expected_asset_settings) for c in EXPECTED_PRINT_SETTINGS_COMMAND])
expected_messages.extend(
[c.format(settings=expected_asset_settings) for c in EXPECTED_PRINT_SETTINGS_COMMAND]
)
expected_messages.append(EXPECTED_WEBPACK_COMMAND.format(
node_env="production" if expected_asset_settings != Env.DEVSTACK_SETTINGS else "development",
static_root_lms=None,

5
pavelib/paver_tests/test_stylelint.py
View File

@@ -13,11 +13,6 @@ class TestPaverStylelint(PaverTestCase):
"""
Tests for Paver's Stylelint tasks.
"""
def setUp(self):
super(TestPaverStylelint, self).setUp()
pass
@ddt.data(
[0, False],
[99, False],

8
pavelib/paver_tests/utils.py
View File

@@ -63,7 +63,7 @@ class MockEnvironment(tasks.Environment):
self.messages.append(unicode(output))
def fail_on_eslint(*args, **kwargs):
def fail_on_eslint(*args):
"""
For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what
is going to fail when we pass in a percentage ("p") requirement.
@@ -75,7 +75,7 @@ def fail_on_eslint(*args, **kwargs):
return
def fail_on_pylint(*args, **kwargs):
def fail_on_pylint(*args):
"""
For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what
is going to fail when we pass in a percentage ("p") requirement.
@@ -87,7 +87,7 @@ def fail_on_pylint(*args, **kwargs):
return
def fail_on_npm_install(*args, **kwargs):
def fail_on_npm_install(*args):
"""
For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what
is going to fail when we pass in a percentage ("p") requirement.
@@ -98,7 +98,7 @@ def fail_on_npm_install(*args, **kwargs):
return
def unexpected_fail_on_npm_install(*args, **kwargs):
def unexpected_fail_on_npm_install(*args):
"""
For our tests, we need the call for diff-quality running pep8 reports to fail, since that is what
is going to fail when we pass in a percentage ("p") requirement.

5
pavelib/quality.py
View File

@@ -134,7 +134,7 @@ def run_pylint(options):
errors = getattr(options, 'errors', False)
systems = getattr(options, 'system', ALL_SYSTEMS).split(',')
num_violations, violations_list = _get_pylint_violations(systems, errors)
num_violations, _ = _get_pylint_violations(systems, errors)
# Print number of violations to log
violations_count_str = "Number of pylint violations: " + str(num_violations)
@@ -496,7 +496,7 @@ def run_xsslint(options):
violations_limit=violation_thresholds['rules'][threshold_key],
)
if error_message is not "":
if error_message:
raise BuildFailure(
"FAILURE: XSSLinter Failed.\n{error_message}\n"
"See {xsslint_report} or run the following command to hone in on the problem:\n"
@@ -688,6 +688,7 @@ def _get_xsscommitlint_count(filename):
("limit=", "l", "Limits for number of acceptable violations - either <upper> or <lower>:<upper>"),
])
@timed
# pylint: disable=too-many-statements
def run_quality(options):
"""
Build the html diff quality reports, and print the reports to the console.

6
pavelib/tests.py
View File

@@ -80,8 +80,8 @@ def test_system(options, passthrough_options):
test_id = getattr(options, 'test_id', None)
django_version = getattr(options, 'django_version', None)
assert(system in (None, 'lms', 'cms'))
assert(django_version in (None, '1.8', '1.9', '1.10', '1.11'))
assert system in (None, 'lms', 'cms')
assert django_version in (None, '1.8', '1.9', '1.10', '1.11')
if test_id:
# Testing a single test ID.
@@ -159,7 +159,7 @@ def test_lib(options, passthrough_options):
test_id = getattr(options, 'test_id', lib)
django_version = getattr(options, 'django_version', None)
assert(django_version in (None, '1.8', '1.9', '1.10', '1.11'))
assert django_version in (None, '1.8', '1.9', '1.10', '1.11')
if test_id:
# Testing a single test id.

2
pavelib/utils/envs.py
View File

@@ -225,7 +225,7 @@ class Env(object):
SERVICE_VARIANT = 'lms'
@classmethod
def get_django_setting(self, django_setting, system, settings=None):
def get_django_setting(cls, django_setting, system, settings=None):
"""
Interrogate Django environment for specific settings values
:param django_setting: the django setting to get

4
pavelib/utils/process.py
View File

@@ -18,8 +18,6 @@ def kill_process(proc):
Kill the process `proc` created with `subprocess`.
"""
p1_group = psutil.Process(proc.pid)
# pylint: disable=unexpected-keyword-arg
child_pids = p1_group.get_children(recursive=True)
for child_pid in child_pids:
@@ -112,8 +110,6 @@ def run_background_process(cmd, out_log=None, err_log=None, cwd=None):
killed properly.
"""
p1_group = psutil.Process(proc.pid)
# pylint: disable=unexpected-keyword-arg
child_pids = p1_group.get_children(recursive=True)
for child_pid in child_pids:

8
pavelib/utils/test/suites/acceptance_suite.py
View File

@@ -37,22 +37,22 @@ def setup_acceptance_db():
definitions to sync and migrate.
"""
for db in DBS.keys():
for db in DBS:
if DBS[db].isfile():
# Since we are using SQLLite, we can reset the database by deleting it on disk.
DBS[db].remove()
settings = 'acceptance_docker' if Env.USING_DOCKER else 'acceptance'
if all(DB_CACHES[cache].isfile() for cache in DB_CACHES.keys()):
if all(DB_CACHES[cache].isfile() for cache in DB_CACHES):
# To speed up migrations, we check for a cached database file and start from that.
# The cached database file should be checked into the repo
# Copy the cached database to the test root directory
for db_alias in DBS.keys():
for db_alias in DBS:
sh("cp {db_cache} {db}".format(db_cache=DB_CACHES[db_alias], db=DBS[db_alias]))
# Run migrations to update the db, starting from its cached state
for db_alias in sorted(DBS.keys()):
for db_alias in sorted(DBS):
# pylint: disable=line-too-long
sh("./manage.py lms --settings {} migrate --traceback --noinput --fake-initial --database {}".format(settings, db_alias))
sh("./manage.py cms --settings {} migrate --traceback --noinput --fake-initial --database {}".format(settings, db_alias))

7
pavelib/utils/test/suites/bokchoy_suite.py
View File

@@ -1,6 +1,7 @@
"""
Class used for defining and running Bok Choy acceptance test suite
"""
import os
from time import sleep
from textwrap import dedent
@@ -23,8 +24,6 @@ from pavelib.utils.test import utils as test_utils
from pavelib.utils.timer import timed
from pavelib.database import update_local_bokchoy_db_from_s3
import os
try:
from pygments.console import colorize
except ImportError:
@@ -142,7 +141,7 @@ def reset_test_database():
If not, reset the test database and apply migrations
"""
if os.environ.get('USER', None) == 'jenkins':
update_local_bokchoy_db_from_s3()
update_local_bokchoy_db_from_s3() # pylint: disable=no-value-for-parameter
else:
sh("{}/scripts/reset-test-db.sh --migrations".format(Env.REPO_ROOT))
@@ -238,7 +237,7 @@ class BokChoyTestSuite(TestSuite):
check_services()
if not self.testsonly:
call_task('prepare_bokchoy_run', options={'log_dir': self.log_dir}) # pylint: disable=no-value-for-parameter
call_task('prepare_bokchoy_run', options={'log_dir': self.log_dir})
else:
# load data in db_fixtures
load_bok_choy_data() # pylint: disable=no-value-for-parameter

8
pavelib/utils/test/suites/pytest_suite.py
View File

@@ -7,10 +7,6 @@ from pavelib.utils.test import utils as test_utils
from pavelib.utils.test.suites.suite import TestSuite
from pavelib.utils.envs import Env
try:
from pygments.console import colorize
except ImportError:
colorize = lambda color, text: text
__test__ = False # do not collect
@@ -123,12 +119,8 @@ class SystemTestSuite(PytestSuite):
self.processes = int(self.processes)
def __enter__(self):
super(SystemTestSuite, self).__enter__()
@property
def cmd(self):
if self.django_toxenv:
cmd = ['tox', '-e', self.django_toxenv, '--']
else:

6
pavelib/utils/test/suites/suite.py
View File

@@ -114,14 +114,14 @@ class TestSuite(object):
for suite in self.subsuites:
suite.run_suite_tests()
if len(suite.failed_suites) > 0:
if suite.failed_suites:
self.failed_suites.extend(suite.failed_suites)
def report_test_results(self):
"""
Writes a list of failed_suites to sys.stderr
"""
if len(self.failed_suites) > 0:
if self.failed_suites:
msg = colorize('red', "\n\n{bar}\nTests failed in the following suites:\n* ".format(bar="=" * 48))
msg += colorize('red', '\n* '.join([s.root for s in self.failed_suites]) + '\n\n')
else:
@@ -140,5 +140,5 @@ class TestSuite(object):
self.report_test_results()
if len(self.failed_suites) > 0:
if self.failed_suites:
sys.exit(1)

2
pavelib/utils/timer.py
View File

@@ -37,7 +37,7 @@ def timed(wrapped, instance, args, kwargs): # pylint: disable=unused-argument
exception_info = {}
try:
return wrapped(*args, **kwargs)
except Exception as exc: # pylint: disable=broad-except
except Exception as exc:
exception_info = {
'exception': "".join(traceback.format_exception_only(type(exc), exc)).strip()
}