From 043576258b5f685fdfafc7a9d87e4793e622e640 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Mon, 18 Dec 2023 23:16:11 +0900 Subject: [PATCH 1/2] test(bzlmod): refactor tests to not depend on implementation details This refactors the `whl_mods` tests to not rely on the layout of the repositories, which I found to be needed whilst prototyping on #1625. Whilst doing this I realized that in general it would be great to support `Path` instances in the `runfiles` library, but that should be done next time. --- examples/bzlmod/whl_mods/BUILD.bazel | 8 ++- examples/bzlmod/whl_mods/pip_whl_mods_test.py | 70 +++++++++---------- 2 files changed, 39 insertions(+), 39 deletions(-) diff --git a/examples/bzlmod/whl_mods/BUILD.bazel b/examples/bzlmod/whl_mods/BUILD.bazel index 6ca07dd2d1..e8f10b656e 100644 --- a/examples/bzlmod/whl_mods/BUILD.bazel +++ b/examples/bzlmod/whl_mods/BUILD.bazel @@ -8,9 +8,13 @@ exports_files( py_test( name = "pip_whl_mods_test", srcs = ["pip_whl_mods_test.py"], + data = [ + "@pip//requests:dist_info", + "@pip//wheel:dist_info", + ], env = { - "REQUESTS_PKG_DIR": "pip_39_requests", - "WHEEL_PKG_DIR": "pip_39_wheel", + "REQUESTS_DISTINFO": "$(rlocationpaths @pip//requests:dist_info)", + "WHEEL_DISTINFO": "$(rlocationpaths @pip//wheel:dist_info)", }, main = "pip_whl_mods_test.py", deps = [ diff --git a/examples/bzlmod/whl_mods/pip_whl_mods_test.py b/examples/bzlmod/whl_mods/pip_whl_mods_test.py index c739b805bd..bccd84edba 100644 --- a/examples/bzlmod/whl_mods/pip_whl_mods_test.py +++ b/examples/bzlmod/whl_mods/pip_whl_mods_test.py @@ -27,22 +27,18 @@ class PipWhlModsTest(unittest.TestCase): maxDiff = None - def package_path(self) -> str: - return "rules_python~override~pip~" - - def wheel_pkg_dir(self) -> str: - env = os.environ.get("WHEEL_PKG_DIR") - self.assertIsNotNone(env) - return env + def wheel_pkg_dir(self) -> Path: + distinfo = os.environ.get("WHEEL_DISTINFO") + self.assertIsNotNone(distinfo) + return Path(distinfo.split(" ")[0]).parents[2].name def test_build_content_and_data(self): r = runfiles.Create() rpath = r.Rlocation( - "{}{}/generated_file.txt".format( - self.package_path(), - self.wheel_pkg_dir(), - ), - ) + "{}/generated_file.txt".format( + self.wheel_pkg_dir(), + ), + ) generated_file = Path(rpath) self.assertTrue(generated_file.exists()) @@ -52,11 +48,10 @@ def test_build_content_and_data(self): def test_copy_files(self): r = runfiles.Create() rpath = r.Rlocation( - "{}{}/copied_content/file.txt".format( - self.package_path(), - self.wheel_pkg_dir(), - ) - ) + "{}/copied_content/file.txt".format( + self.wheel_pkg_dir(), + ) + ) copied_file = Path(rpath) self.assertTrue(copied_file.exists()) @@ -64,14 +59,17 @@ def test_copy_files(self): self.assertEqual(content, "Hello world from copied file") def test_copy_executables(self): + executable_name = ( + "executable.exe" if platform.system() == "windows" else "executable.py" + ) + r = runfiles.Create() rpath = r.Rlocation( - "{}{}/copied_content/executable{}".format( - self.package_path(), - self.wheel_pkg_dir(), - ".exe" if platform.system() == "windows" else ".py", - ) - ) + "{}/copied_content/{}".format( + self.wheel_pkg_dir(), + executable_name, + ) + ) executable = Path(rpath) self.assertTrue(executable.exists()) @@ -88,11 +86,10 @@ def test_data_exclude_glob(self): current_wheel_version = "0.40.0" r = runfiles.Create() - dist_info_dir = "{}{}/site-packages/wheel-{}.dist-info".format( - self.package_path(), - self.wheel_pkg_dir(), - current_wheel_version, - ) + dist_info_dir = "{}/site-packages/wheel-{}.dist-info".format( + self.wheel_pkg_dir(), + current_wheel_version, + ) # Note: `METADATA` is important as it's consumed by https://docs.python.org/3/library/importlib.metadata.html # `METADATA` is expected to be there to show dist-info files are included in the runfiles. @@ -104,21 +101,20 @@ def test_data_exclude_glob(self): self.assertTrue(Path(metadata_path).exists()) self.assertFalse(Path(wheel_path).exists()) - def requests_pkg_dir(self) -> str: - env = os.environ.get("REQUESTS_PKG_DIR") - self.assertIsNotNone(env) - return env + def requests_pkg_dir(self) -> Path: + distinfo = os.environ.get("REQUESTS_DISTINFO") + self.assertIsNotNone(distinfo) + return Path(distinfo.split(" ")[0]).parents[2].name def test_extra(self): # This test verifies that annotations work correctly for pip packages with extras # specified, in this case requests[security]. r = runfiles.Create() rpath = r.Rlocation( - "{}{}/generated_file.txt".format( - self.package_path(), - self.requests_pkg_dir(), - ), - ) + "{}/generated_file.txt".format( + self.requests_pkg_dir(), + ), + ) generated_file = Path(rpath) self.assertTrue(generated_file.exists()) From 2866c96b00b80bc391e63ebfe9d32e3c84652fd6 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Wed, 20 Dec 2023 08:56:07 +0900 Subject: [PATCH 2/2] finally fix the tests --- examples/bzlmod/whl_mods/BUILD.bazel | 8 +--- examples/bzlmod/whl_mods/pip_whl_mods_test.py | 39 ++++++++++++------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/examples/bzlmod/whl_mods/BUILD.bazel b/examples/bzlmod/whl_mods/BUILD.bazel index e8f10b656e..241d9c1073 100644 --- a/examples/bzlmod/whl_mods/BUILD.bazel +++ b/examples/bzlmod/whl_mods/BUILD.bazel @@ -8,13 +8,9 @@ exports_files( py_test( name = "pip_whl_mods_test", srcs = ["pip_whl_mods_test.py"], - data = [ - "@pip//requests:dist_info", - "@pip//wheel:dist_info", - ], env = { - "REQUESTS_DISTINFO": "$(rlocationpaths @pip//requests:dist_info)", - "WHEEL_DISTINFO": "$(rlocationpaths @pip//wheel:dist_info)", + "REQUESTS_PKG": "$(rlocationpaths @pip//requests:pkg)", + "WHEEL_PKG": "$(rlocationpaths @pip//wheel:pkg)", }, main = "pip_whl_mods_test.py", deps = [ diff --git a/examples/bzlmod/whl_mods/pip_whl_mods_test.py b/examples/bzlmod/whl_mods/pip_whl_mods_test.py index bccd84edba..a88134b150 100644 --- a/examples/bzlmod/whl_mods/pip_whl_mods_test.py +++ b/examples/bzlmod/whl_mods/pip_whl_mods_test.py @@ -27,16 +27,28 @@ class PipWhlModsTest(unittest.TestCase): maxDiff = None + @staticmethod + def _get_bazel_pkg_dir_name(env_var: str) -> str: + a_file = Path(os.environ.get(env_var).split(" ")[0]) + head = a_file + while head.parent.name: + head = head.parent + + return head.name + + @classmethod + def setUpClass(cls): + cls._wheel_pkg_dir = cls._get_bazel_pkg_dir_name("WHEEL_PKG") + cls._requests_pkg_dir = cls._get_bazel_pkg_dir_name("REQUESTS_PKG") + def wheel_pkg_dir(self) -> Path: - distinfo = os.environ.get("WHEEL_DISTINFO") - self.assertIsNotNone(distinfo) - return Path(distinfo.split(" ")[0]).parents[2].name + return self._wheel_pkg def test_build_content_and_data(self): r = runfiles.Create() rpath = r.Rlocation( "{}/generated_file.txt".format( - self.wheel_pkg_dir(), + self._wheel_pkg_dir, ), ) generated_file = Path(rpath) @@ -49,7 +61,7 @@ def test_copy_files(self): r = runfiles.Create() rpath = r.Rlocation( "{}/copied_content/file.txt".format( - self.wheel_pkg_dir(), + self._wheel_pkg_dir, ) ) copied_file = Path(rpath) @@ -66,7 +78,7 @@ def test_copy_executables(self): r = runfiles.Create() rpath = r.Rlocation( "{}/copied_content/{}".format( - self.wheel_pkg_dir(), + self._wheel_pkg_dir, executable_name, ) ) @@ -87,7 +99,7 @@ def test_data_exclude_glob(self): r = runfiles.Create() dist_info_dir = "{}/site-packages/wheel-{}.dist-info".format( - self.wheel_pkg_dir(), + self._wheel_pkg_dir, current_wheel_version, ) @@ -98,13 +110,10 @@ def test_data_exclude_glob(self): # However, `WHEEL` was explicitly excluded, so it should be missing wheel_path = r.Rlocation("{}/WHEEL".format(dist_info_dir)) - self.assertTrue(Path(metadata_path).exists()) - self.assertFalse(Path(wheel_path).exists()) - - def requests_pkg_dir(self) -> Path: - distinfo = os.environ.get("REQUESTS_DISTINFO") - self.assertIsNotNone(distinfo) - return Path(distinfo.split(" ")[0]).parents[2].name + self.assertTrue(Path(metadata_path).exists(), f"Could not find {metadata_path}") + self.assertFalse( + Path(wheel_path).exists(), f"Expected to not find {wheel_path}" + ) def test_extra(self): # This test verifies that annotations work correctly for pip packages with extras @@ -112,7 +121,7 @@ def test_extra(self): r = runfiles.Create() rpath = r.Rlocation( "{}/generated_file.txt".format( - self.requests_pkg_dir(), + self._requests_pkg_dir, ), ) generated_file = Path(rpath)