zmake: Fix unit tests outside chroot

Fixed various bugs with running zmake tests outside of the chroot.

BRANCH=None
BUG=b:219062927
TEST=Ran commands in docker

Signed-off-by: Jeremy Bettis <jbettis@google.com>
Change-Id: I942d4195b97384239869ac1d928c9eca47c70829
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3472687
Tested-by: Jeremy Bettis <jbettis@chromium.org>
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Commit-Queue: Jeremy Bettis <jbettis@chromium.org>

4 files changed, 107 insertions, 85 deletions

diff --git a/zephyr/zmake/tests/conftest.py b/zephyr/zmake/tests/conftest.py
index 4572e23ffe..46030f4753 100644
--- a/zephyr/zmake/tests/conftest.py
+++ b/zephyr/zmake/tests/conftest.py
@@ -2,8 +2,52 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
+import os
+import pathlib
+
 import hypothesis
+import pytest
+
+import zmake.zmake as zm
 
 hypothesis.settings.register_profile(
     "cq", suppress_health_check=hypothesis.HealthCheck.all()
 )
+
+
+@pytest.fixture
+def zmake_factory_from_dir(tmp_path):
+    """Creates module dirs and returns a Zmake object."""
+
+    os.mkdir(tmp_path / "ec")
+    os.mkdir(tmp_path / "ec" / "zephyr")
+    (tmp_path / "ec" / "zephyr" / "module.yml").write_text("")
+    zephyr_base = tmp_path / "zephyr_base"
+
+    def _zmake_factory(**kwargs):
+        return zm.Zmake(zephyr_base=zephyr_base, modules_dir=tmp_path, **kwargs)
+
+    return _zmake_factory
+
+
+@pytest.fixture
+def zmake_from_dir(zmake_factory_from_dir):
+    return zmake_factory_from_dir()
+
+
+@pytest.fixture
+def fake_checkout(tmp_path):
+    """Creates a fake checkout dir and returns the path to zmake."""
+
+    actual_zmake_src_path = pathlib.Path(__file__).parent.parent
+    fake_zmake_path = pathlib.Path(tmp_path) / "src/platform/ec/zephyr/zmake"
+    os.makedirs(fake_zmake_path.parent)
+    os.symlink(actual_zmake_src_path, fake_zmake_path)
+    return fake_zmake_path
+
+
+@pytest.fixture
+def zmake_from_checkout(tmp_path, fake_checkout):
+    """Creates a fake checkout dir and returns a Zmake object."""
+
+    return zm.Zmake(checkout=tmp_path)
diff --git a/zephyr/zmake/tests/test_generate_readme.py b/zephyr/zmake/tests/test_generate_readme.py
index 2a05de9cce..b20e3e2fdb 100644
--- a/zephyr/zmake/tests/test_generate_readme.py
+++ b/zephyr/zmake/tests/test_generate_readme.py
@@ -5,7 +5,6 @@
 import pytest
 
 import zmake.generate_readme as gen_readme
-import zmake.zmake as zm
 
 
 def test_generate_readme_contents():
@@ -24,7 +23,12 @@ def test_generate_readme_contents():
     ],
 )
 def test_generate_readme_diff(
-    monkeypatch, tmp_path, expected_contents, actual_contents, return_code
+    monkeypatch,
+    tmp_path,
+    expected_contents,
+    actual_contents,
+    return_code,
+    zmake_from_dir,
 ):
     def generate_readme():
         return expected_contents
@@ -35,12 +39,11 @@ def test_generate_readme_diff(
     if actual_contents is not None:
         readme_file.write_text(actual_contents)
 
-    zmk = zm.Zmake()
-    assert zmk.generate_readme(readme_file, diff=True) == return_code
+    assert zmake_from_dir.generate_readme(readme_file, diff=True) == return_code
 
 
 @pytest.mark.parametrize("exist", [False, True])
-def test_generate_readme_file(monkeypatch, tmp_path, exist):
+def test_generate_readme_file(monkeypatch, tmp_path, exist, zmake_from_dir):
     def generate_readme():
         return "hello\n"
 
@@ -50,6 +53,5 @@ def test_generate_readme_file(monkeypatch, tmp_path, exist):
     if exist:
         readme_file.write_text("some existing contents\n")
 
-    zmk = zm.Zmake()
-    assert zmk.generate_readme(readme_file) == 0
+    assert zmake_from_dir.generate_readme(readme_file) == 0
     assert readme_file.read_text() == "hello\n"
diff --git a/zephyr/zmake/tests/test_reexec.py b/zephyr/zmake/tests/test_reexec.py
index 9f25b5a834..7922511a48 100644
--- a/zephyr/zmake/tests/test_reexec.py
+++ b/zephyr/zmake/tests/test_reexec.py
@@ -46,12 +46,12 @@ def test_zmake_does_not_exist(fake_env, mock_execve):
     mock_execve.assert_not_called()
 
 
-def test_zmake_reexec(fake_env, mock_execve):
-    # Nothing else applies?  The re-exec should happen.
-    fake_env["CROS_WORKON_SRCROOT"] = "/mnt/host/source"
+def test_zmake_reexec(fake_env, mock_execve, tmp_path, fake_checkout):
+    # Nothing else applies?  The re-exec should happen, when in a checkout
+    fake_env["CROS_WORKON_SRCROOT"] = tmp_path
     main.maybe_reexec(["--help"])
     new_env = dict(fake_env)
-    new_env["PYTHONPATH"] = "/mnt/host/source/src/platform/ec/zephyr/zmake"
+    new_env["PYTHONPATH"] = str(fake_checkout.resolve())
     mock_execve.assert_called_once_with(
         sys.executable,
         [sys.executable, "-m", "zmake", "--help"],
diff --git a/zephyr/zmake/tests/test_zmake.py b/zephyr/zmake/tests/test_zmake.py
index cce837ccbc..30942a74c9 100644
--- a/zephyr/zmake/tests/test_zmake.py
+++ b/zephyr/zmake/tests/test_zmake.py
@@ -8,10 +8,7 @@ import logging
 import os
 import pathlib
 import re
-import tempfile
-import unittest
-import unittest.mock as mock
-from unittest.mock import patch
+import unittest.mock
 
 import pytest
 from testfixtures import LogCapture
@@ -22,7 +19,6 @@ import zmake.multiproc as multiproc
 import zmake.output_packers
 import zmake.project
 import zmake.toolchains
-import zmake.zmake as zm
 
 OUR_PATH = os.path.dirname(os.path.realpath(__file__))
 
@@ -33,8 +29,8 @@ class FakeProject:
     # pylint: disable=too-few-public-methods
 
     def __init__(self):
-        self.packer = mock.Mock()
-        self.packer.pack_firmware = mock.Mock(return_value=[])
+        self.packer = unittest.mock.Mock()
+        self.packer.pack_firmware = unittest.mock.Mock(return_value=[])
 
         self.config = zmake.project.ProjectConfig(
             project_name="fakeproject",
@@ -79,7 +75,7 @@ class FakeJobserver(zmake.jobserver.GNUMakeJobServer):
 
     def get_job(self):
         """Fake implementation of get_job(), which returns a real JobHandle()"""
-        return zmake.jobserver.JobHandle(mock.Mock())
+        return zmake.jobserver.JobHandle(unittest.mock.Mock())
 
     # pylint: disable=arguments-differ
     def popen(self, cmd, *args, **kwargs):
@@ -108,7 +104,7 @@ def get_test_filepath(suffix):
     return os.path.join(OUR_PATH, "files", "sample_{}.txt".format(suffix))
 
 
-def do_test_with_log_level(log_level, fnames=None):
+def do_test_with_log_level(zmake_factory_from_dir, log_level, fnames=None):
     """Test filtering using a particular log level
 
     Args:
@@ -119,9 +115,7 @@ def do_test_with_log_level(log_level, fnames=None):
             (None to use default ro/rw output)
 
     Returns:
-        tuple:
-            - List of log strings obtained from the run
-            - Temporary directory used for build
+        - List of log strings obtained from the run
     """
     if fnames is None:
         fnames = {
@@ -129,68 +123,51 @@ def do_test_with_log_level(log_level, fnames=None):
             re.compile(r".*build-rw"): get_test_filepath("rw"),
         }
 
-    with tempfile.TemporaryDirectory() as tmpname:
-        tmpname = pathlib.Path(tmpname)
-        os.mkdir(tmpname / "ec")
-        os.mkdir(tmpname / "ec" / "zephyr")
-        with open(tmpname / "ec" / "zephyr" / "module.yml", "w") as fd:
-            fd.write("hi")
-        zephyr_base = tmpname / "zephyr_base"
-        zmk = zm.Zmake(
-            jobserver=FakeJobserver(fnames),
-            zephyr_base=zephyr_base,
-            modules_dir=tmpname,
-        )
-
-        with LogCapture(level=log_level) as cap:
-            with open(tmpname / "VERSION", "w") as fd:
-                fd.write(
-                    """VERSION_MAJOR = 2
-VERSION_MINOR = 5
-PATCHLEVEL = 0
-VERSION_TWEAK = 0
-EXTRAVERSION =
-"""
-                )
-            with patch("zmake.version.get_version_string", return_value="123"):
-                with patch.object(
-                    zmake.project,
-                    "find_projects",
-                    return_value={"fakeproject": FakeProject()},
+    zmk = zmake_factory_from_dir(jobserver=FakeJobserver(fnames))
+    with LogCapture(level=log_level) as cap:
+        with unittest.mock.patch(
+            "zmake.version.get_version_string", return_value="123"
+        ):
+            with unittest.mock.patch.object(
+                zmake.project,
+                "find_projects",
+                return_value={"fakeproject": FakeProject()},
+            ):
+                with unittest.mock.patch(
+                    "zmake.version.write_version_header", autospec=True
                 ):
-                    with patch("zmake.version.write_version_header", autospec=True):
-                        zmk.build(
-                            ["fakeproject"],
-                            clobber=True,
-                        )
-                    multiproc.wait_for_log_end()
+                    zmk.build(
+                        ["fakeproject"],
+                        clobber=True,
+                    )
+                multiproc.wait_for_log_end()
 
     recs = [rec.getMessage() for rec in cap.records]
-    return recs, tmpname
+    return recs
 
 
-class TestFilters(unittest.TestCase):
+class TestFilters:
     """Test filtering of stdout and stderr"""
 
-    def test_filter_normal(self):
+    def test_filter_normal(self, zmake_factory_from_dir):
         """Test filtering of a normal build (with no errors)"""
-        recs, _ = do_test_with_log_level(logging.ERROR)
-        self.assertFalse(recs)
+        recs = do_test_with_log_level(zmake_factory_from_dir, logging.ERROR)
+        assert not recs
 
-    def test_filter_info(self):
+    def test_filter_info(self, zmake_factory_from_dir, tmp_path):
         """Test what appears on the INFO level"""
-        recs, tmpname = do_test_with_log_level(logging.INFO)
+        recs = do_test_with_log_level(zmake_factory_from_dir, logging.INFO)
         # TODO: Remove sets and figure out how to check the lines are in the
         # right order.
         expected = {
             "Configuring fakeproject:build-rw.",
             "Configuring fakeproject:build-ro.",
-            "Building fakeproject in {}/ec/build/zephyr/fakeproject.".format(tmpname),
+            "Building fakeproject in {}/ec/build/zephyr/fakeproject.".format(tmp_path),
             "Building fakeproject:build-ro: /usr/bin/ninja -C {}-build-ro".format(
-                tmpname / "ec/build/zephyr/fakeproject/build"
+                tmp_path / "ec/build/zephyr/fakeproject/build"
             ),
             "Building fakeproject:build-rw: /usr/bin/ninja -C {}-build-rw".format(
-                tmpname / "ec/build/zephyr/fakeproject/build"
+                tmp_path / "ec/build/zephyr/fakeproject/build"
             ),
         }
         for suffix in ["ro", "rw"]:
@@ -200,22 +177,22 @@ class TestFilters(unittest.TestCase):
                         "[fakeproject:build-{}]{}".format(suffix, line.strip())
                     )
         # This produces an easy-to-read diff if there is a difference
-        self.assertEqual(expected, set(recs))
+        assert expected == set(recs)
 
-    def test_filter_debug(self):
+    def test_filter_debug(self, zmake_factory_from_dir, tmp_path):
         """Test what appears on the DEBUG level"""
-        recs, tmpname = do_test_with_log_level(logging.DEBUG)
+        recs = do_test_with_log_level(zmake_factory_from_dir, logging.DEBUG)
         # TODO: Remove sets and figure out how to check the lines are in the
         # right order.
         expected = {
             "Configuring fakeproject:build-rw.",
             "Configuring fakeproject:build-ro.",
-            "Building fakeproject in {}/ec/build/zephyr/fakeproject.".format(tmpname),
+            "Building fakeproject in {}/ec/build/zephyr/fakeproject.".format(tmp_path),
             "Building fakeproject:build-ro: /usr/bin/ninja -C {}-build-ro".format(
-                tmpname / "ec/build/zephyr/fakeproject/build"
+                tmp_path / "ec/build/zephyr/fakeproject/build"
             ),
             "Building fakeproject:build-rw: /usr/bin/ninja -C {}-build-rw".format(
-                tmpname / "ec/build/zephyr/fakeproject/build"
+                tmp_path / "ec/build/zephyr/fakeproject/build"
             ),
             "Running cat {}/files/sample_ro.txt".format(OUR_PATH),
             "Running cat {}/files/sample_rw.txt".format(OUR_PATH),
@@ -227,12 +204,14 @@ class TestFilters(unittest.TestCase):
                         "[fakeproject:build-{}]{}".format(suffix, line.strip())
                     )
         # This produces an easy-to-read diff if there is a difference
-        self.assertEqual(expected, set(recs))
+        assert expected == set(recs)
 
-    def test_filter_devicetree_error(self):
+    def test_filter_devicetree_error(self, zmake_factory_from_dir):
         """Test that devicetree errors appear"""
-        recs, tmpname = do_test_with_log_level(
-            logging.ERROR, {re.compile(r".*"): get_test_filepath("err")}
+        recs = do_test_with_log_level(
+            zmake_factory_from_dir,
+            logging.ERROR,
+            {re.compile(r".*"): get_test_filepath("err")},
         )
 
         dt_errs = [rec for rec in recs if "adc" in rec]
@@ -280,7 +259,9 @@ class TestFilters(unittest.TestCase):
         ),
     ],
 )
-def test_list_projects(project_names, format, search_dir, expected_output, capsys):
+def test_list_projects(
+    project_names, format, search_dir, expected_output, capsys, zmake_from_dir
+):
     """Test listing projects with default directory."""
     fake_projects = {
         name: zmake.project.Project(
@@ -293,17 +274,12 @@ def test_list_projects(project_names, format, search_dir, expected_output, capsy
         )
         for name in project_names
     }
-    zmk = zm.Zmake()
-    with mock.patch(
+    with unittest.mock.patch(
         "zmake.project.find_projects",
         autospec=True,
         return_value=fake_projects,
     ):
-        zmk.list_projects(format=format, search_dir=search_dir)
+        zmake_from_dir.list_projects(format=format, search_dir=search_dir)
 
     captured = capsys.readouterr()
     assert captured.out == expected_output
-
-
-if __name__ == "__main__":
-    unittest.main()