diff --git a/.pylintrc b/.pylintrc index 037717e35..ad25a7ca1 100644 --- a/.pylintrc +++ b/.pylintrc @@ -40,7 +40,27 @@ max-attributes=15 max-module-lines=2000 [MESSAGES CONTROL] -disable= +# * locally-disabled, locally-enabled: If we disable or enable a message +# locally, it's by design. There's no need to clutter the Pylint output +# with this information. +# * logging-format-interpolation: Pylint warns about things like +# ``log.info('...'.format(...))``. It insists on ``log.info('...', ...)``. +# This is of minor utility (mainly a performance gain when there are +# many messages that use formatting and are below the log level). +# Some versions of Pylint (including 1.8, which is the version on +# Ubuntu 18.04) only recognize old-style format strings using '%', +# and complain about something like ``log.info('{}', foo)`` with +# logging-too-many-args (Pylint supports new-style formatting if +# declared globally with logging_format_style under [LOGGING] but +# this requires Pylint >=2.2). +# * no-else-return: Allow the perfectly reasonable idiom +# if condition1: +# return value1 +# else: +# return value2 +# * unnecessary-pass: If we take the trouble of adding a line with "pass", +# it's because we think the code is clearer that way. +disable=locally-disabled,locally-enabled,logging-format-interpolation,no-else-return,unnecessary-pass [REPORTS] # Don't diplay statistics. Just the facts. diff --git a/scripts/abi_check.py b/scripts/abi_check.py index e19f2c0c6..c2aca501d 100755 --- a/scripts/abi_check.py +++ b/scripts/abi_check.py @@ -29,7 +29,7 @@ from types import SimpleNamespace import xml.etree.ElementTree as ET -class AbiChecker(object): +class AbiChecker: """API and ABI checker.""" def __init__(self, old_version, new_version, configuration): diff --git a/scripts/config.py b/scripts/config.py index b7a9a080e..20521a57a 100755 --- a/scripts/config.py +++ b/scripts/config.py @@ -283,9 +283,13 @@ class ConfigFile(Config): def __init__(self, filename=None): """Read the Mbed TLS configuration file.""" if filename is None: - for filename in self.default_path: - if os.path.lexists(filename): + for candidate in self.default_path: + if os.path.lexists(candidate): + filename = candidate break + else: + raise Exception('Mbed TLS configuration file not found', + self.default_path) super().__init__() self.filename = filename self.current_section = 'header' @@ -448,7 +452,7 @@ if __name__ == '__main__': value = config[args.symbol] if value: sys.stdout.write(value + '\n') - return args.symbol not in config + return 0 if args.symbol in config else 1 elif args.command == 'set': if not args.force and args.symbol not in config.settings: sys.stderr.write("A #define for the symbol {} " @@ -461,6 +465,7 @@ if __name__ == '__main__': else: config.adapt(args.adapter) config.write(args.write) + return 0 # Import modules only used by main only if main is defined and called. # pylint: disable=wrong-import-position diff --git a/tests/scripts/all.sh b/tests/scripts/all.sh index c3d0f6f83..5b70caa22 100755 --- a/tests/scripts/all.sh +++ b/tests/scripts/all.sh @@ -1773,15 +1773,6 @@ component_test_zeroize () { unset gdb_disable_aslr } -support_check_python_files () { - # Find the installed version of Pylint. Installed as a distro package this can - # be pylint3 and as a PEP egg, pylint. - if type pylint >/dev/null 2>/dev/null || type pylint3 >/dev/null 2>/dev/null; then - true; - else - false; - fi -} component_check_python_files () { msg "Lint: Python scripts" record_status tests/scripts/check-python-files.sh diff --git a/tests/scripts/check-files.py b/tests/scripts/check-files.py index 6e35f5224..65edecc78 100755 --- a/tests/scripts/check-files.py +++ b/tests/scripts/check-files.py @@ -17,7 +17,7 @@ import codecs import sys -class FileIssueTracker(object): +class FileIssueTracker: """Base class for file-wide issue tracking. To implement a checker that processes a file as a whole, inherit from @@ -37,20 +37,31 @@ class FileIssueTracker(object): self.files_with_issues = {} def should_check_file(self, filepath): + """Whether the given file name should be checked. + + Files whose name ends with a string listed in ``self.files_exemptions`` + will not be checked. + """ for files_exemption in self.files_exemptions: if filepath.endswith(files_exemption): return False return True def check_file_for_issue(self, filepath): + """Check the specified file for the issue that this class is for. + + Subclasses must implement this method. + """ raise NotImplementedError def record_issue(self, filepath, line_number): + """Record that an issue was found at the specified location.""" if filepath not in self.files_with_issues.keys(): self.files_with_issues[filepath] = [] self.files_with_issues[filepath].append(line_number) def output_file_issues(self, logger): + """Log all the locations where the issue was found.""" if self.files_with_issues.values(): logger.info(self.heading) for filename, lines in sorted(self.files_with_issues.items()): @@ -70,6 +81,10 @@ class LineIssueTracker(FileIssueTracker): """ def issue_with_line(self, line, filepath): + """Check the specified line for the issue that this class is for. + + Subclasses must implement this method. + """ raise NotImplementedError def check_file_line(self, filepath, line, line_number): @@ -77,6 +92,10 @@ class LineIssueTracker(FileIssueTracker): self.record_issue(filepath, line_number) def check_file_for_issue(self, filepath): + """Check the lines of the specified file. + + Subclasses must implement the ``issue_with_line`` method. + """ with open(filepath, "rb") as f: for i, line in enumerate(iter(f.readline, b"")): self.check_file_line(filepath, line, i + 1) @@ -170,7 +189,7 @@ class MergeArtifactIssueTracker(LineIssueTracker): return False -class IntegrityChecker(object): +class IntegrityChecker: """Sanity-check files under the current directory.""" def __init__(self, log_file): diff --git a/tests/scripts/check-python-files.sh b/tests/scripts/check-python-files.sh index 6b864d264..cd18518ca 100755 --- a/tests/scripts/check-python-files.sh +++ b/tests/scripts/check-python-files.sh @@ -9,15 +9,10 @@ # Run 'pylint' on Python files for programming errors and helps enforcing # PEP8 coding standards. -# Find the installed version of Pylint. Installed as a distro package this can -# be pylint3 and as a PEP egg, pylint. We prefer pylint over pylint3 -if type pylint >/dev/null 2>/dev/null; then - PYLINT=pylint -elif type pylint3 >/dev/null 2>/dev/null; then - PYLINT=pylint3 +if type python3 >/dev/null 2>/dev/null; then + PYTHON=python3 else - echo 'Pylint was not found.' - exit 1 + PYTHON=python fi -$PYLINT -j 2 scripts/*.py tests/scripts/*.py +$PYTHON -m pylint -j 2 scripts/*.py tests/scripts/*.py diff --git a/tests/scripts/check-test-cases.py b/tests/scripts/check-test-cases.py index 4abaa6882..35a998749 100755 --- a/tests/scripts/check-test-cases.py +++ b/tests/scripts/check-test-cases.py @@ -77,6 +77,7 @@ def check_description(results, seen, file_name, line_number, description): seen[description] = line_number def check_test_suite(results, data_file_name): + """Check the test cases in the given unit test data file.""" in_paragraph = False descriptions = {} with open(data_file_name, 'rb') as data_file: @@ -94,6 +95,7 @@ def check_test_suite(results, data_file_name): in_paragraph = True def check_ssl_opt_sh(results, file_name): + """Check the test cases in ssl-opt.sh or a file with a similar format.""" descriptions = {} with open(file_name, 'rb') as file_contents: for line_number, line in enumerate(file_contents, 1): diff --git a/tests/scripts/generate_test_code.py b/tests/scripts/generate_test_code.py index 1fff09992..21f816ea9 100755 --- a/tests/scripts/generate_test_code.py +++ b/tests/scripts/generate_test_code.py @@ -208,7 +208,7 @@ class GeneratorInputError(Exception): pass -class FileWrapper(io.FileIO, object): +class FileWrapper(io.FileIO): """ This class extends built-in io.FileIO class with attribute line_no, that indicates line number for the line that is read. @@ -402,8 +402,7 @@ def parse_dependencies(inp_str): :param inp_str: Input string with macros delimited by ':'. :return: list of dependencies """ - dependencies = [dep for dep in map(validate_dependency, - inp_str.split(':'))] + dependencies = list(map(validate_dependency, inp_str.split(':'))) return dependencies diff --git a/tests/scripts/mbedtls_test.py b/tests/scripts/mbedtls_test.py index 8f24435bf..709bb1a3e 100755 --- a/tests/scripts/mbedtls_test.py +++ b/tests/scripts/mbedtls_test.py @@ -1,3 +1,5 @@ +#!/usr/bin/env python3 + # Greentea host test script for Mbed TLS on-target test suite testing. # # Copyright (C) 2018, Arm Limited, All Rights Reserved @@ -46,7 +48,7 @@ class TestDataParserError(Exception): pass -class TestDataParser(object): +class TestDataParser: """ Parses test name, dependencies, test function name and test parameters from the data file. @@ -260,7 +262,7 @@ class MbedTlsTest(BaseHostTest): data_bytes += bytearray(dependencies) data_bytes += bytearray([function_id, len(parameters)]) for typ, param in parameters: - if typ == 'int' or typ == 'exp': + if typ in ('int', 'exp'): i = int(param, 0) data_bytes += b'I' if typ == 'int' else b'E' self.align_32bit(data_bytes) diff --git a/tests/scripts/test_config_script.py b/tests/scripts/test_config_script.py index 40ed9fd9b..c8fdea5ee 100755 --- a/tests/scripts/test_config_script.py +++ b/tests/scripts/test_config_script.py @@ -92,6 +92,7 @@ def list_presets(options): return re.split(r'[ ,]+', options.presets) else: help_text = subprocess.run([options.script, '--help'], + check=False, # config.pl --help returns 255 stdout=subprocess.PIPE, stderr=subprocess.STDOUT).stdout return guess_presets_from_help(help_text.decode('ascii')) diff --git a/tests/scripts/test_generate_test_code.py b/tests/scripts/test_generate_test_code.py index 6d7113e18..c8e8c5ce1 100755 --- a/tests/scripts/test_generate_test_code.py +++ b/tests/scripts/test_generate_test_code.py @@ -294,7 +294,7 @@ class GenDispatch(TestCase): self.assertEqual(code, expected) -class StringIOWrapper(StringIO, object): +class StringIOWrapper(StringIO): """ file like class to mock file object in tests. """ @@ -1127,9 +1127,8 @@ Diffie-Hellman selftest dhm_selftest: """ stream = StringIOWrapper('test_suite_ut.function', data) - tests = [(name, test_function, dependencies, args) - for name, test_function, dependencies, args in - parse_test_data(stream)] + # List of (name, function_name, dependencies, args) + tests = list(parse_test_data(stream)) test1, test2, test3, test4 = tests self.assertEqual(test1[0], 'Diffie-Hellman full exchange #1') self.assertEqual(test1[1], 'dhm_do_dhm') @@ -1170,9 +1169,8 @@ dhm_do_dhm:10:"93450983094850938450983409623":10:"9345098304850938450983409622" """ stream = StringIOWrapper('test_suite_ut.function', data) - tests = [(name, function_name, dependencies, args) - for name, function_name, dependencies, args in - parse_test_data(stream)] + # List of (name, function_name, dependencies, args) + tests = list(parse_test_data(stream)) test1, test2 = tests self.assertEqual(test1[0], 'Diffie-Hellman full exchange #1') self.assertEqual(test1[1], 'dhm_do_dhm') diff --git a/tests/scripts/test_psa_constant_names.py b/tests/scripts/test_psa_constant_names.py index c02555e88..2c9f058ea 100755 --- a/tests/scripts/test_psa_constant_names.py +++ b/tests/scripts/test_psa_constant_names.py @@ -453,7 +453,7 @@ def main(): tests.run_all(inputs) tests.report(sys.stdout) if tests.errors: - exit(1) + sys.exit(1) if __name__ == '__main__': main()