From 59f0c6bb7180020a86c2cf069895b7a579ee35e8 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 19 Feb 2025 09:38:52 +0000 Subject: [PATCH 01/30] Added CLI to automate building of Docker images --- pyproject.toml | 1 + src/murfey/cli/build_images.py | 304 +++++++++++++++++++++++++++++++++ 2 files changed, 305 insertions(+) create mode 100644 src/murfey/cli/build_images.py diff --git a/pyproject.toml b/pyproject.toml index 0daf313f8..bc7aac7b8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -83,6 +83,7 @@ GitHub = "https://github.com/DiamondLightSource/python-murfey" [project.scripts] murfey = "murfey.client:run" "murfey.add_user" = "murfey.cli.add_user:run" +"murfey.build_images" = "murfey.cli.build_images:run" "murfey.create_db" = "murfey.cli.create_db:run" "murfey.db_sql" = "murfey.cli.murfey_db_sql:run" "murfey.decrypt_password" = "murfey.cli.decrypt_db_password:run" diff --git a/src/murfey/cli/build_images.py b/src/murfey/cli/build_images.py new file mode 100644 index 000000000..bf215a9c9 --- /dev/null +++ b/src/murfey/cli/build_images.py @@ -0,0 +1,304 @@ +""" +Helper function to automate the process of building and publishing Docker images for +Murfey using Python subprocesses. + +This CLI is designed to run with Podman commands and in a bash shell that has been +configured to push to a valid Docker repo, which has to be specified using a flag. +""" + +import grp +import os +import subprocess +from argparse import ArgumentParser +from pathlib import Path + + +def run_subprocess(cmd: list[str], src: str = "."): + process = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + bufsize=1, + universal_newlines=True, + env=os.environ, + cwd=Path(src), + ) + + # Parse stdout and stderr + if process.stdout: + for line in process.stdout: + print(line, end="") + if process.stderr: + for line in process.stderr: + print(line, end="") + + # Wait for process to complete + process.wait() + + return process.returncode + + +# Function to build Docker image +def build_image( + image: str, + tag: str, + source: str, + destination: str, + user_id: int, + group_id: int, + group_name: str, + dry_run: bool = False, +): + # Construct path to Dockerfile + dockerfile = Path(source) / "Dockerfiles" / image + if not dockerfile.exists(): + raise FileNotFoundError( + f"Unable to find Dockerfile for {image} at {str(dockerfile)!r}" + ) + + # Construct tag + image_path = f"{destination}/{image}" + if tag: + image_path = f"{image_path}:{tag}" + + # Construct bash command to build image + build_cmd = [ + "podman build", + f"--build-arg=userid={user_id}", + f"--build-arg=groupid={group_id}", + f"--build-arg=groupname={group_name}", + "--no-cache", + f"-f {str(dockerfile)}", + f"-t {image_path}", + f"{source}", + ] + bash_cmd = ["bash", "-c", " ".join(build_cmd)] + + if not dry_run: + print() + # Run subprocess command to build image + result = run_subprocess(bash_cmd, source) + + # Check for errors + if result != 0: + raise RuntimeError(f"Build command failed with exit code {result}") + + if dry_run: + print() + print(f"Will build image {image!r}") + print(f"Will use Dockerfile from {str(dockerfile)!r}") + print( + f"Will build image with UID {user_id}, GID {group_id}, and group name {group_name}" + ) + print(f"Will build image with tag {image_path}") + print("Will run the following bash command:") + print(bash_cmd) + + return image_path + + +def tag_image( + image_path: str, + tags: list[str], + dry_run: bool = False, +): + # Construct list of tags to create + base_path = image_path.split(":")[0] + new_tags = [f"{base_path}:{tag}" for tag in tags] + + # Construct bash command to add all additional tags + tag_cmd = [ + f"for IMAGE in {' '.join(new_tags)};", + f"do podman tag {image_path} $IMAGE;", + "done", + ] + bash_cmd = ["bash", "-c", " ".join(tag_cmd)] + if not dry_run: + print() + # Run subprocess command to tag image + result = run_subprocess(bash_cmd) + + # Check for errors + if result != 0: + raise RuntimeError(f"Tag command failed with exit code {result}") + + if dry_run: + print() + print("Will run the following bash command:") + print(bash_cmd) + for tag in new_tags: + print(f"Will create new tag {tag}") + + return new_tags + + +def push_images( + images: list[str], + dry_run: bool = False, +): + # Construct bash command to push images + push_cmd = [f"for IMAGE in {' '.join(images)};", "do podman push $IMAGE;", "done"] + bash_cmd = ["bash", "-c", " ".join(push_cmd)] + if not dry_run: + print() + # Run subprocess command to push image + result = run_subprocess(bash_cmd) + + # Check for errors + if result != 0: + raise RuntimeError(f"Push command failed with exit code {result}") + + if dry_run: + print() + print("Will run the following bash command:") + print(bash_cmd) + for image in images: + print(f"Will push image {image}") + + return True + + +def cleanup(dry_run: bool = False): + # Construct bash command to push images + cleanup_cmd = [ + "podman image prune -f", + ] + bash_cmd = ["bash", "-c", " ".join(cleanup_cmd)] + if not dry_run: + print() + # Run subprocess command to clean up Podman repo + result = run_subprocess(bash_cmd) + + # Check for errors + if result != 0: + raise RuntimeError(f"Cleanup command failed with exit code {result}") + + if dry_run: + print() + print("Will run the following bash command:") + print(bash_cmd) + + return True + + +def run(): + + parser = ArgumentParser( + description=( + "Uses Podman to build, tag, and push the specified images either locally " + "or to a remote repository" + ) + ) + + parser.add_argument( + "images", + nargs="+", + help=("Space-separated list of Murfey Dockerfiles that you want to build."), + ) + + parser.add_argument( + "--tags", + "-t", + nargs="*", + default=["latest"], + help=("Space-separated list of tags to apply to the built images"), + ) + + parser.add_argument( + "--source", + "-s", + default=".", + help=("Directory path to the Murfey repository"), + ) + + parser.add_argument( + "--destination", + "-d", + default="localhost", + help=("The URL of the repo to push the built images to"), + ) + + parser.add_argument( + "--user-id", + default=os.getuid(), + help=("The user ID to install in the images"), + ) + + parser.add_argument( + "--group-id", + default=os.getgid(), + help=("The group ID to install in the images"), + ) + + parser.add_argument( + "--group-name", + default=( + grp.getgrgid(os.getgid()).gr_name if hasattr(grp, "getgrgid") else "nogroup" + ), + help=("The group name to install in the images"), + ) + + parser.add_argument( + "--dry-run", + default=False, + action="store_true", + help=( + "When specified, prints out what the command would have done for each " + "stage of the process" + ), + ) + + args = parser.parse_args() + + # Validate the paths to the images + for image in args.images: + if not (Path(args.source) / "Dockerfiles" / str(image)).exists(): + raise FileNotFoundError( + "No Dockerfile found in " + f"source repository {str(Path(args.source).resolve())!r} for" + f"image {str(image)!r}" + ) + + # Build image + images = [] + for image in args.images: + image_path = build_image( + image=image, + tag=args.tags[0], + source=args.source, + destination=( + str(args.destination).rstrip("/") + if str(args.destination).endswith("/") + else str(args.destination) + ), + user_id=args.user_id, + group_id=args.group_id, + group_name=args.group_name, + dry_run=args.dry_run, + ) + images.append(image_path) + + # Create additional tags (if any) for each built image + if len(args.tags) > 1: + new_tags = tag_image( + image_path=image_path, + tags=args.tags[1:], + dry_run=args.dry_run, + ) + images.extend(new_tags) + + # Push all built images to specified repo + push_images(images, dry_run=args.dry_run) + + # Perform final cleanup + cleanup(dry_run=args.dry_run) + + # Notify that job is completed + print() + print("Done") + + +# Allow it to be run directly from the file +if __name__ == "__main__": + run() From 37a87ce414156f8396ae63284e32b95d86204d24 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Wed, 19 Feb 2025 09:44:55 +0000 Subject: [PATCH 02/30] Fixed comments --- src/murfey/cli/build_images.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/murfey/cli/build_images.py b/src/murfey/cli/build_images.py index bf215a9c9..1c09e3384 100644 --- a/src/murfey/cli/build_images.py +++ b/src/murfey/cli/build_images.py @@ -39,7 +39,6 @@ def run_subprocess(cmd: list[str], src: str = "."): return process.returncode -# Function to build Docker image def build_image( image: str, tag: str, @@ -116,7 +115,7 @@ def tag_image( bash_cmd = ["bash", "-c", " ".join(tag_cmd)] if not dry_run: print() - # Run subprocess command to tag image + # Run subprocess command to tag images result = run_subprocess(bash_cmd) # Check for errors @@ -137,12 +136,12 @@ def push_images( images: list[str], dry_run: bool = False, ): - # Construct bash command to push images + # Construct bash command to push images to the repo push_cmd = [f"for IMAGE in {' '.join(images)};", "do podman push $IMAGE;", "done"] bash_cmd = ["bash", "-c", " ".join(push_cmd)] if not dry_run: print() - # Run subprocess command to push image + # Run subprocess command result = run_subprocess(bash_cmd) # Check for errors @@ -160,14 +159,14 @@ def push_images( def cleanup(dry_run: bool = False): - # Construct bash command to push images + # Construct bash command to clean up Podman repo cleanup_cmd = [ "podman image prune -f", ] bash_cmd = ["bash", "-c", " ".join(cleanup_cmd)] if not dry_run: print() - # Run subprocess command to clean up Podman repo + # Run subprocess command result = run_subprocess(bash_cmd) # Check for errors From 24a683a0304a019a343adc059db2c0207d0ff14a Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 25 Feb 2025 07:58:55 +0000 Subject: [PATCH 03/30] Added start of unit test for 'build_images' CLI --- tests/cli/test_build_images.py | 96 ++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 tests/cli/test_build_images.py diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py new file mode 100644 index 000000000..9827b049d --- /dev/null +++ b/tests/cli/test_build_images.py @@ -0,0 +1,96 @@ +import grp +import os +import sys +from unittest.mock import call, patch + +import pytest + +from murfey.cli.build_images import run + +test_run_params_matrix: tuple[ + tuple[list[str], list[str], str, str, str, str, str, bool] +] = ( + # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Default settings + ([f"test_image_{n}" for n in range(3)], [], "", "", "", "", "", False), +) + + +@pytest.mark.parametrize("build_params", test_run_params_matrix) +@patch("murfey.cli.build_images.cleanup") +@patch("murfey.cli.build_images.push_images") +@patch("murfey.cli.build_images.tag_image") +@patch("murfey.cli.build_images.build_image") +def test_run( + mock_build, + mock_tag, + mock_push, + mock_clean, + build_params: tuple[list[str], list[str], str, str, str, str, str, bool], +): + """ + Tests that the function is run with the expected arguments for a given + combination of flags. + """ + + # Unpack build params + images, tags, src, dst, uid, gid, gname, dry_run = build_params + + # Set defaults of the various flags + def_tags = ["latest"] + def_src = "." + def_dst = "localhost" + def_uid = os.getuid() + def_gid = os.getgid() + def_gname = ( + grp.getgrgid(os.getgid()).gr_name if hasattr(grp, "getgrgid") else "nogroup" + ) + def_dry_run = False + + # Set up the command based on what these values are + build_cmd = [ + "murfey.build_images", + " ".join(images), + ] + + # Iterate through flags and add them according to the command + flags = ( + # 'images' already include by default + ("--tags", tags), + ("--source", src), + ("--destination", dst), + ("--user-id", uid), + ("--group-id", gid), + ("--group-name", gname), + ("--dry-run", dry_run), + ) + for flag, value in flags: + if isinstance(value, list) and value: + build_cmd.extend([flag, *value]) + if isinstance(value, str) and value: + build_cmd.extend([flag, value]) + if isinstance(value, bool) and value: + build_cmd.append(flag) + + # Assign it to the CLI to pass to the function + sys.argv = build_cmd + + # Run the function with the command + run() + + # Check that the functions were called with the correct flags + assert mock_build.call_count == len(images) + expected_calls = ( + call( + image=image, + tag=tags[0] if tags else def_tags[0], + source=src if src else def_src, + destination=dst if dst else def_dst, + user_id=uid if uid else def_uid, + group_id=gid if gid else def_gid, + groupd_name=gname if gname else def_gname, + dry_run=dry_run if dry_run else def_dry_run, + ) + for image in images + ) + mock_build.assert_has_calls(expected_calls, any_order=True) From 45afe7c0f3427f10839f3ffc43d6b5d78e88ebf1 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 25 Feb 2025 08:23:40 +0000 Subject: [PATCH 04/30] Mocked out the results of 'build_images' --- tests/cli/test_build_images.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 9827b049d..54dc6f688 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -12,11 +12,12 @@ ] = ( # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Default settings - ([f"test_image_{n}" for n in range(3)], [], "", "", "", "", "", False), + ([f"test_image_{n}" for n in range(1)], [], "", "", "", "", "", False), ) @pytest.mark.parametrize("build_params", test_run_params_matrix) +@patch("murfey.cli.build_images.run_subprocess") @patch("murfey.cli.build_images.cleanup") @patch("murfey.cli.build_images.push_images") @patch("murfey.cli.build_images.tag_image") @@ -26,6 +27,7 @@ def test_run( mock_tag, mock_push, mock_clean, + mock_subprocess, build_params: tuple[list[str], list[str], str, str, str, str, str, bool], ): """ @@ -75,6 +77,12 @@ def test_run( # Assign it to the CLI to pass to the function sys.argv = build_cmd + # Mock 'build_image' return value + mock_build.side_effect = [ + f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" + for image in images + ] + # Run the function with the command run() From 18a4e7fdf8c191bcaa5bb93cd858c517b443500f Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 25 Feb 2025 08:34:55 +0000 Subject: [PATCH 05/30] Create Dockerfiles at the location the 'build_image' command expects --- tests/cli/test_build_images.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 54dc6f688..b4f2981fb 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -1,6 +1,7 @@ import grp import os import sys +from pathlib import Path from unittest.mock import call, patch import pytest @@ -12,7 +13,7 @@ ] = ( # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Default settings - ([f"test_image_{n}" for n in range(1)], [], "", "", "", "", "", False), + ([f"test_image_{n}" for n in range(3)], [], "", "", "", "", "", False), ) @@ -83,6 +84,11 @@ def test_run( for image in images ] + # Create Dockerfiles at the location it expects + for image in images: + dockerfile = Path(src if src else def_src) / "Dockerfiles" / image + dockerfile.touch(exist_ok=True) + # Run the function with the command run() From d2a0905fe2413bdb7c626ecb4cf2ae56232e3322 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Tue, 25 Feb 2025 08:40:10 +0000 Subject: [PATCH 06/30] Use default source directory for Murfey from pytest reports --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index b4f2981fb..719e7fd9c 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -41,7 +41,7 @@ def test_run( # Set defaults of the various flags def_tags = ["latest"] - def_src = "." + def_src = "/home/runner/work/python-murfey/python-murfey" def_dst = "localhost" def_uid = os.getuid() def_gid = os.getgid() From d3b33c20094943bd00a6c3d93fbac6a44cb0a5f6 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:09:55 +0000 Subject: [PATCH 07/30] Moved default flag values out as module-wide variables; tried mocking return value of 'Path.exists()' --- tests/cli/test_build_images.py | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 719e7fd9c..edb8f4a9c 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -1,23 +1,35 @@ import grp import os import sys -from pathlib import Path from unittest.mock import call, patch import pytest from murfey.cli.build_images import run +images = [f"test_image_{n}" for n in range(3)] + +# Set defaults of the various flags +def_tags = ["latest"] +def_src = "/home/runner/work/python-murfey/python-murfey" +def_dst = "localhost" +def_uid = os.getuid() +def_gid = os.getgid() +def_gname = grp.getgrgid(os.getgid()).gr_name if hasattr(grp, "getgrgid") else "nogroup" +def_dry_run = False + + test_run_params_matrix: tuple[ tuple[list[str], list[str], str, str, str, str, str, bool] ] = ( # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Default settings - ([f"test_image_{n}" for n in range(3)], [], "", "", "", "", "", False), + (images, [], "", "", "", "", "", False), ) @pytest.mark.parametrize("build_params", test_run_params_matrix) +@patch("murfey.cli.build_images.Path.exists") @patch("murfey.cli.build_images.run_subprocess") @patch("murfey.cli.build_images.cleanup") @patch("murfey.cli.build_images.push_images") @@ -29,6 +41,7 @@ def test_run( mock_push, mock_clean, mock_subprocess, + mock_exists, build_params: tuple[list[str], list[str], str, str, str, str, str, bool], ): """ @@ -39,17 +52,6 @@ def test_run( # Unpack build params images, tags, src, dst, uid, gid, gname, dry_run = build_params - # Set defaults of the various flags - def_tags = ["latest"] - def_src = "/home/runner/work/python-murfey/python-murfey" - def_dst = "localhost" - def_uid = os.getuid() - def_gid = os.getgid() - def_gname = ( - grp.getgrgid(os.getgid()).gr_name if hasattr(grp, "getgrgid") else "nogroup" - ) - def_dry_run = False - # Set up the command based on what these values are build_cmd = [ "murfey.build_images", @@ -84,10 +86,8 @@ def test_run( for image in images ] - # Create Dockerfiles at the location it expects - for image in images: - dockerfile = Path(src if src else def_src) / "Dockerfiles" / image - dockerfile.touch(exist_ok=True) + # Mock the check for the existence of the Dockerfiles + mock_exists.return_value = True # Run the function with the command run() From c2ac907377f1c56f15dc62df18463c466f765d96 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:16:57 +0000 Subject: [PATCH 08/30] Mocked out return value of the subprocesses --- tests/cli/test_build_images.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index edb8f4a9c..36c89ece8 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -80,15 +80,18 @@ def test_run( # Assign it to the CLI to pass to the function sys.argv = build_cmd - # Mock 'build_image' return value + # Mock the check for the existence of the Dockerfiles + mock_exists.return_value = True + + # Mock the exit code of the subprocesses being run + mock_subprocess.return_value = 0 + + # Mock 'build_image' return values mock_build.side_effect = [ f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" for image in images ] - # Mock the check for the existence of the Dockerfiles - mock_exists.return_value = True - # Run the function with the command run() From db637c89125457882c002211c715325d1be40c3d Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:27:19 +0000 Subject: [PATCH 09/30] Mocked return values for the tag, push, and cleanup functions in the workflow --- tests/cli/test_build_images.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 36c89ece8..b15bb478c 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -87,10 +87,25 @@ def test_run( mock_subprocess.return_value = 0 # Mock 'build_image' return values - mock_build.side_effect = [ + image_paths = [ f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" for image in images ] + mock_build.side_effect = image_paths + + # Mock all the return values when tagging the images + all_tags = [ + f"{image.split(':')[0]}:{tag}" + for image in image_paths + for tag in (tags if tags else def_tags) + ] + mock_tag.side_effect = all_tags + + # Mock the push function + mock_push.return_value = True + + # Mock the cleanup function + mock_clean.return_value = True # Run the function with the command run() From f42b0f0ca77a3636c4d26123f259851c471682d1 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:38:47 +0000 Subject: [PATCH 10/30] Pass images as individual list items in the subprocess command --- tests/cli/test_build_images.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index b15bb478c..08ff0fbdb 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -55,7 +55,7 @@ def test_run( # Set up the command based on what these values are build_cmd = [ "murfey.build_images", - " ".join(images), + *images, ] # Iterate through flags and add them according to the command @@ -87,16 +87,16 @@ def test_run( mock_subprocess.return_value = 0 # Mock 'build_image' return values - image_paths = [ + built_images = [ f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" for image in images ] - mock_build.side_effect = image_paths + mock_build.side_effect = built_images # Mock all the return values when tagging the images all_tags = [ f"{image.split(':')[0]}:{tag}" - for image in image_paths + for image in built_images for tag in (tags if tags else def_tags) ] mock_tag.side_effect = all_tags From e182945bdc82aa69a899605d527b6afaf6939b00 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:47:47 +0000 Subject: [PATCH 11/30] Changed default source directory value --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 08ff0fbdb..c9424edfb 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -11,7 +11,7 @@ # Set defaults of the various flags def_tags = ["latest"] -def_src = "/home/runner/work/python-murfey/python-murfey" +def_src = "." def_dst = "localhost" def_uid = os.getuid() def_gid = os.getgid() From d77a3579a3e9ec0084f5e578132264a37611002d Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 03:59:13 +0000 Subject: [PATCH 12/30] Typo in variables to check for --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index c9424edfb..9a3179b0f 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -120,7 +120,7 @@ def test_run( destination=dst if dst else def_dst, user_id=uid if uid else def_uid, group_id=gid if gid else def_gid, - groupd_name=gname if gname else def_gname, + group_name=gname if gname else def_gname, dry_run=dry_run if dry_run else def_dry_run, ) for image in images From 42dfc8720a9b978b180cbb3581bc7c81d682d990 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 04:44:08 +0000 Subject: [PATCH 13/30] Refactored tests to build all generated Docker image URLs and group them in lists accordingly; tested the calls made by the 'tag_image' function as well --- tests/cli/test_build_images.py | 51 +++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 19 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 9a3179b0f..9998696d4 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -86,33 +86,35 @@ def test_run( # Mock the exit code of the subprocesses being run mock_subprocess.return_value = 0 - # Mock 'build_image' return values - built_images = [ - f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" - for image in images - ] + # Construct images that will be generated at the different stages of the process + built_images: list[str] = [] + other_tags: list[str] = [] + images_to_push: list[str] = [] + for image in images: + built_image = ( + f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" + ) + built_images.append(built_image) + images_to_push.append(built_image) + for tag in (tags if tags else def_tags)[1:]: + new_tag = f"{image.split(':')[0]}:{tag}" + other_tags.append(new_tag) + images_to_push.append(new_tag) + + # Mock the return values of 'build_image' and 'tag_iamge' mock_build.side_effect = built_images + mock_tag.side_effect = other_tags - # Mock all the return values when tagging the images - all_tags = [ - f"{image.split(':')[0]}:{tag}" - for image in built_images - for tag in (tags if tags else def_tags) - ] - mock_tag.side_effect = all_tags - - # Mock the push function + # Mock the push and cleanup functions mock_push.return_value = True - - # Mock the cleanup function mock_clean.return_value = True # Run the function with the command run() - # Check that the functions were called with the correct flags + # Check that 'build_image' was called with the correct arguments assert mock_build.call_count == len(images) - expected_calls = ( + expected_build_calls = ( call( image=image, tag=tags[0] if tags else def_tags[0], @@ -125,4 +127,15 @@ def test_run( ) for image in images ) - mock_build.assert_has_calls(expected_calls, any_order=True) + mock_build.assert_has_calls(expected_build_calls, any_order=True) + + if other_tags: + expected_tag_calls = ( + call( + image_path=image, + tags=other_tags, + dry_run=dry_run if dry_run else def_dry_run, + ) + for image in built_images + ) + mock_tag.assert_has_calls(expected_tag_calls, any_order=True) From 8d6978d5dc1861a04f8555a9824c335d70058638 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 05:56:46 +0000 Subject: [PATCH 14/30] Checked that 'push_images' and 'cleanup ' are called correctly --- tests/cli/test_build_images.py | 47 +++++++++++++++++++++++++++++++++- 1 file changed, 46 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 9998696d4..e0aa10cb9 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -20,11 +20,37 @@ test_run_params_matrix: tuple[ - tuple[list[str], list[str], str, str, str, str, str, bool] + tuple[list[str], list[str], str, str, str, str, str, bool], ... ] = ( # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Default settings (images, [], "", "", "", "", "", False), + ( + images, + [ + "latest", + "dev", + ], + "", + "docker.io", + "12345", + "34567", + "my-group", + False, + ), + ( + images, + [ + "latest", + "dev", + ], + "", + "docker.io", + "12345", + "34567", + "my-group", + True, + ), ) @@ -129,7 +155,9 @@ def test_run( ) mock_build.assert_has_calls(expected_build_calls, any_order=True) + # Check that 'tag_image' was called with the correct arguments if other_tags: + assert mock_tag.call_count == len(images) * len(other_tags) expected_tag_calls = ( call( image_path=image, @@ -139,3 +167,20 @@ def test_run( for image in built_images ) mock_tag.assert_has_calls(expected_tag_calls, any_order=True) + + # Check that 'push_images' was called with the correct arguments + mock_push.assert_called_once_with( + call( + images=images_to_push, + dry_run=dry_run if dry_run else def_dry_run, + ), + any_order=True, + ) + + # Check that 'cleanup' was called correctly + mock_clean.assert_called_once_with( + call( + dry_run=dry_run if dry_run else def_dry_run, + ), + any_order=True, + ) From 05aff186477e2759171c0f3a0db4f8f96cf58455 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 06:08:38 +0000 Subject: [PATCH 15/30] Tried to fix errors with assert calls for 'push_images' and 'cleanup' --- tests/cli/test_build_images.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index e0aa10cb9..743405327 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -118,7 +118,7 @@ def test_run( images_to_push: list[str] = [] for image in images: built_image = ( - f"{dst if dst else def_dst}/{image[0]}:{tags[0] if tags else def_tags[0]}" + f"{dst if dst else def_dst}/{image}:{tags[0] if tags else def_tags[0]}" ) built_images.append(built_image) images_to_push.append(built_image) @@ -170,17 +170,11 @@ def test_run( # Check that 'push_images' was called with the correct arguments mock_push.assert_called_once_with( - call( - images=images_to_push, - dry_run=dry_run if dry_run else def_dry_run, - ), - any_order=True, + images=images_to_push, + dry_run=dry_run if dry_run else def_dry_run, ) # Check that 'cleanup' was called correctly mock_clean.assert_called_once_with( - call( - dry_run=dry_run if dry_run else def_dry_run, - ), - any_order=True, + dry_run=dry_run if dry_run else def_dry_run, ) From 8fa7ebf205248a7a5a07dae97e875cdd9d67e2e0 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 06:21:12 +0000 Subject: [PATCH 16/30] Fixed incorrect logic to get call count for 'tag_image' --- tests/cli/test_build_images.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 743405327..00ee5c213 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -25,12 +25,10 @@ # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Default settings (images, [], "", "", "", "", "", False), + # Populated flags ( images, - [ - "latest", - "dev", - ], + ["latest", "dev", "1.1.1"], "", "docker.io", "12345", @@ -40,10 +38,7 @@ ), ( images, - [ - "latest", - "dev", - ], + ["latest", "dev", "1.1.1"], "", "docker.io", "12345", @@ -157,11 +152,11 @@ def test_run( # Check that 'tag_image' was called with the correct arguments if other_tags: - assert mock_tag.call_count == len(images) * len(other_tags) + assert mock_tag.call_count == len(other_tags) expected_tag_calls = ( call( image_path=image, - tags=other_tags, + tags=tags[1:], dry_run=dry_run if dry_run else def_dry_run, ) for image in built_images From b8b10bd00f087f621ce8558e1cd2d6205b71720d Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 06:37:43 +0000 Subject: [PATCH 17/30] Passed 'images' in 'push_images' as a positional argument --- src/murfey/cli/build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/murfey/cli/build_images.py b/src/murfey/cli/build_images.py index 1c09e3384..2232cdea1 100644 --- a/src/murfey/cli/build_images.py +++ b/src/murfey/cli/build_images.py @@ -288,7 +288,7 @@ def run(): images.extend(new_tags) # Push all built images to specified repo - push_images(images, dry_run=args.dry_run) + push_images(images=images, dry_run=args.dry_run) # Perform final cleanup cleanup(dry_run=args.dry_run) From b9a4801f9b609239a1973dc4273487c37ddcdf6f Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 06:43:49 +0000 Subject: [PATCH 18/30] Call count for 'tag_image' was calculated incorrectly --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 00ee5c213..7d29bfd4a 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -152,7 +152,7 @@ def test_run( # Check that 'tag_image' was called with the correct arguments if other_tags: - assert mock_tag.call_count == len(other_tags) + assert mock_tag.call_count == len(built_images) expected_tag_calls = ( call( image_path=image, From 4e8927f8f8a5dab4481653de9d87b7e736435c04 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 06:58:52 +0000 Subject: [PATCH 19/30] Tried to fix logic when parsing space-separated tags --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 7d29bfd4a..27fb79c82 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -92,7 +92,7 @@ def test_run( ) for flag, value in flags: if isinstance(value, list) and value: - build_cmd.extend([flag, *value]) + build_cmd.extend([flag, " ".join(value)]) if isinstance(value, str) and value: build_cmd.extend([flag, value]) if isinstance(value, bool) and value: From 336ea0bc481d6e05dfa79bc8b796e158298cbfc2 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 07:03:43 +0000 Subject: [PATCH 20/30] Fixed logic when building tags for the mock return value to pass to next function --- tests/cli/test_build_images.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 27fb79c82..b4896f395 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -92,7 +92,7 @@ def test_run( ) for flag, value in flags: if isinstance(value, list) and value: - build_cmd.extend([flag, " ".join(value)]) + build_cmd.extend([flag, *value]) if isinstance(value, str) and value: build_cmd.extend([flag, value]) if isinstance(value, bool) and value: @@ -118,7 +118,7 @@ def test_run( built_images.append(built_image) images_to_push.append(built_image) for tag in (tags if tags else def_tags)[1:]: - new_tag = f"{image.split(':')[0]}:{tag}" + new_tag = f"{built_image.split(':')[0]}:{tag}" other_tags.append(new_tag) images_to_push.append(new_tag) From c69a79047b843d5253a58de0e48dbdac3548997f Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 08:40:33 +0000 Subject: [PATCH 21/30] Logic for preparing return value of 'tag_image' was incorrect --- tests/cli/test_build_images.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index b4896f395..5f0c405d2 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -109,7 +109,7 @@ def test_run( # Construct images that will be generated at the different stages of the process built_images: list[str] = [] - other_tags: list[str] = [] + other_tags: list[list[str]] = [] images_to_push: list[str] = [] for image in images: built_image = ( @@ -117,10 +117,12 @@ def test_run( ) built_images.append(built_image) images_to_push.append(built_image) - for tag in (tags if tags else def_tags)[1:]: - new_tag = f"{built_image.split(':')[0]}:{tag}" - other_tags.append(new_tag) - images_to_push.append(new_tag) + new_tags = [ + f"{built_image.split(':')[0]}:{tag}" + for tag in (tags if tags else def_tags)[1:] + ] + other_tags.append(new_tags) + images_to_push.extend(new_tags) # Mock the return values of 'build_image' and 'tag_iamge' mock_build.side_effect = built_images From 5d45b2de00a0a68107b85d721c18c5edd15e0531 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 08:53:43 +0000 Subject: [PATCH 22/30] More fixes to test logic for 'tag_image' call count --- tests/cli/test_build_images.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 5f0c405d2..50a2042bd 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -153,8 +153,8 @@ def test_run( mock_build.assert_has_calls(expected_build_calls, any_order=True) # Check that 'tag_image' was called with the correct arguments - if other_tags: - assert mock_tag.call_count == len(built_images) + if tags[1:]: + assert mock_tag.call_count == len(tags[1:]) expected_tag_calls = ( call( image_path=image, From 298f9ce9e5f2d285b24c470c62674ed941b910f2 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 08:57:54 +0000 Subject: [PATCH 23/30] More fixes to test logic for 'tag_image' call count --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 50a2042bd..9c6143677 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -154,7 +154,7 @@ def test_run( # Check that 'tag_image' was called with the correct arguments if tags[1:]: - assert mock_tag.call_count == len(tags[1:]) + assert mock_tag.call_count == len(built_images) expected_tag_calls = ( call( image_path=image, From 6660f2ae89585db1b8a0ed610e53d385657b323c Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 10:32:18 +0000 Subject: [PATCH 24/30] Added unit test for the 'build_image' function --- tests/cli/test_build_images.py | 58 +++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 9c6143677..189904ba7 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -5,7 +5,7 @@ import pytest -from murfey.cli.build_images import run +from murfey.cli.build_images import build_image, run images = [f"test_image_{n}" for n in range(3)] @@ -19,6 +19,62 @@ def_dry_run = False +build_image_params_matrix: tuple[ + tuple[list[str], list[str], str, str, int, int, str, bool], ... +] = ( + # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Populated flags + ( + images, + ["latest", "dev", "1.1.1"], + "", + "docker.io", + 12345, + 34567, + "my-group", + False, + ), + ( + images, + ["latest", "dev", "1.1.1"], + "", + "docker.io", + 12345, + 34567, + "my-group", + True, + ), +) + + +@pytest.mark.parametrize("build_params", build_image_params_matrix) +@patch("murfey.cli.build_images.Path.exists") +@patch("murfey.cli.build_images.run_subprocess") +def test_build_images(mock_subprocess, mock_exists, build_params): + + # Unpack build params + images, tags, src, dst, uid, gid, gname, dry_run = build_params + + # Set the return values for 'Path().exists()' and 'run_subprocess' + mock_exists.return_value = True + mock_subprocess.return_value = 0 + + # Run the command + built_image = build_image( + image=images[0], + tag=tags[0], + source=src, + destination=dst, + user_id=uid, + group_id=gid, + group_name=gname, + dry_run=dry_run, + ) + + # Check that the image path generated is correct + assert built_image == f"{dst}/{images[0]:{tags[0]}}" + + test_run_params_matrix: tuple[ tuple[list[str], list[str], str, str, str, str, str, bool], ... ] = ( From 3c148e2afe49046a6d111f594553a5eb0da1c03b Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 10:40:25 +0000 Subject: [PATCH 25/30] Incorrect brackets --- tests/cli/test_build_images.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 189904ba7..5ad183f93 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -72,7 +72,7 @@ def test_build_images(mock_subprocess, mock_exists, build_params): ) # Check that the image path generated is correct - assert built_image == f"{dst}/{images[0]:{tags[0]}}" + assert built_image == f"{dst}/{images[0]}:{tags[0]}" test_run_params_matrix: tuple[ From a7a51c6b907e99ad3facb42e71b3113930789b65 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 11:17:17 +0000 Subject: [PATCH 26/30] Added unit test for 'tag_image' function --- tests/cli/test_build_images.py | 46 ++++++++++++++++++++++++++++++++-- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 5ad183f93..cd41675ab 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -5,7 +5,7 @@ import pytest -from murfey.cli.build_images import build_image, run +from murfey.cli.build_images import build_image, run, tag_image images = [f"test_image_{n}" for n in range(3)] @@ -50,7 +50,7 @@ @pytest.mark.parametrize("build_params", build_image_params_matrix) @patch("murfey.cli.build_images.Path.exists") @patch("murfey.cli.build_images.run_subprocess") -def test_build_images(mock_subprocess, mock_exists, build_params): +def test_build_image(mock_subprocess, mock_exists, build_params): # Unpack build params images, tags, src, dst, uid, gid, gname, dry_run = build_params @@ -75,6 +75,48 @@ def test_build_images(mock_subprocess, mock_exists, build_params): assert built_image == f"{dst}/{images[0]}:{tags[0]}" +tag_image_params_matrix: tuple[tuple[list[str], list[str], str, bool], ...] = ( + # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Populated flags + ( + images, + ["latest", "dev", "1.1.1"], + "docker.io", + False, + ), + ( + images, + ["latest", "dev", "1.1.1"], + "docker.io", + True, + ), +) + + +@pytest.mark.parametrize("tag_params", tag_image_params_matrix) +@patch("murfey.cli.build_images.run_subprocess") +def test_tag_image(mock_subprocess, tag_params): + + # Unpack build params + images, tags, src, dst, uid, gid, gname, dry_run = tag_params + + # Check that the image path generated is correct + built_image = f"{dst}/{images[0]}:{tags[0]}" + + # Set the return value for 'run_subprocess' + mock_subprocess.return_value = 0 + + # Run the command + image_tags = tag_image( + image_path=built_image, + tags=tags[1:], + dry_run=dry_run, + ) + + # Check that the images are tagged correctly + assert image_tags == [f"{built_image.split(':')[0]}:{tag}" for tag in tags[1:]] + + test_run_params_matrix: tuple[ tuple[list[str], list[str], str, str, str, str, str, bool], ... ] = ( From 00dea1592a797fcf05d74f0e35d2fcee9deefe21 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 11:23:59 +0000 Subject: [PATCH 27/30] Fixed incorrect unpacking of test paramaters --- tests/cli/test_build_images.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index cd41675ab..966d293ee 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -98,7 +98,7 @@ def test_build_image(mock_subprocess, mock_exists, build_params): def test_tag_image(mock_subprocess, tag_params): # Unpack build params - images, tags, src, dst, uid, gid, gname, dry_run = tag_params + images, tags, dst, dry_run = tag_params # Check that the image path generated is correct built_image = f"{dst}/{images[0]}:{tags[0]}" @@ -147,7 +147,7 @@ def test_tag_image(mock_subprocess, tag_params): ) -@pytest.mark.parametrize("build_params", test_run_params_matrix) +@pytest.mark.parametrize("run_params", test_run_params_matrix) @patch("murfey.cli.build_images.Path.exists") @patch("murfey.cli.build_images.run_subprocess") @patch("murfey.cli.build_images.cleanup") @@ -161,7 +161,7 @@ def test_run( mock_clean, mock_subprocess, mock_exists, - build_params: tuple[list[str], list[str], str, str, str, str, str, bool], + run_params: tuple[list[str], list[str], str, str, str, str, str, bool], ): """ Tests that the function is run with the expected arguments for a given @@ -169,7 +169,7 @@ def test_run( """ # Unpack build params - images, tags, src, dst, uid, gid, gname, dry_run = build_params + images, tags, src, dst, uid, gid, gname, dry_run = run_params # Set up the command based on what these values are build_cmd = [ From 90485290a74d5ad8ba715356f170237f3e4c10bf Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 12:57:07 +0000 Subject: [PATCH 28/30] Added unit test for the 'push_images' function --- tests/cli/test_build_images.py | 45 ++++++++++++++++++++++++++++++++-- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 966d293ee..932360fce 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -5,7 +5,7 @@ import pytest -from murfey.cli.build_images import build_image, run, tag_image +from murfey.cli.build_images import build_image, push_images, run, tag_image images = [f"test_image_{n}" for n in range(3)] @@ -147,9 +147,50 @@ def test_tag_image(mock_subprocess, tag_params): ) +push_image_params_matrix: tuple[tuple[list[str], list[str], str, bool], ...] = ( + # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Populated flags + ( + images, + ["latest", "dev", "1.1.1"], + "docker.io", + False, + ), + ( + images, + ["latest", "dev", "1.1.1"], + "docker.io", + True, + ), +) + + +@pytest.mark.parametrize("push_params", push_image_params_matrix) +@patch("murfey.cli.build_images.run_subprocess") +def test_push_images( + mock_subprocess, + push_params, +): + + # Unpack test parameters + images, tags, dst, dry_run = push_params + + # Construct all images to be pushed + images_to_push = [f"{dst}/{image}:{tag}" for image in images for tag in tags] + + # Mock the subprocess return value + mock_subprocess.return_value = True + + # Run the function + result = push_images( + images=images_to_push, + dry_run=dry_run, + ) + assert result + + @pytest.mark.parametrize("run_params", test_run_params_matrix) @patch("murfey.cli.build_images.Path.exists") -@patch("murfey.cli.build_images.run_subprocess") @patch("murfey.cli.build_images.cleanup") @patch("murfey.cli.build_images.push_images") @patch("murfey.cli.build_images.tag_image") From be2f38e41a24d8edf24549fc959ee90f7943c673 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 13:07:35 +0000 Subject: [PATCH 29/30] Accidentally removed a patched function from 'run()' test; fixed incorrect return code for mocked subprocess; fixed incorrect order to variables and the functions they are passed into --- tests/cli/test_build_images.py | 63 +++++++++++++++++----------------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 932360fce..785cb0de3 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -117,36 +117,6 @@ def test_tag_image(mock_subprocess, tag_params): assert image_tags == [f"{built_image.split(':')[0]}:{tag}" for tag in tags[1:]] -test_run_params_matrix: tuple[ - tuple[list[str], list[str], str, str, str, str, str, bool], ... -] = ( - # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run - # Default settings - (images, [], "", "", "", "", "", False), - # Populated flags - ( - images, - ["latest", "dev", "1.1.1"], - "", - "docker.io", - "12345", - "34567", - "my-group", - False, - ), - ( - images, - ["latest", "dev", "1.1.1"], - "", - "docker.io", - "12345", - "34567", - "my-group", - True, - ), -) - - push_image_params_matrix: tuple[tuple[list[str], list[str], str, bool], ...] = ( # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run # Populated flags @@ -179,7 +149,7 @@ def test_push_images( images_to_push = [f"{dst}/{image}:{tag}" for image in images for tag in tags] # Mock the subprocess return value - mock_subprocess.return_value = True + mock_subprocess.return_value = 0 # Run the function result = push_images( @@ -189,8 +159,39 @@ def test_push_images( assert result +test_run_params_matrix: tuple[ + tuple[list[str], list[str], str, str, str, str, str, bool], ... +] = ( + # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Default settings + (images, [], "", "", "", "", "", False), + # Populated flags + ( + images, + ["latest", "dev", "1.1.1"], + "", + "docker.io", + "12345", + "34567", + "my-group", + False, + ), + ( + images, + ["latest", "dev", "1.1.1"], + "", + "docker.io", + "12345", + "34567", + "my-group", + True, + ), +) + + @pytest.mark.parametrize("run_params", test_run_params_matrix) @patch("murfey.cli.build_images.Path.exists") +@patch("murfey.cli.build_images.run_subprocess") @patch("murfey.cli.build_images.cleanup") @patch("murfey.cli.build_images.push_images") @patch("murfey.cli.build_images.tag_image") From 94a7cc9d21588d55bf57764c31b192485c736ac6 Mon Sep 17 00:00:00 2001 From: Eu Pin Tien Date: Fri, 28 Feb 2025 13:49:10 +0000 Subject: [PATCH 30/30] Added unit test for 'cleanup' function --- tests/cli/test_build_images.py | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/tests/cli/test_build_images.py b/tests/cli/test_build_images.py index 785cb0de3..86489b114 100644 --- a/tests/cli/test_build_images.py +++ b/tests/cli/test_build_images.py @@ -5,7 +5,7 @@ import pytest -from murfey.cli.build_images import build_image, push_images, run, tag_image +from murfey.cli.build_images import build_image, cleanup, push_images, run, tag_image images = [f"test_image_{n}" for n in range(3)] @@ -76,7 +76,7 @@ def test_build_image(mock_subprocess, mock_exists, build_params): tag_image_params_matrix: tuple[tuple[list[str], list[str], str, bool], ...] = ( - # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Images | Tags | Destination | Dry Run # Populated flags ( images, @@ -118,7 +118,7 @@ def test_tag_image(mock_subprocess, tag_params): push_image_params_matrix: tuple[tuple[list[str], list[str], str, bool], ...] = ( - # Images | Tags | Source | Destination | User ID | Group ID | Group Name | Dry Run + # Images | Tags | Destination | Dry Run # Populated flags ( images, @@ -159,6 +159,27 @@ def test_push_images( assert result +test_cleanup_params_matrix: tuple[tuple[bool], ...] = ((True,), (False,)) + + +@pytest.mark.parametrize("cleanup_params", test_cleanup_params_matrix) +@patch("murfey.cli.build_images.run_subprocess") +def test_cleanup( + mock_subprocess, + cleanup_params, +): + + # Unpack test params + (dry_run,) = cleanup_params + + # Mock the subprocess return value + mock_subprocess.return_value = 0 + + # Run the function + result = cleanup(dry_run) + assert result + + test_run_params_matrix: tuple[ tuple[list[str], list[str], str, str, str, str, str, bool], ... ] = (