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 <jbettis@google.com>
Change-Id: I01e06bdfbfca239db2092680635fa3bc72008647
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3582802
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Jeremy Bettis <jbettis@chromium.org>
Tested-by: Jeremy Bettis <jbettis@chromium.org>

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"