Handle ImageMagick 6 and 7 command names in tests#68
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the ImageMagick command detection in functional tests to support both ImageMagick version 7.x (using the magick command) and version 6.x (using the convert command). This ensures the tests can run successfully regardless of which ImageMagick version is installed.
Key changes:
- Added fallback logic to check for the
convertcommand whenmagickis not found - Updated the error message to be version-agnostic
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c0e741d to
fb07a25
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (magick_path := shutil.which("magick")) is None and ( | ||
| magick_path := shutil.which("convert") | ||
| ) is None: |
There was a problem hiding this comment.
The convert command is ambiguous as it exists in both ImageMagick and other system utilities. Consider using magick convert for ImageMagick 7.x compatibility or checking for convert from ImageMagick specifically by verifying its version output to ensure the correct tool is being used.
| cmd = f'magick "{base_image_path}" "{image_path}"' | ||
| _ = subprocess.run( # noqa: S603 - false positive :( | ||
| [magick_path, str(base_image_path), str(image_path)], | ||
| check=True, | ||
| shell=False, | ||
| ) | ||
|
|
||
| if not image_path.exists(): | ||
| pytest.fail( | ||
| f"Failed to create {image_format} image with command: {cmd}" |
There was a problem hiding this comment.
| cmd = [magick_path, str(base_image_path), str(image_path)] | |
| _ = subprocess.run( # noqa: S603 - false positive :( | |
| cmd, | |
| check=True, | |
| shell=False, | |
| ) | |
| if not image_path.exists(): | |
| pytest.fail( | |
| f"Failed to create {image_format} image with command: {cmd}" |
not gonna block over it but that error isnt quite right anymore, and setting cmd and then not using it is confusing
No description provided.