From 27e162afc7b3da5c4afec43ad0d057b49faf7203 Mon Sep 17 00:00:00 2001 From: Jeremy Bettis Date: Mon, 11 Apr 2022 14:37:45 -0600 Subject: zmake: Fix cros lint warnings in tests Fixed all the cros lint warnings in the zmake tests. BRANCH=None BUG=b:217969201 TEST=zephyr/zmake/run_tests.sh && \ find zephyr/zmake/tests -name '*.py' -print0 | xargs -0 cros lint Signed-off-by: Jeremy Bettis Change-Id: I01e06bdfbfca239db2092680635fa3bc72008647 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3582802 Reviewed-by: Jack Rosenthal Commit-Queue: Jeremy Bettis Tested-by: Jeremy Bettis --- zephyr/zmake/tests/test_build_config.py | 52 +++++++++++++---------- zephyr/zmake/tests/test_modules.py | 2 + zephyr/zmake/tests/test_multiproc_executor.py | 30 +++++++++----- zephyr/zmake/tests/test_multiproc_logging.py | 13 ++++-- zephyr/zmake/tests/test_packers.py | 59 ++++++++++++++++----------- zephyr/zmake/tests/test_project.py | 4 ++ zephyr/zmake/tests/test_util.py | 17 +++++--- zephyr/zmake/tests/test_version.py | 12 ++++++ 8 files changed, 123 insertions(+), 66 deletions(-) diff --git a/zephyr/zmake/tests/test_build_config.py b/zephyr/zmake/tests/test_build_config.py index aab41b84ef..8fb1d2d3d0 100644 --- a/zephyr/zmake/tests/test_build_config.py +++ b/zephyr/zmake/tests/test_build_config.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Tests of zmake's build config system.""" + import argparse import os import pathlib @@ -16,6 +18,8 @@ import zmake.jobserver import zmake.util as util from zmake.build_config import BuildConfig +# pylint:disable=redefined-outer-name,unused-argument + # Strategies for use with hypothesis filenames = st.text( alphabet=set(string.printable) - {"/", ";"}, min_size=1, max_size=254 @@ -71,13 +75,13 @@ def test_merge(coins, combined): kconf1, kconf2 = split(combined.kconfig_defs.items()) files1, files2 = split(combined.kconfig_files) - c1 = BuildConfig( + config1 = BuildConfig( environ_defs=dict(env1), cmake_defs=dict(cmake1), kconfig_defs=dict(kconf1), kconfig_files=files1, ) - c2 = BuildConfig( + config2 = BuildConfig( environ_defs=dict(env2), cmake_defs=dict(cmake2), kconfig_defs=dict(kconf2), @@ -85,7 +89,7 @@ def test_merge(coins, combined): ) # Merge the split configs - merged = c1 | c2 + merged = config1 | config2 # Assert that the merged split configs is the original config assert merged.environ_defs == combined.environ_defs @@ -104,9 +108,10 @@ class FakeJobClient(zmake.jobserver.JobClient): def get_job(self): return zmake.jobserver.JobHandle(lambda: None) - def popen(self, argv, env={}, **kwargs): + def popen(self, argv, **kwargs): + kwargs.setdefault("env", {}) self.captured_argv = [str(arg) for arg in argv] - self.captured_env = {str(k): str(v) for k, v in env.items()} + self.captured_env = {str(k): str(v) for k, v in kwargs["env"].items()} def parse_cmake_args(argv): @@ -150,7 +155,7 @@ def test_popen_cmake_no_kconfig(conf, project_dir, build_dir): job_client = FakeJobClient() conf.popen_cmake(job_client, project_dir, build_dir) - args, cmake_defs = parse_cmake_args(job_client.captured_argv) + _, cmake_defs = parse_cmake_args(job_client.captured_argv) assert cmake_defs == conf.cmake_defs assert job_client.captured_env == conf.environ_defs @@ -171,17 +176,18 @@ def test_popen_cmake_kconfig_but_no_file(conf, project_dir, build_dir): @hypothesis.given(build_configs, paths, paths) @hypothesis.settings(deadline=60000) def test_popen_cmake_kconfig(conf, project_dir, build_dir): + """Test calling popen_cmake and verifying the kconfig_files.""" job_client = FakeJobClient() - with tempfile.NamedTemporaryFile("w", delete=False) as f: - temp_path = f.name + with tempfile.NamedTemporaryFile("w", delete=False) as file: + temp_path = file.name try: conf.popen_cmake( job_client, project_dir, build_dir, kconfig_path=pathlib.Path(temp_path) ) - args, cmake_defs = parse_cmake_args(job_client.captured_argv) + _, cmake_defs = parse_cmake_args(job_client.captured_argv) expected_kconfig_files = set(str(f) for f in conf.kconfig_files) expected_kconfig_files.add(temp_path) @@ -215,9 +221,10 @@ def fake_kconfig_files(tmp_path): def test_build_config_json_stability(fake_kconfig_files): - # as_json() should return equivalent strings for two equivalent - # build configs. - a = BuildConfig( + """as_json() should return equivalent strings for two equivalent + build configs. + """ + config_a = BuildConfig( environ_defs={ "A": "B", "B": "C", @@ -234,7 +241,7 @@ def test_build_config_json_stability(fake_kconfig_files): ) # Dict ordering is intentionally reversed in b. - b = BuildConfig( + config_b = BuildConfig( environ_defs={ "B": "C", "A": "B", @@ -250,20 +257,21 @@ def test_build_config_json_stability(fake_kconfig_files): kconfig_files=list(fake_kconfig_files), ) - assert a.as_json() == b.as_json() + assert config_a.as_json() == config_b.as_json() def test_build_config_json_inequality(): - # Two differing build configs should not have the same json - # representation. - a = BuildConfig(cmake_defs={"A": "B"}) - b = BuildConfig(environ_defs={"A": "B"}) + """Two differing build configs should not have the same json + representation. + """ + config_a = BuildConfig(cmake_defs={"A": "B"}) + config_b = BuildConfig(environ_defs={"A": "B"}) - assert a.as_json() != b.as_json() + assert config_a.as_json() != config_b.as_json() def test_build_config_json_inequality_dtc_changes(tmp_path): - # When DTC overlay files change, so should the JSON. + """When DTC overlay files change, so should the JSON.""" dts_file_1 = tmp_path / "overlay1.dts" dts_file_1.write_text("/* blah */\n") @@ -287,8 +295,8 @@ def test_build_config_json_inequality_dtc_changes(tmp_path): def test_kconfig_file_duplicates(fake_kconfig_files): - # Kconfig files should be like the "uniq" command. Repeats should - # be removed, but not duplicates. + """Kconfig files should be like the "uniq" command. Repeats should + be removed, but not duplicates.""" cfg = BuildConfig( kconfig_files=[ fake_kconfig_files[0], diff --git a/zephyr/zmake/tests/test_modules.py b/zephyr/zmake/tests/test_modules.py index 87e5d7bfc9..600544d2e7 100644 --- a/zephyr/zmake/tests/test_modules.py +++ b/zephyr/zmake/tests/test_modules.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Test zmake modules module.""" + import pathlib import tempfile diff --git a/zephyr/zmake/tests/test_multiproc_executor.py b/zephyr/zmake/tests/test_multiproc_executor.py index ebc2be5e4f..c905ef03ec 100644 --- a/zephyr/zmake/tests/test_multiproc_executor.py +++ b/zephyr/zmake/tests/test_multiproc_executor.py @@ -1,52 +1,62 @@ # Copyright 2021 The Chromium OS Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + +"""Tests for zmake multiproc.""" + import threading import zmake.multiproc def test_single_function_executor_success(): + """Test single function success.""" executor = zmake.multiproc.Executor() executor.append(lambda: 0) assert executor.wait() == 0 def test_single_function_executor_fail(): + """Test single function fail.""" executor = zmake.multiproc.Executor() executor.append(lambda: -2) assert executor.wait() == -2 def test_single_function_executor_raise(): + """Test single function raising an exception.""" executor = zmake.multiproc.Executor() executor.append(lambda: 1 / 0) assert executor.wait() != 0 -def _lock_step(cv, predicate, step, return_value=0): - with cv: - cv.wait_for(predicate=lambda: step[0] == predicate) +def _lock_step(cond, predicate, step, return_value=0): + with cond: + cond.wait_for(predicate=lambda: step[0] == predicate) step[0] += 1 - cv.notify_all() + cond.notify_all() return return_value def test_two_function_executor_wait_for_both(): - cv = threading.Condition() + """Test two functions in executor.""" + cond = threading.Condition() step = [0] executor = zmake.multiproc.Executor() - executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step)) - executor.append(lambda: _lock_step(cv=cv, predicate=1, step=step)) + executor.append(lambda: _lock_step(cond=cond, predicate=0, step=step)) + executor.append(lambda: _lock_step(cond=cond, predicate=1, step=step)) assert executor.wait() == 0 assert step[0] == 2 def test_two_function_executor_one_fails(): - cv = threading.Condition() + """Test two functions in executor, when one fails.""" + cond = threading.Condition() step = [0] executor = zmake.multiproc.Executor() - executor.append(lambda: _lock_step(cv=cv, predicate=0, step=step, return_value=-1)) - executor.append(lambda: _lock_step(cv=cv, predicate=1, step=step)) + executor.append( + lambda: _lock_step(cond=cond, predicate=0, step=step, return_value=-1) + ) + executor.append(lambda: _lock_step(cond=cond, predicate=1, step=step)) assert executor.wait() == -1 assert step[0] == 2 diff --git a/zephyr/zmake/tests/test_multiproc_logging.py b/zephyr/zmake/tests/test_multiproc_logging.py index 2eac9326d3..a64501f5a8 100644 --- a/zephyr/zmake/tests/test_multiproc_logging.py +++ b/zephyr/zmake/tests/test_multiproc_logging.py @@ -1,6 +1,9 @@ # Copyright 2021 The Chromium OS Authors. All rights reserved. # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. + +"""Tests for zmake multiproc logging code.""" + import io import logging import os @@ -11,21 +14,23 @@ import zmake.multiproc def test_read_output_from_pipe(): + """Test reading output from a pipe.""" semaphore = threading.Semaphore(0) pipe = os.pipe() - fd = io.TextIOWrapper(os.fdopen(pipe[0], "rb"), encoding="utf-8") + file_desc = io.TextIOWrapper(os.fdopen(pipe[0], "rb"), encoding="utf-8") logger = mock.Mock(spec=logging.Logger) logger.log.side_effect = lambda log_lvl, line: semaphore.release() - zmake.multiproc.log_output(logger, logging.DEBUG, fd, job_id="") + zmake.multiproc.log_output(logger, logging.DEBUG, file_desc, job_id="") os.write(pipe[1], "Hello\n".encode("utf-8")) semaphore.acquire() logger.log.assert_called_with(logging.DEBUG, "Hello") def test_read_output_change_log_level(): + """Test changing the log level.""" semaphore = threading.Semaphore(0) pipe = os.pipe() - fd = io.TextIOWrapper(os.fdopen(pipe[0], "rb"), encoding="utf-8") + file_desc = io.TextIOWrapper(os.fdopen(pipe[0], "rb"), encoding="utf-8") logger = mock.Mock(spec=logging.Logger) logger.log.side_effect = lambda log_lvl, line: semaphore.release() # This call will log output from fd (the file descriptor) to DEBUG, though @@ -34,7 +39,7 @@ def test_read_output_change_log_level(): zmake.multiproc.log_output( logger=logger, log_level=logging.DEBUG, - file_descriptor=fd, + file_descriptor=file_desc, log_level_override_func=lambda line, lvl: logging.CRITICAL if line.startswith("World") else lvl, diff --git a/zephyr/zmake/tests/test_packers.py b/zephyr/zmake/tests/test_packers.py index 1709c68098..f7b91ac7be 100644 --- a/zephyr/zmake/tests/test_packers.py +++ b/zephyr/zmake/tests/test_packers.py @@ -2,9 +2,10 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Tests for zmake packers.""" + import pathlib import tempfile -import unittest.mock as mock import hypothesis import hypothesis.strategies as st @@ -16,42 +17,52 @@ import zmake.output_packers as packers absolute_path = st.from_regex(regex=r"\A/[\w/]*\Z") -@hypothesis.given(absolute_path) +class FakePacker(packers.BasePacker): + """Fake packer to expose protected methods.""" + + def check_packed_file_size(self, file, dirs): + """Expose the _check_packed_file_size method.""" + return self._check_packed_file_size(file, dirs) + + def _get_max_image_bytes(self): + return 100 + + def pack_firmware(self, work_dir, jobclient, version_string=""): + assert False + + +@hypothesis.given(st.binary(min_size=101, max_size=200)) @hypothesis.settings(deadline=60000) -def test_file_size_unbounded(path): - packer = packers.BasePacker(project=None) - packer._is_size_bound = mock.Mock(name="_is_size_bound", return_value=False) - file = pathlib.Path(path) / "zephyr.bin" - assert packer._check_packed_file_size(file=file, dirs={}) == file - packer._is_size_bound.assert_called_once_with(file) +def test_file_size_unbounded(data): + """Test with file size unbounded.""" + packer = FakePacker(project=None) + with tempfile.TemporaryDirectory() as temp_dir_name: + file = pathlib.Path(temp_dir_name) / "zephyr.elf" + with open(file, "wb") as outfile: + outfile.write(data) + assert packer.check_packed_file_size(file=file, dirs={}) == file @hypothesis.given(st.binary(min_size=5, max_size=100)) @hypothesis.settings(deadline=60000) def test_file_size_in_bounds(data): - packer = packers.BasePacker(project=None) - packer._is_size_bound = mock.Mock(name="_is_size_bound", return_value=True) - packer._get_max_image_bytes = mock.Mock( - name="_get_max_image_bytes", return_value=100 - ) + """Test with file size limited.""" + packer = FakePacker(project=None) with tempfile.TemporaryDirectory() as temp_dir_name: file = pathlib.Path(temp_dir_name) / "zephyr.bin" - with open(file, "wb") as f: - f.write(data) - assert packer._check_packed_file_size(file=file, dirs={}) == file + with open(file, "wb") as outfile: + outfile.write(data) + assert packer.check_packed_file_size(file=file, dirs={}) == file @hypothesis.given(st.binary(min_size=101, max_size=200)) @hypothesis.settings(deadline=60000) def test_file_size_out_of_bounds(data): - packer = packers.BasePacker(project=None) - packer._is_size_bound = mock.Mock(name="_is_size_bound", return_value=True) - packer._get_max_image_bytes = mock.Mock( - name="_get_max_image_bytes", return_value=100 - ) + """Test with file size limited, and file exceeds limit.""" + packer = FakePacker(project=None) with tempfile.TemporaryDirectory() as temp_dir_name: file = pathlib.Path(temp_dir_name) / "zephyr.bin" - with open(file, "wb") as f: - f.write(data) + with open(file, "wb") as outfile: + outfile.write(data) with pytest.raises(RuntimeError): - packer._check_packed_file_size(file=file, dirs={}) + packer.check_packed_file_size(file=file, dirs={}) diff --git a/zephyr/zmake/tests/test_project.py b/zephyr/zmake/tests/test_project.py index f4808d14d5..b5facbc331 100644 --- a/zephyr/zmake/tests/test_project.py +++ b/zephyr/zmake/tests/test_project.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Test zmake project module.""" + import pathlib import string import tempfile @@ -193,6 +195,7 @@ def test_find_projects_name_conflict(tmp_path): def test_register_variant(tmp_path): + """Test registering a variant.""" (tmp_path / "BUILD.py").write_text( """ some = register_raw_project( @@ -236,6 +239,7 @@ another = some_variant.variant( ], ) def test_kconfig_files(tmp_path, actual_files, config_files, expected_files): + """Test for setting kconfig_files property.""" for name in actual_files: (tmp_path / name).write_text("") diff --git a/zephyr/zmake/tests/test_util.py b/zephyr/zmake/tests/test_util.py index 438c5efcf0..1ec0076162 100644 --- a/zephyr/zmake/tests/test_util.py +++ b/zephyr/zmake/tests/test_util.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Tests for zmake util.""" + import pathlib import tempfile @@ -19,12 +21,13 @@ version_tuples = st.tuples(version_integers, version_integers, version_integers) @hypothesis.given(version_tuples) @hypothesis.settings(deadline=60000) def test_read_zephyr_version(version_tuple): + """Test reading the zephyr version.""" with tempfile.TemporaryDirectory() as zephyr_base: - with open(pathlib.Path(zephyr_base) / "VERSION", "w") as f: + with open(pathlib.Path(zephyr_base) / "VERSION", "w") as file: for name, value in zip( ("VERSION_MAJOR", "VERSION_MINOR", "PATCHLEVEL"), version_tuple ): - f.write("{} = {}\n".format(name, value)) + file.write("{} = {}\n".format(name, value)) assert util.read_zephyr_version(zephyr_base) == version_tuple @@ -32,10 +35,11 @@ def test_read_zephyr_version(version_tuple): @hypothesis.given(st.integers()) @hypothesis.settings(deadline=60000) def test_read_kconfig_autoconf_value(value): - with tempfile.TemporaryDirectory() as dir: - path = pathlib.Path(dir) - with open(path / "autoconf.h", "w") as f: - f.write("#define TEST {}".format(value)) + """Test reading the kconfig autoconf.""" + with tempfile.TemporaryDirectory() as temp_dir: + path = pathlib.Path(temp_dir) + with open(path / "autoconf.h", "w") as file: + file.write("#define TEST {}".format(value)) read_value = util.read_kconfig_autoconf_value(path, "TEST") assert int(read_value) == value @@ -52,4 +56,5 @@ def test_read_kconfig_autoconf_value(value): ], ) def test_c_str(input_str, expected_result): + """Test the util.c_str function.""" assert util.c_str(input_str) == expected_result diff --git a/zephyr/zmake/tests/test_version.py b/zephyr/zmake/tests/test_version.py index b2c6b43fec..e56e13a519 100644 --- a/zephyr/zmake/tests/test_version.py +++ b/zephyr/zmake/tests/test_version.py @@ -2,6 +2,8 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +"""Tests for zmake version code.""" + import datetime import subprocess import unittest.mock as mock @@ -12,6 +14,8 @@ import zmake.output_packers import zmake.project import zmake.version as version +# pylint:disable=redefined-outer-name,unused-argument + def _git_init(repo): """Create a new git repository.""" @@ -88,6 +92,7 @@ def _setup_example_repos(tmp_path): def test_version_string(tmp_path): + """Test a that version string is as expected.""" project, zephyr_base, modules = _setup_example_repos(tmp_path) assert ( version.get_version_string(project, zephyr_base, modules) @@ -96,6 +101,7 @@ def test_version_string(tmp_path): def test_version_string_static(tmp_path): + """Test a that version string with no git hashes.""" project, zephyr_base, modules = _setup_example_repos(tmp_path) assert ( version.get_version_string(project, zephyr_base, modules, static=True) @@ -105,6 +111,7 @@ def test_version_string_static(tmp_path): @pytest.fixture def fake_user_hostname(): + """Fixture to provide a fake user and hostname.""" with mock.patch("getpass.getuser", return_value="toukmond", autospec=True): with mock.patch("platform.node", return_value="pokey", autospec=True): yield @@ -112,6 +119,7 @@ def fake_user_hostname(): @pytest.fixture def fake_date(): + """Fixture to provide a fake date.""" fixed_date = datetime.datetime(2021, 6, 28, 3, 18, 53) with mock.patch("datetime.datetime") as mock_datetime: mock_datetime.now.return_value = fixed_date @@ -141,6 +149,7 @@ EXPECTED_HEADER_STATIC = ( def test_header_gen(fake_user_hostname, fake_date, tmp_path): + """Test generating the version header.""" # Test the simple case (static=False, no existing header). output_file = tmp_path / "ec_version.h" version.write_version_header(HEADER_VERSION_STR, output_file) @@ -148,6 +157,7 @@ def test_header_gen(fake_user_hostname, fake_date, tmp_path): def test_header_gen_reproducible_build(tmp_path): + """Test that reproducible builds produce the right header.""" # With static=True this time. output_file = tmp_path / "ec_version.h" version.write_version_header(HEADER_VERSION_STR_STATIC, output_file, static=True) @@ -155,6 +165,7 @@ def test_header_gen_reproducible_build(tmp_path): def test_header_gen_exists_not_changed(fake_user_hostname, fake_date, tmp_path): + """Test that the version file is not changed.""" # Test we don't overwrite if no changes needed. output_file = tmp_path / "ec_version.h" @@ -170,6 +181,7 @@ def test_header_gen_exists_not_changed(fake_user_hostname, fake_date, tmp_path): def test_header_gen_exists_needs_changes(fake_user_hostname, fake_date, tmp_path): + """Test that the version file is changed, when needed.""" # Test we overwrite when it exists already and changes are needed. output_file = tmp_path / "ec_version.h" -- cgit v1.2.1