From 288f38dca3e97de86ad9e894d3de37f736d2756f Mon Sep 17 00:00:00 2001
From: Simon Rose <simon.rose@ess.eu>
Date: Tue, 6 Sep 2022 13:48:48 +0200
Subject: [PATCH] E3-731: Improve wrapper fixture
Previously, the wrapper fixture would set some environment variables
when running `run_make`; this meant that you could not debug a test
case by simply navigating to the test directory and trying to build
the generated module. This change fixes that.
---
tests/conftest.py | 66 +++++++++++++++++++++++--------------------
tests/test_build.py | 34 +++++++++++-----------
tests/test_e3.py | 26 ++++++-----------
tests/test_fixture.py | 8 ++++++
4 files changed, 69 insertions(+), 65 deletions(-)
create mode 100644 tests/test_fixture.py
diff --git a/tests/conftest.py b/tests/conftest.py
index 7fdf70eb..12ada8cb 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -10,6 +10,20 @@ from git import Repo
class Wrapper:
def __init__(self, root_path: Path, name=None, include_dbd=True, **kwargs):
+ test_env = os.environ.copy()
+
+ assert "EPICS_BASE" in test_env
+ assert "E3_REQUIRE_VERSION" in test_env
+
+ e3_require_config = (
+ Path(os.environ.get("EPICS_BASE"))
+ / "require"
+ / os.environ.get("E3_REQUIRE_VERSION")
+ / "configure"
+ )
+
+ assert e3_require_config.is_dir()
+
if name is None:
name = "test_mod_" + "".join(choice(ascii_lowercase) for _ in range(16))
self.path = root_path / f"e3-{name}"
@@ -24,11 +38,10 @@ class Wrapper:
self.config_dir = self.path / "configure"
self.config_dir.mkdir()
- self.config_module = self.path / "CONFIG_MODULE"
+ self.config_module = self.config_dir / "CONFIG_MODULE"
self.config_module.touch()
- self.makefile = self.path / f"{name}.Makefile"
-
+ self.makefile = self.path / "Makefile"
makefile_contents = f"""
TOP:=$(CURDIR)
@@ -37,20 +50,24 @@ E3_MODULE_VERSION?=0.0.0+0
E3_MODULE_SRC_PATH:={module_path}
E3_MODULE_MAKEFILE:={name}.Makefile
-include $(TOP)/CONFIG_MODULE
+include $(TOP)/configure/CONFIG_MODULE
+-include $(TOP)/configure/CONFIG_MODULE.local
+
+REQUIRE_CONFIG:={e3_require_config}
include $(REQUIRE_CONFIG)/CONFIG
include $(REQUIRE_CONFIG)/RULES_SITEMODS
"""
- with open(self.path / "Makefile", "w") as f:
- f.write(makefile_contents)
+ (self.makefile).write_text(makefile_contents)
+ self.module_makefile = self.path / f"{name}.Makefile"
module_makefile_contents = """
where_am_I := $(dir $(abspath $(lastword $(MAKEFILE_LIST))))
include $(E3_REQUIRE_TOOLS)/driver.makefile
+
+EXCLUDE_ARCHS+=debug
"""
- with open(self.path / f"{name}.Makefile", "w") as f:
- f.write(module_makefile_contents)
+ (self.module_makefile).write_text(module_makefile_contents)
if include_dbd:
self.add_file("test.dbd")
@@ -71,8 +88,8 @@ include $(E3_REQUIRE_TOOLS)/driver.makefile
with open(self.config_module, "a") as f:
f.write(f"export {makefile_var} {modifier}= {value}\n")
- def add_var_to_makefile(self, makefile_var: str, value: str, modifier="+"):
- with open(self.makefile, "a") as f:
+ def add_var_to_module_makefile(self, makefile_var: str, value: str, modifier="+"):
+ with open(self.module_makefile, "a") as f:
f.write(f"{makefile_var} {modifier}= {value}\n")
def get_env_var(self, var: str):
@@ -88,34 +105,21 @@ include $(E3_REQUIRE_TOOLS)/driver.makefile
self,
target: str,
*args,
- module_name: str = "",
module_version: str = "",
cell_path: str = "",
):
"""Attempt to run `make <target> <args>` for the current wrapper."""
- test_env = os.environ.copy()
-
- assert "EPICS_BASE" in test_env
- assert Path(test_env["EPICS_BASE"]).is_dir
- assert "E3_REQUIRE_VERSION" in test_env
- assert test_env["E3_REQUIRE_VERSION"]
-
- e3_require_location = (
- Path(os.environ.get("EPICS_BASE"))
- / "require"
- / os.environ.get("E3_REQUIRE_VERSION")
- )
- assert e3_require_location.is_dir()
- test_env["E3_REQUIRE_LOCATION"] = e3_require_location
+ # We should not install in the global environment during tests
+ if target == "install":
+ target = "cellinstall"
- require_config = f"{e3_require_location}/configure"
- test_env["REQUIRE_CONFIG"] = require_config
+ if target == "uninstall":
+ target = "celluninstall"
- if module_name:
- test_env["E3_MODULE_NAME"] = module_name
+ env = os.environ.copy()
if module_version:
- test_env["E3_MODULE_VERSION"] = module_version
+ env["E3_MODULE_VERSION"] = module_version
if cell_path:
self.write_dot_local_data("CONFIG_CELL", {"E3_CELL_PATH": cell_path})
make_cmd = ["make", "-C", self.path, target] + list(args)
@@ -123,7 +127,7 @@ include $(E3_REQUIRE_TOOLS)/driver.makefile
make_cmd,
stdout=subprocess.PIPE,
stderr=subprocess.PIPE,
- env=test_env,
+ env=env,
encoding="utf-8",
)
outs, errs = p.communicate()
diff --git a/tests/test_build.py b/tests/test_build.py
index fa12115a..8f0c46b2 100644
--- a/tests/test_build.py
+++ b/tests/test_build.py
@@ -69,7 +69,7 @@ def test_local_module(wrapper):
def test_missing_dbd_file(wrapper):
- wrapper.add_var_to_makefile("DBDS", "nonexistent.dbd")
+ wrapper.add_var_to_module_makefile("DBDS", "nonexistent.dbd")
rc, _, errs = wrapper.run_make("build")
assert rc == 2
@@ -80,7 +80,7 @@ def test_missing_dbd_file(wrapper):
def test_missing_source_file(wrapper):
- wrapper.add_var_to_makefile("SOURCES", "nonexistent.c")
+ wrapper.add_var_to_module_makefile("SOURCES", "nonexistent.c")
rc, _, errs = wrapper.run_make("build")
assert rc == 2
@@ -125,8 +125,8 @@ def test_header_install_location(wrapper):
extensions = ["h", "hpp", "hxx", "hh"]
for ext in extensions:
- wrapper.add_var_to_makefile("HEADERS", f"db/subdir/header.{ext}")
- wrapper.add_var_to_makefile("KEEP_HEADER_SUBDIRS", "db")
+ wrapper.add_var_to_module_makefile("HEADERS", f"db/subdir/header.{ext}")
+ wrapper.add_var_to_module_makefile("KEEP_HEADER_SUBDIRS", "db")
for ext in extensions:
(subdir / f"header.{ext}").touch()
@@ -444,10 +444,10 @@ def test_recursive_header_include(wrappers):
wrapper_b.add_var_to_config_module(f"{wrapper_c.name}_DEP_VERSION", module_version)
wrapper_a.add_var_to_config_module(f"{wrapper_b.name}_DEP_VERSION", module_version)
- wrapper_c.add_var_to_makefile("HEADERS", f"{wrapper_c.name}.h")
+ wrapper_c.add_var_to_module_makefile("HEADERS", f"{wrapper_c.name}.h")
(wrapper_c.module_dir / f"{wrapper_c.name}.h").touch()
- wrapper_a.add_var_to_makefile("SOURCES", f"{wrapper_a.name}.c")
+ wrapper_a.add_var_to_module_makefile("SOURCES", f"{wrapper_a.name}.c")
with open(wrapper_a.module_dir / f"{wrapper_a.name}.c", "w") as f:
f.write(f'#include "{wrapper_c.name}.h"')
@@ -472,7 +472,7 @@ def test_recursive_header_include(wrappers):
def test_updated_template_files(wrapper):
- wrapper.add_var_to_makefile("SUBS", "x.substitutions")
+ wrapper.add_var_to_module_makefile("SUBS", "x.substitutions")
substitution_file = wrapper.module_dir / "x.substitutions"
substitution_file.write_text("file x.template {pattern {x} {y}}")
@@ -497,9 +497,9 @@ def test_updated_template_files(wrapper):
def test_expand_db_files(wrapper):
"""Test that the automated template/substitution file expansion works."""
- wrapper.add_var_to_makefile("TMPS", "templates/a.template")
- wrapper.add_var_to_makefile("SUBS", "b.substitutions")
- wrapper.add_var_to_makefile("USR_DBFLAGS", "-I templates")
+ wrapper.add_var_to_module_makefile("TMPS", "templates/a.template")
+ wrapper.add_var_to_module_makefile("SUBS", "b.substitutions")
+ wrapper.add_var_to_module_makefile("USR_DBFLAGS", "-I templates")
template_dir = wrapper.module_dir / "templates"
template_dir.mkdir()
@@ -543,7 +543,7 @@ def test_expand_db_files(wrapper):
def test_arch_filter(wrapper, installed_archs, param, expected):
arch_regex = re.compile(r"Pass 2: T_A =\s*([^\s]+)")
- wrapper.add_var_to_makefile(
+ wrapper.add_var_to_module_makefile(
"CROSS_COMPILER_TARGET_ARCHS", installed_archs, modifier=""
)
@@ -562,8 +562,8 @@ def test_build_fails_if_nth_architecture_fails(wrapper, archs, failing_arch):
# LIBOBJS is determined in part based on configuration data coming from
# $(CONFIG)/os/CONFIG.Common.$(T_A); since our architectures do not actually
# exist, we need to manually define these /before/ driver.makefile is included.
- makefile_content = wrapper.makefile.read_text()
- with open(wrapper.makefile, "w") as f:
+ makefile_content = wrapper.module_makefile.read_text()
+ with open(wrapper.module_makefile, "w") as f:
f.write(
f"""ifeq ($(T_A),{failing_arch})
LIBOBJS = nonexistent_{failing_arch}.o
@@ -576,10 +576,12 @@ def test_build_fails_if_nth_architecture_fails(wrapper, archs, failing_arch):
# Skip the host architecture, we are not testing it.
host_arch = os.getenv("EPICS_HOST_ARCH")
- wrapper.add_var_to_makefile("EXCLUDE_ARCHS", host_arch)
+ wrapper.add_var_to_module_makefile("EXCLUDE_ARCHS", host_arch)
- wrapper.add_var_to_makefile("CROSS_COMPILER_TARGET_ARCHS", archs, modifier="")
- wrapper.add_var_to_makefile("SOURCES", "-none-")
+ wrapper.add_var_to_module_makefile(
+ "CROSS_COMPILER_TARGET_ARCHS", archs, modifier=""
+ )
+ wrapper.add_var_to_module_makefile("SOURCES", "-none-")
rc, _, errs = wrapper.run_make("build")
assert rc == 2
diff --git a/tests/test_e3.py b/tests/test_e3.py
index 4918681d..5b2c0d4f 100644
--- a/tests/test_e3.py
+++ b/tests/test_e3.py
@@ -6,27 +6,17 @@ def test_loc(wrappers):
def test_sitelibs(wrappers):
- # Overwrite the default makefile
wrapper = wrappers.get()
- with open(wrapper.path / "Makefile", "w") as f:
- f.write(
- f"""
-TOP:=$(CURDIR)
-E3_MODULE_NAME:={wrapper.name}
+ makefile_contents = wrapper.makefile.read_text()
-include $(REQUIRE_CONFIG)/RULES_E3
-include $(REQUIRE_CONFIG)/RULES_CELL
-include $(REQUIRE_CONFIG)/DEFINES_FT
-include $(REQUIRE_CONFIG)/RULES_PATCH
-include $(REQUIRE_CONFIG)/RULES_TEST
-
-include $(REQUIRE_CONFIG)/RULES_DKMS
-include $(REQUIRE_CONFIG)/RULES_VARS
-
-include $(REQUIRE_CONFIG)/RULES_DEV
-"""
+ wrapper.makefile.write_text(
+ makefile_contents.replace(
+ "include $(REQUIRE_CONFIG)/RULES_SITEMODS",
+ "include $(REQUIRE_CONFIG)/RULES_E3",
)
+ )
+
rc, _, errs = wrapper.run_make("build")
assert rc == 2
assert "RULES_E3 should only be loaded from RULES_SITEMODS" in errs
@@ -36,6 +26,6 @@ def test_incorrect_module_name(wrappers):
module_name = "ADCore"
wrapper = wrappers.get(name=module_name)
- rc, _, errs = wrapper.run_make("build", module_name=module_name)
+ rc, _, errs = wrapper.run_make("build")
assert rc == 2
assert f"E3_MODULE_NAME '{module_name}' is not valid" in errs
diff --git a/tests/test_fixture.py b/tests/test_fixture.py
new file mode 100644
index 00000000..2e61921b
--- /dev/null
+++ b/tests/test_fixture.py
@@ -0,0 +1,8 @@
+import subprocess
+
+from .conftest import Wrapper
+
+
+def test_can_run_make_in_wrapper_directory(wrapper: Wrapper):
+ results = subprocess.run(["make", "-C", wrapper.path, "vars"])
+ assert results.returncode == 0
--
GitLab