From af305b22440ebdbbc23ee3b14fb89da5a042b82a Mon Sep 17 00:00:00 2001 From: Luke Hallinan Date: Fri, 13 Jun 2025 23:29:55 +0100 Subject: [PATCH 1/9] decklist from scryfall option added --- desktop-tool/src/constants.py | 5 + desktop-tool/src/io.py | 28 +++ desktop-tool/src/order.py | 417 ++++++++++++++++++++++++++++++--- desktop-tool/src/processing.py | 26 +- 4 files changed, 444 insertions(+), 32 deletions(-) diff --git a/desktop-tool/src/constants.py b/desktop-tool/src/constants.py index 319ff133c..30c054860 100644 --- a/desktop-tool/src/constants.py +++ b/desktop-tool/src/constants.py @@ -181,3 +181,8 @@ class TargetSites(Enum): THREADS = 5 # shared between CardImageCollections POST_LAUNCH_HTML_FILENAME = "post-launch.html" + +# Card physical dimensions for border calculation +CARD_WIDTH_INCHES = 2.5 +CARD_HEIGHT_INCHES = 3.5 +BORDER_INCHES = 0.125 diff --git a/desktop-tool/src/io.py b/desktop-tool/src/io.py index 576441dd8..b907c85a4 100644 --- a/desktop-tool/src/io.py +++ b/desktop-tool/src/io.py @@ -193,4 +193,32 @@ def download_google_drive_file( return True +def download_image_from_url( + url: str, file_path: str, post_processing_config: Optional[ImagePostProcessingConfig] +) -> bool: + """ + Download an image from a URL to the specified `file_path`. + Returns whether the request was successful or not. + """ + logging.debug(f"Downloading image from {url}...") + try: + response = requests.get(url) + response.raise_for_status() + file_bytes = response.content + except requests.exceptions.RequestException as e: + logging.error(f"Encountered an error while downloading image from {url}: {e}") + return False + + if post_processing_config is not None: + logging.debug(f"Post-processing image from {url}...") + processed_image = post_process_image(raw_image=file_bytes, config=post_processing_config) + processed_image.save(file_path) + else: + # Save the bytes directly to disk + with open(file_path, "wb") as f: + f.write(file_bytes) + logging.debug(f"Finished downloading image from {url}!") + return True + + # endregion diff --git a/desktop-tool/src/order.py b/desktop-tool/src/order.py index 3efb8b6dc..11c453b30 100644 --- a/desktop-tool/src/order.py +++ b/desktop-tool/src/order.py @@ -1,19 +1,26 @@ import logging import os +import re import sys +import time from concurrent.futures import ThreadPoolExecutor from functools import reduce from glob import glob +from io import BytesIO from itertools import groupby from pathlib import Path from queue import Queue +from tkinter import Tk, filedialog from typing import Optional +from urllib.parse import quote_plus from xml.etree.ElementTree import Element, ParseError import attr import enlighten +import requests from defusedxml.ElementTree import parse as defused_parse from InquirerPy import prompt +from PIL import Image, ImageOps from sanitize_filename import sanitize from src import constants @@ -21,6 +28,7 @@ from src.io import ( CURRDIR, download_google_drive_file, + download_image_from_url, file_exists, get_google_drive_file_name, image_directory, @@ -31,7 +39,8 @@ @attr.s class CardImage: - drive_id: str = attr.ib(default="") + drive_id: Optional[str] = attr.ib(default=None) + image_url: Optional[str] = attr.ib(default=None) slots: set[int] = attr.ib(factory=set) name: Optional[str] = attr.ib(default="") file_path: Optional[str] = attr.ib(default="") @@ -55,7 +64,7 @@ def retrieve_card_name(self) -> None: Retrieves the file's name based on Google Drive ID. `None` indicates that the file on GDrive is invalid. """ - if not self.name: + if not self.name and self.drive_id: self.name = get_google_drive_file_name(drive_id=self.drive_id) def generate_file_path(self) -> None: @@ -66,7 +75,7 @@ def generate_file_path(self) -> None: * Otherwise, use `self.name` with `self.drive_id` in parentheses in the `cards` directory as the file path. """ - if file_exists(self.drive_id): + if self.drive_id and file_exists(self.drive_id): self.file_path = self.drive_id self.name = os.path.basename(self.file_path) return @@ -113,14 +122,13 @@ def __attrs_post_init__(self) -> None: # region public def combine(self, other: "CardImage") -> "CardImage": - assert self.drive_id == other.drive_id - return CardImage( - drive_id=self.drive_id, - slots=self.slots | other.slots, - name=self.name, - file_path=self.file_path, - query=self.query, - ) + # Assert that all identifying fields are identical before combining. + assert self.drive_id == other.drive_id, f"Drive IDs do not match for {self.name}" + assert self.image_url == other.image_url, f"Image URLs do not match for {self.name}" + assert self.file_path == other.file_path, f"File paths do not match for {self.name}" + + # If they are identical, combine their slots. + return attr.evolve(self, slots=self.slots | other.slots) @classmethod def from_element(cls, element: Element) -> "CardImage": @@ -148,23 +156,34 @@ def download_image( ) -> None: try: if not self.file_exists() and not self.errored and self.file_path is not None: - self.errored = not download_google_drive_file( - drive_id=self.drive_id, file_path=self.file_path, post_processing_config=post_processing_config - ) + if self.image_url: + # Download from URL + self.errored = not download_image_from_url( + url=self.image_url, file_path=self.file_path, post_processing_config=post_processing_config + ) + elif self.drive_id: + # Download from Google Drive + self.errored = not download_google_drive_file( + drive_id=self.drive_id, file_path=self.file_path, post_processing_config=post_processing_config + ) if self.file_exists() and not self.errored: self.downloaded = True else: + download_link = self.image_url or ( + self.drive_id and f"https://drive.google.com/uc?id={self.drive_id}&export=download" + ) logging.info( f"Failed to download '{bold(self.name)}' - allocated to slot/s {bold(sorted(self.slots))}.\n" - f"Download link - {bold(f'https://drive.google.com/uc?id={self.drive_id}&export=download')}\n" + f"Download link - {bold(download_link)}\n" ) except Exception as e: - # note: python threads die silently if they encounter an exception. if an exception does occur, - # log it, but still put the card onto the queue so the main thread doesn't spin its wheels forever waiting. + download_link = self.image_url or ( + self.drive_id and f"https://drive.google.com/uc?id={self.drive_id}&export=download" + ) logging.info( f"An uncaught exception occurred when attempting to download '{bold(self.name)}':\n{bold(e)}\n" - f"Download link - {bold(f'https://drive.google.com/uc?id={self.drive_id}&export=download')}\n" + f"Download link - {bold(download_link)}\n" ) finally: queue.put(self) @@ -216,10 +235,14 @@ class CardImageCollection: face: constants.Faces = attr.ib(default=constants.Faces.front) def append(self, card: CardImage) -> None: - if card.drive_id in self.cards_by_id.keys(): - self.cards_by_id[card.drive_id] = self.cards_by_id[card.drive_id].combine(card) - else: - self.cards_by_id[card.drive_id] = card + key = card.drive_id or card.image_url or card.file_path + + if key: + if key in self.cards_by_id: + self.cards_by_id[key] = self.cards_by_id[key].combine(card) + else: + # Copy the card to avoid side effects + self.cards_by_id[key] = attr.evolve(card) def combine(self, other: "CardImageCollection") -> "CardImageCollection": assert self.face == other.face @@ -268,10 +291,12 @@ def from_element( if element: for x in element: card_image = CardImage.from_element(x) - if card_image.drive_id in card_images.keys(): - card_images[card_image.drive_id] = card_images[card_image.drive_id].combine(card_image) - else: - card_images[card_image.drive_id] = card_image + key = card_image.drive_id + if key is not None: + if key in card_images: + card_images[key] = card_images[key].combine(card_image) + else: + card_images[key] = card_image card_image_collection = cls(cards_by_id=card_images, num_slots=num_slots, face=face) if fill_image_id: # fill the remaining slots in this card image collection with a new card image based off the given id @@ -549,6 +574,23 @@ def from_xmls_in_folder(cls) -> list["CardOrder"]: The primary public entry point to this class. """ + # Ask user for input method + questions = { + "type": "list", + "name": "input_method", + "message": "How would you like to create your order?", + "choices": [ + {"name": "From one or more XML files", "value": "xml"}, + {"name": "By selecting a .txt file with the decklist", "value": "decklist"}, + ], + } + answers = prompt(questions) + input_method = answers["input_method"] + + if input_method == "decklist": + # Create a single order from a decklist and return it in a list + return [cls.from_decklist()] + xml_glob = sorted(glob(os.path.join(CURRDIR, "*.xml"))) if len(xml_glob) <= 0: input("No XML files found in this directory. Press enter to exit.") @@ -561,14 +603,18 @@ def from_xmls_in_folder(cls) -> list["CardOrder"]: "Select files by pressing Space, then confirm your selection by pressing Enter." ) questions = { - "type": "list", + "type": "checkbox", "name": "xml_choice", "message": xml_select_string, - "choices": xml_glob, - "multiselect": True, + "choices": [{"name": os.path.basename(p), "value": p} for p in xml_glob], } answers = prompt(questions) file_paths = answers["xml_choice"] + + if not file_paths: + input("No XML files selected. Press enter to exit.") + sys.exit(0) + return [cls.from_file_path(file_path) for file_path in file_paths] @classmethod @@ -586,6 +632,319 @@ def get_overview(self) -> str: f"{bold(self.details.stock)} cardstock ({bold('foil' if self.details.foil else 'nonfoil')}). " ) + @classmethod + def from_decklist(cls) -> "CardOrder": + """ + Creates a CardOrder by prompting the user to select a decklist file and provide order details. + """ + SCRYFALL_API_BASE_URL = "https://api.scryfall.com" + REQUEST_DELAY = 1 + + def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: + return ImageOps.expand(image, border=border_size, fill="black") + + # Get User Inputs + print("\nA file dialog will now open. Please select a decklist .txt file.") + root = Tk() + root.attributes("-topmost", True) + root.withdraw() + decklist_path = filedialog.askopenfilename( + parent=root, title="Please select your decklist .txt file", filetypes=[("Text files", "*.txt")] + ) + + if not decklist_path: + input("No decklist file selected. Press enter to exit.") + sys.exit(0) + + with open(decklist_path, "r", encoding="utf-8") as f: + deck_lines = f.readlines() + + details_answers = prompt( + [ + { + "type": "list", + "name": "stock", + "message": "Select cardstock:", + "choices": [s.value for s in constants.Cardstocks], + }, + {"type": "confirm", "name": "foil", "message": "Are the cards foil?"}, + ] + ) + + print("\nA file dialog will now open. Please select a common card back for this order.") + card_back_path = filedialog.askopenfilename(parent=root, title="Please select a common card back image") + root.destroy() + if not card_back_path: + input("\nNo card back selected. Exiting program.") + sys.exit(0) + + # Fetch all card data and defer meld cards + fronts = CardImageCollection(face=constants.Faces.front) + backs = CardImageCollection(face=constants.Faces.back) + current_slot = 0 + + meld_cards_in_deck = [] + final_messages = [] + + print("\nParsing decklist and fetching from Scryfall...") + for line in deck_lines: + line = line.strip() + if not line: + continue + + try: + qty_str, rest_of_line = line.split(" ", 1) + card_qty = int(qty_str) + except ValueError: + logging.warning(f"Could not parse quantity from line: '{line}'. Skipping.") + continue + + set_code, number, name = None, None, "" + set_regex = re.compile(r"\s+\(([^)]+)\)\s+([\w\d-]+)") + set_match = set_regex.search(rest_of_line) + + if set_match: + name = rest_of_line[: set_match.start()].strip() + set_code = set_match.group(1) + number = set_match.group(2) + else: + name = re.split(r"\s*(\/\/|\*F\*|\s\()", rest_of_line, 1)[0].strip() + + if not name: + logging.warning(f"Could not parse card name from line: '{line}'. Skipping.") + continue + + api_url = "" + if set_code and number: + search_query = f'++!"{name}" set:{set_code} cn:"{number}"' + api_url = f"{SCRYFALL_API_BASE_URL}/cards/search?q={quote_plus(search_query)}" + else: + api_url = f"{SCRYFALL_API_BASE_URL}/cards/named?exact={quote_plus(name)}" + + try: + time.sleep(REQUEST_DELAY) + response = requests.get(api_url) + response.raise_for_status() + json_response = response.json() + + card_data = None + if ( + "object" in json_response + and json_response["object"] == "list" + and json_response.get("total_cards", 0) > 0 + ): + card_data = json_response["data"][0] + elif "object" in json_response and json_response["object"] == "card": + card_data = json_response + + if not card_data: + raise ValueError(f"Card not found on Scryfall for query: {name}") + + layout = card_data.get("layout", "normal") + + if layout == "meld": + logging.info(f" Deferring Meld Part: {name}") + meld_cards_in_deck.append({"data": card_data, "qty": card_qty}) + else: + if ( + layout in ["transform", "modal_dfc", "double_faced_token", "reversible_card"] + and "card_faces" in card_data + and len(card_data["card_faces"]) > 1 + ): + face_front, face_back = card_data["card_faces"][0], card_data["card_faces"][1] + url_front, url_back = face_front.get("image_uris", {}).get("png"), face_back.get( + "image_uris", {} + ).get("png") + + if url_front and url_back: + logging.info(f" Found DFC: {name}") + for _ in range(card_qty): + fronts.append( + CardImage( + image_url=url_front, + name=f"{sanitize(face_front.get('name'))}.png", + slots={current_slot}, + ) + ) + backs.append( + CardImage( + image_url=url_back, + name=f"{sanitize(face_back.get('name'))}.png", + slots={current_slot}, + ) + ) + current_slot += 1 + elif url_front: + final_messages.append(f"Back face missing for '{name}'. It will be left blank.") + for _ in range(card_qty): + fronts.append( + CardImage( + image_url=url_front, + name=f"{sanitize(face_front.get('name'))}.png", + slots={current_slot}, + ) + ) + backs.append( + CardImage(name="MISSING_BACK.png", slots={current_slot}) + ) # Blank placeholder + current_slot += 1 + else: + image_url = card_data.get("image_uris", {}).get("png") + if image_url: + logging.info(f" Found SFC: {name} (layout: {layout})") + for _ in range(card_qty): + fronts.append( + CardImage( + image_url=image_url, + name=f"{sanitize(card_data.get('name'))}.png", + slots={current_slot}, + ) + ) + current_slot += 1 + except (requests.exceptions.RequestException, ValueError) as e: + logging.error(f" Error finding '{name}': {e}. Skipping.") + + # Group and process deferred meld cards + if meld_cards_in_deck: + logging.info("\nProcessing meld cards...") + meld_groups = {} + for card_info in meld_cards_in_deck: + result_part = next( + (p for p in card_info["data"].get("all_parts", []) if p.get("component") == "meld_result"), None + ) + if result_part and result_part.get("uri"): + if result_part["uri"] not in meld_groups: + meld_groups[result_part["uri"]] = { + "all_parts": card_info["data"]["all_parts"], + "cards_to_add": [], + } + meld_groups[result_part["uri"]]["cards_to_add"].append(card_info) + else: + final_messages.append( + f"Meld result data missing for '{card_info['data']['name']}'. Back will be left blank." + ) + for _ in range(card_info["qty"]): + fronts.append( + CardImage( + image_url=card_info["data"]["image_uris"]["png"], + name=f"{sanitize(card_info['data']['name'])}.png", + slots={current_slot}, + ) + ) + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) # Blank placeholder + current_slot += 1 + + for result_uri, group in meld_groups.items(): + try: + meld_parts_data = [p for p in group["all_parts"] if p.get("component") == "meld_part"] + if len(meld_parts_data) != 2: + raise ValueError(f"Expected 2 meld parts, but found {len(meld_parts_data)}") + + logging.info( + f" Processing meld group for: {meld_parts_data[0]['name']} & {meld_parts_data[1]['name']}" + ) + result_res = requests.get(result_uri) + result_data = result_res.json() + result_image_url = result_data.get("image_uris", {}).get("png") + if not result_image_url: + raise ValueError("Meld result has no png image_uri") + + meld_image_response = requests.get(result_image_url) + meld_image_response.raise_for_status() + img = Image.open(BytesIO(meld_image_response.content)) + + width, height = img.size + top_half = img.crop((0, 0, width, height // 2)) + bottom_half = img.crop((0, height // 2, width, height)) + top_half, bottom_half = top_half.transpose(Image.Transpose.ROTATE_90), bottom_half.transpose( + Image.Transpose.ROTATE_90 + ) + + border_top = round((top_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES) + border_bottom = round( + (bottom_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES + ) + + top_path = os.path.join(image_directory(), f"meld_back_{sanitize(meld_parts_data[0]['name'])}.png") + bottom_path = os.path.join( + image_directory(), f"meld_back_{sanitize(meld_parts_data[1]['name'])}.png" + ) + _add_black_border(top_half, border_top).save(top_path) + _add_black_border(bottom_half, border_bottom).save(bottom_path) + + back_paths = {meld_parts_data[0]["id"]: top_path, meld_parts_data[1]["id"]: bottom_path} + + for card_info in group["cards_to_add"]: + card_data, qty = card_info["data"], card_info["qty"] + back_path = back_paths.get(card_data["id"]) + + for _ in range(qty): + fronts.append( + CardImage( + image_url=card_data["image_uris"]["png"], + name=f"{sanitize(card_data['name'])}.png", + slots={current_slot}, + ) + ) + if back_path: + backs.append( + CardImage( + file_path=back_path, name=os.path.basename(back_path), slots={current_slot} + ) + ) + else: + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 + except Exception as e: + card_names = [c["data"]["name"] for c in group["cards_to_add"]] + final_messages.append( + f"Could not process meld back for: {', '.join(card_names)}. Backs will be left blank. Reason: {e}" + ) + for card_info in group["cards_to_add"]: + for _ in range(card_info["qty"]): + fronts.append( + CardImage( + image_url=card_info["data"]["image_uris"]["png"], + name=f"{sanitize(card_info['data']['name'])}.png", + slots={current_slot}, + ) + ) + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 + + # Finalize and create order + total_quantity = current_slot + if total_quantity == 0: + input("Could not find any cards from the provided decklist. Press enter to exit.") + sys.exit(0) + + fronts.num_slots, backs.num_slots = total_quantity, total_quantity + + if card_back_path: + slots_for_common_back = set(range(total_quantity)) - backs.slots() + if slots_for_common_back: + backs.append( + CardImage( + file_path=card_back_path, name=os.path.basename(card_back_path), slots=slots_for_common_back + ) + ) + + details = Details( + quantity=total_quantity, + stock=details_answers["stock"], + foil=details_answers["foil"], + allowed_to_exceed_project_max_size=True, + ) + deck_name = sanitize(input("\nEnter a name for this deck/order: ").strip()) or "Decklist Order" + + if final_messages: + print("\n--- Notes on your order ---") + for msg in final_messages: + print(f"- {msg}") + print("--------------------------") + + return cls(name=deck_name, details=details, fronts=fronts, backs=backs) + # endregion diff --git a/desktop-tool/src/processing.py b/desktop-tool/src/processing.py index 9ed7d8678..6e3189c64 100644 --- a/desktop-tool/src/processing.py +++ b/desktop-tool/src/processing.py @@ -1,9 +1,15 @@ import io +import logging from dataclasses import dataclass -from PIL import Image +from PIL import Image, ImageOps -from src.constants import DPI_HEIGHT_RATIO, ImageResizeMethods +from src.constants import ( + BORDER_INCHES, + CARD_WIDTH_INCHES, + DPI_HEIGHT_RATIO, + ImageResizeMethods, +) @dataclass @@ -13,9 +19,23 @@ class ImagePostProcessingConfig: # jpeg: bool -def post_process_image(raw_image: bytes, config: ImagePostProcessingConfig) -> Image: +def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: + """Adds a black border to a given Pillow image object.""" + return ImageOps.expand(image, border=border_size, fill="black") + + +def post_process_image(raw_image: bytes, config: ImagePostProcessingConfig) -> Image.Image: img = Image.open(io.BytesIO(raw_image)) + # Automatically add a 1/8 inch black border + pixel_width, _ = img.size + # Assuming standard card dimensions to calculate DPI for the border + if pixel_width > 0: + effective_dpi = pixel_width / CARD_WIDTH_INCHES + border_pixels = round(effective_dpi * BORDER_INCHES) + img = _add_black_border(img, border_pixels) + logging.info(f"Added {border_pixels}px black border for printing.") + # downscale the image to `max_dpi` img_dpi = 10 * round(int(img.height) * DPI_HEIGHT_RATIO / 10) if img_dpi > config.max_dpi: From e5384e51318db607fcb0bb03686478984292196d Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Sat, 14 Jun 2025 18:18:53 +0100 Subject: [PATCH 2/9] updated selection of both xml and text --- desktop-tool/src/order.py | 213 ++++++++++++++++++++------------- desktop-tool/src/processing.py | 17 +-- 2 files changed, 128 insertions(+), 102 deletions(-) diff --git a/desktop-tool/src/order.py b/desktop-tool/src/order.py index 11c453b30..8fb5b3284 100644 --- a/desktop-tool/src/order.py +++ b/desktop-tool/src/order.py @@ -1,4 +1,5 @@ import logging +import math import os import re import sys @@ -580,8 +581,8 @@ def from_xmls_in_folder(cls) -> list["CardOrder"]: "name": "input_method", "message": "How would you like to create your order?", "choices": [ - {"name": "From one or more XML files", "value": "xml"}, - {"name": "By selecting a .txt file with the decklist", "value": "decklist"}, + {"name": "From an XML file (MPC autofill)", "value": "xml"}, + {"name": "From a text file (Scryfall downloader)", "value": "decklist"}, ], } answers = prompt(questions) @@ -598,21 +599,22 @@ def from_xmls_in_folder(cls) -> list["CardOrder"]: elif len(xml_glob) == 1: file_paths = [xml_glob[0]] else: - xml_select_string = ( - "Multiple XML files found. Please select any number of them to process.\n" - "Select files by pressing Space, then confirm your selection by pressing Enter." - ) + xml_select_string = "Multiple XML files found. Please select which one to process." questions = { - "type": "checkbox", + "type": "list", "name": "xml_choice", "message": xml_select_string, "choices": [{"name": os.path.basename(p), "value": p} for p in xml_glob], } answers = prompt(questions) - file_paths = answers["xml_choice"] + selected_file = answers.get("xml_choice") + if selected_file: + file_paths = [selected_file] + else: + file_paths = [] if not file_paths: - input("No XML files selected. Press enter to exit.") + input("No XML file selected. Press enter to exit.") sys.exit(0) return [cls.from_file_path(file_path) for file_path in file_paths] @@ -643,6 +645,35 @@ def from_decklist(cls) -> "CardOrder": def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: return ImageOps.expand(image, border=border_size, fill="black") + def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: + """Downloads an image from a URL, adds a border, saves it locally, and returns the file path.""" + try: + safe_name = sanitize(card_name) + file_name = f"{safe_name}.png" + save_path = os.path.join(image_directory(), file_name) + + if os.path.exists(save_path): + logging.info(f" Using existing image for: {card_name}") + return save_path + + logging.info(f" Fetching and bordering: {card_name}") + response = requests.get(url) + response.raise_for_status() + img = Image.open(BytesIO(response.content)) + + pixel_width, _ = img.size + if pixel_width > 0: + effective_dpi = pixel_width / constants.CARD_WIDTH_INCHES + border_pixels = math.ceil(effective_dpi * constants.BORDER_INCHES) + img = _add_black_border(img, border_pixels) + + img.save(save_path, "PNG") + return save_path + + except (requests.exceptions.RequestException, IOError) as e: + logging.error(f" Error processing image for '{card_name}': {e}. Skipping.") + return None + # Get User Inputs print("\nA file dialog will now open. Please select a decklist .txt file.") root = Tk() @@ -752,55 +783,54 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: and len(card_data["card_faces"]) > 1 ): face_front, face_back = card_data["card_faces"][0], card_data["card_faces"][1] - url_front, url_back = face_front.get("image_uris", {}).get("png"), face_back.get( - "image_uris", {} - ).get("png") + url_front = face_front.get("image_uris", {}).get("png") + url_back = face_back.get("image_uris", {}).get("png") - if url_front and url_back: + if url_front: logging.info(f" Found DFC: {name}") - for _ in range(card_qty): - fronts.append( - CardImage( - image_url=url_front, - name=f"{sanitize(face_front.get('name'))}.png", - slots={current_slot}, - ) - ) - backs.append( - CardImage( - image_url=url_back, - name=f"{sanitize(face_back.get('name'))}.png", - slots={current_slot}, - ) - ) - current_slot += 1 - elif url_front: - final_messages.append(f"Back face missing for '{name}'. It will be left blank.") - for _ in range(card_qty): - fronts.append( - CardImage( - image_url=url_front, - name=f"{sanitize(face_front.get('name'))}.png", - slots={current_slot}, + front_path = _fetch_and_prepare_image(url_front, face_front.get("name")) + back_path = None + if url_back: + back_path = _fetch_and_prepare_image(url_back, face_back.get("name")) + else: + final_messages.append(f"Back face missing for '{name}'. It will be left blank.") + + if front_path: + for _ in range(card_qty): + fronts.append( + CardImage( + file_path=front_path, + name=os.path.basename(front_path), + slots={current_slot}, + ) ) - ) - backs.append( - CardImage(name="MISSING_BACK.png", slots={current_slot}) - ) # Blank placeholder - current_slot += 1 + if back_path: + backs.append( + CardImage( + file_path=back_path, + name=os.path.basename(back_path), + slots={current_slot}, + ) + ) + else: + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 + else: image_url = card_data.get("image_uris", {}).get("png") if image_url: logging.info(f" Found SFC: {name} (layout: {layout})") - for _ in range(card_qty): - fronts.append( - CardImage( - image_url=image_url, - name=f"{sanitize(card_data.get('name'))}.png", - slots={current_slot}, + image_path = _fetch_and_prepare_image(image_url, card_data.get("name")) + if image_path: + for _ in range(card_qty): + fronts.append( + CardImage( + file_path=image_path, + name=os.path.basename(image_path), + slots={current_slot}, + ) ) - ) - current_slot += 1 + current_slot += 1 except (requests.exceptions.RequestException, ValueError) as e: logging.error(f" Error finding '{name}': {e}. Skipping.") @@ -823,16 +853,18 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: final_messages.append( f"Meld result data missing for '{card_info['data']['name']}'. Back will be left blank." ) - for _ in range(card_info["qty"]): - fronts.append( - CardImage( - image_url=card_info["data"]["image_uris"]["png"], - name=f"{sanitize(card_info['data']['name'])}.png", - slots={current_slot}, - ) - ) - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) # Blank placeholder - current_slot += 1 + image_url = card_info["data"].get("image_uris", {}).get("png") + if image_url: + front_path = _fetch_and_prepare_image(image_url, card_info["data"]["name"]) + if front_path: + for _ in range(card_info["qty"]): + fronts.append( + CardImage( + file_path=front_path, name=os.path.basename(front_path), slots={current_slot} + ) + ) + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 for result_uri, group in meld_groups.items(): try: @@ -860,8 +892,8 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: Image.Transpose.ROTATE_90 ) - border_top = round((top_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES) - border_bottom = round( + border_top = math.ceil((top_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES) + border_bottom = math.ceil( (bottom_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES ) @@ -878,39 +910,48 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: card_data, qty = card_info["data"], card_info["qty"] back_path = back_paths.get(card_data["id"]) - for _ in range(qty): - fronts.append( - CardImage( - image_url=card_data["image_uris"]["png"], - name=f"{sanitize(card_data['name'])}.png", - slots={current_slot}, - ) - ) - if back_path: - backs.append( + image_url = card_data.get("image_uris", {}).get("png") + if not image_url: + continue + + front_path = _fetch_and_prepare_image(image_url, card_data["name"]) + + if front_path: + for _ in range(qty): + fronts.append( CardImage( - file_path=back_path, name=os.path.basename(back_path), slots={current_slot} + file_path=front_path, name=os.path.basename(front_path), slots={current_slot} ) ) - else: - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) - current_slot += 1 + if back_path: + backs.append( + CardImage( + file_path=back_path, name=os.path.basename(back_path), slots={current_slot} + ) + ) + else: + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 except Exception as e: card_names = [c["data"]["name"] for c in group["cards_to_add"]] final_messages.append( f"Could not process meld back for: {', '.join(card_names)}. Backs will be left blank. Reason: {e}" ) for card_info in group["cards_to_add"]: - for _ in range(card_info["qty"]): - fronts.append( - CardImage( - image_url=card_info["data"]["image_uris"]["png"], - name=f"{sanitize(card_info['data']['name'])}.png", - slots={current_slot}, - ) - ) - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) - current_slot += 1 + image_url = card_info["data"].get("image_uris", {}).get("png") + if image_url: + front_path = _fetch_and_prepare_image(image_url, card_info["data"]["name"]) + if front_path: + for _ in range(card_info["qty"]): + fronts.append( + CardImage( + file_path=front_path, + name=os.path.basename(front_path), + slots={current_slot}, + ) + ) + backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + current_slot += 1 # Finalize and create order total_quantity = current_slot diff --git a/desktop-tool/src/processing.py b/desktop-tool/src/processing.py index 6e3189c64..6c3079458 100644 --- a/desktop-tool/src/processing.py +++ b/desktop-tool/src/processing.py @@ -1,15 +1,9 @@ import io -import logging from dataclasses import dataclass from PIL import Image, ImageOps -from src.constants import ( - BORDER_INCHES, - CARD_WIDTH_INCHES, - DPI_HEIGHT_RATIO, - ImageResizeMethods, -) +from src.constants import DPI_HEIGHT_RATIO, ImageResizeMethods @dataclass @@ -27,15 +21,6 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: def post_process_image(raw_image: bytes, config: ImagePostProcessingConfig) -> Image.Image: img = Image.open(io.BytesIO(raw_image)) - # Automatically add a 1/8 inch black border - pixel_width, _ = img.size - # Assuming standard card dimensions to calculate DPI for the border - if pixel_width > 0: - effective_dpi = pixel_width / CARD_WIDTH_INCHES - border_pixels = round(effective_dpi * BORDER_INCHES) - img = _add_black_border(img, border_pixels) - logging.info(f"Added {border_pixels}px black border for printing.") - # downscale the image to `max_dpi` img_dpi = 10 * round(int(img.height) * DPI_HEIGHT_RATIO / 10) if img_dpi > config.max_dpi: From 3f3b54e2f981969e0f0ecb6a33e4a71f5ac6769d Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Sun, 15 Jun 2025 13:03:35 +0100 Subject: [PATCH 3/9] saving work -added tests - some failures --- desktop-tool/tests/test_desktop_client.py | 259 +++++++++++++++++++++- 1 file changed, 258 insertions(+), 1 deletion(-) diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index 3e3488b34..5e2e67fa4 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -9,6 +9,7 @@ import pytest from enlighten import Counter +from PIL import Image from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.wait import WebDriverWait @@ -25,7 +26,7 @@ aggregate_and_split_orders, ) from src.pdf_maker import PdfExporter -from src.processing import ImagePostProcessingConfig +from src.processing import ImagePostProcessingConfig, post_process_image from src.utils import text_to_set DEFAULT_POST_PROCESSING = ImagePostProcessingConfig(max_dpi=800, downscale_alg=constants.ImageResizeMethods.LANCZOS) @@ -492,6 +493,30 @@ def card_order_element_missing_front_image() -> Generator[ElementTree.Element, N # endregion +# endregion + +# region test processing.py + + +def test_post_process_image_with_none_dpi(): + """ + Tests that post_process_image returns the image unchanged when the + config is provided but max_dpi is None. This covers the final + untested branch in the function. + """ + # Create a dummy image that is larger than any potential DPI + img = Image.new("RGB", (1000, 1200), color="red") + + # Create a config with max_dpi set to None + config = ImagePostProcessingConfig(max_dpi=None, downscale_alg=constants.ImageResizeMethods.LANCZOS) + + # Process the image + processed_img = post_process_image(img, config) + + # The image should be identical to the original + assert img == processed_img + + # endregion # region test utils.py @@ -1457,3 +1482,235 @@ def test_card_order_complete_run_multiple_cardbacks(browser, site, input_enter, # endregion + +# region Fixtures for New Tests + + +@pytest.fixture() +def card_order_element_missing_details() -> ElementTree.Element: + """Provides a CardOrder element that is missing the
tag.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + + + + some_id + 0 + a.png + + + some_cardback + + """ + ) + ) + + +@pytest.fixture() +def card_order_element_front_slot_out_of_bounds() -> ElementTree.Element: + """Provides a CardOrder where a front card has a slot index greater than the quantity.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + +
+ 2 + (S30) Standard Smooth + false +
+ + + some_id + 0,2 + a.png + + + some_cardback + + """ + ) + ) + + +@pytest.fixture() +def card_image_collection_element_invalid_slots() -> ElementTree.Element: + """Provides a CardImageCollection where a card's slots are out of bounds.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + + + a + 0,3 + a.png + + + """ + ) + ) + + +# endregion + +# region New Test Cases + + +def test_card_order_missing_details(input_enter, card_order_element_missing_details): + """ + Tests that parsing a CardOrder with a missing
tag exits gracefully. + This covers the error handling path in `CardOrder.from_element`. + """ + with pytest.raises(SystemExit) as exc_info: + CardOrder.from_element(card_order_element_missing_details, allowed_to_exceed_project_max_size=False) + assert exc_info.value.code == 0 + + +def test_card_order_front_slot_out_of_bounds(input_enter, card_order_element_front_slot_out_of_bounds): + """ + Tests that an order is rejected if a front card has a slot index + equal to or greater than the order quantity. + This covers error handling in `CardOrder.from_element`. + """ + with pytest.raises(SystemExit) as exc_info: + CardOrder.from_element(card_order_element_front_slot_out_of_bounds, allowed_to_exceed_project_max_size=False) + assert exc_info.value.code == 0 + + +def test_card_image_collection_invalid_slots(input_enter, card_image_collection_element_invalid_slots): + """ + Tests that a CardImageCollection is rejected if a card's slots exceed the + total number of slots for the collection. + This covers error checking in `CardImageCollection.from_element`. + """ + with pytest.raises(SystemExit) as exc_info: + CardImageCollection.from_element( + card_image_collection_element_invalid_slots, face=constants.Faces.front, num_slots=3 + ) + assert exc_info.value.code == 0 + + +def test_aggregate_orders_with_different_details(monkeypatch_project_max_size): + """ + Tests that orders with different details (stock, foil) are not combined, + even if `combine_orders` is True. + """ + monkeypatch_project_max_size(10) + orders = [ + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S33, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + ] + aggregated = aggregate_and_split_orders( + orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=True + ) + # The orders should not have been combined + assert len(aggregated) == 2 + assert sorted([o.details.quantity for o in aggregated]) == [2, 2] + + +def test_aggregate_orders_no_combine(monkeypatch_project_max_size): + """ + Tests that orders are not combined when `combine_orders` is False. + """ + monkeypatch_project_max_size(10) + orders = [ + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + CardOrder( + details=Details(quantity=3, stock=constants.Cardstocks.S30, foil=False), + fronts=CardImageCollection(num_slots=3), + backs=CardImageCollection(num_slots=3), + ), + ] + aggregated = aggregate_and_split_orders( + orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=False + ) + # The orders should remain separate + assert len(aggregated) == 2 + assert sorted([o.details.quantity for o in aggregated]) == [2, 3] + + +# endregion + +# region test io.py + + +def test_download_image_from_scryfall_success(monkeypatch): + """ + Tests the successful download of an image from Scryfall by mocking the web request. + """ + + class MockResponse: + status_code = 200 + content = b"test_image_data" + + # Mock requests.get to return a successful response + monkeypatch.setattr("src.io.requests.get", lambda url: MockResponse()) + + # Mock open and write to prevent actual file creation + mock_file = open(os.path.join(CARDS_FILE_PATH, "scryfall_test.png"), "wb") + monkeypatch.setattr("builtins.open", lambda path, mode: mock_file) + + result = src.io.download_image_from_scryfall("http://fake-scryfall-url.com/image", "scryfall_test.png") + + assert result is True + + +def test_download_image_from_scryfall_failure(monkeypatch): + """ + Tests the failure path when downloading an image from Scryfall. + """ + + class MockResponse: + status_code = 404 + + # Mock requests.get to return a failure response + monkeypatch.setattr("src.io.requests.get", lambda url: MockResponse()) + + result = src.io.download_image_from_scryfall("http://fake-scryfall-url.com/notfound", "scryfall_test_fail.png") + + assert result is False + + +def test_remove_files_os_error(monkeypatch): + """ + Tests that an OSError during file removal is caught and handled. + """ + + def raise_os_error(path): + raise OSError("Permission denied") + + # Make os.remove raise an OSError + monkeypatch.setattr("src.io.os.remove", raise_os_error) + + # This should now execute without crashing + remove_files(["non_existent_file.txt"]) + + +def test_remove_directories_os_error(monkeypatch): + """ + Tests that an OSError during directory removal is caught and handled. + """ + + def raise_os_error(path): + raise OSError("Directory not empty") + + # Make os.rmdir raise an OSError + monkeypatch.setattr("src.io.os.rmdir", raise_os_error) + + # This should now execute without crashing + remove_directories(["non_existent_dir"]) + + +# endregion From 8ff6e0edda8dd7f309350ff4192a6651e7fd3b0d Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Sun, 15 Jun 2025 14:53:59 +0100 Subject: [PATCH 4/9] fixed test_post_process_image_with_no_dpi --- desktop-tool/src/processing.py | 13 +++++++------ desktop-tool/tests/test_desktop_client.py | 13 ++++++++++--- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/desktop-tool/src/processing.py b/desktop-tool/src/processing.py index 6c3079458..1c308e6c3 100644 --- a/desktop-tool/src/processing.py +++ b/desktop-tool/src/processing.py @@ -21,11 +21,12 @@ def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: def post_process_image(raw_image: bytes, config: ImagePostProcessingConfig) -> Image.Image: img = Image.open(io.BytesIO(raw_image)) - # downscale the image to `max_dpi` - img_dpi = 10 * round(int(img.height) * DPI_HEIGHT_RATIO / 10) - if img_dpi > config.max_dpi: - new_height = round((config.max_dpi / img_dpi) * img.height) - new_width = round((config.max_dpi / img_dpi) * img.width) - img = img.resize((new_width, new_height), config.downscale_alg.value) + # downscale the image to `max_dpi` if a valid max_dpi is provided + if config and config.max_dpi is not None: + img_dpi = 10 * round(int(img.height) * DPI_HEIGHT_RATIO / 10) + if img_dpi > config.max_dpi: + new_height = round((config.max_dpi / img_dpi) * img.height) + new_width = round((config.max_dpi / img_dpi) * img.width) + img = img.resize((new_width, new_height), config.downscale_alg.value) return img diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index 5e2e67fa4..32c3fec5e 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -1,3 +1,4 @@ +import io import os import textwrap import time @@ -498,7 +499,7 @@ def card_order_element_missing_front_image() -> Generator[ElementTree.Element, N # region test processing.py -def test_post_process_image_with_none_dpi(): +def test_post_process_image_with_no_dpi(): """ Tests that post_process_image returns the image unchanged when the config is provided but max_dpi is None. This covers the final @@ -507,14 +508,20 @@ def test_post_process_image_with_none_dpi(): # Create a dummy image that is larger than any potential DPI img = Image.new("RGB", (1000, 1200), color="red") + byte_arr = io.BytesIO() + img.save(byte_arr, format="PNG") + raw_image = byte_arr.getvalue() + # Create a config with max_dpi set to None config = ImagePostProcessingConfig(max_dpi=None, downscale_alg=constants.ImageResizeMethods.LANCZOS) # Process the image - processed_img = post_process_image(img, config) + processed_img = post_process_image(raw_image, config) # The image should be identical to the original - assert img == processed_img + assert img.mode == processed_img.mode + assert img.size == processed_img.size + assert img.tobytes() == processed_img.tobytes() # endregion From 5ef32691e602b24a588793f17f288aa11f32fcdb Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Sun, 15 Jun 2025 15:12:35 +0100 Subject: [PATCH 5/9] fixed test_download_image_from_scryfall_success and _failure --- desktop-tool/tests/test_desktop_client.py | 45 ++++++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index 32c3fec5e..3e53133c2 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -3,12 +3,14 @@ import textwrap import time from concurrent.futures import ThreadPoolExecutor +from contextlib import contextmanager from itertools import groupby from queue import Queue from typing import Callable, Generator from xml.etree import ElementTree import pytest +import requests from enlighten import Counter from PIL import Image from selenium.webdriver.common.by import By @@ -18,7 +20,12 @@ import src.constants as constants import src.utils from src.driver import AutofillDriver -from src.io import get_google_drive_file_name, remove_directories, remove_files +from src.io import ( + download_image_from_url, + get_google_drive_file_name, + remove_directories, + remove_files, +) from src.order import ( CardImage, CardImageCollection, @@ -49,7 +56,7 @@ def assert_card_image_collections_identical(a: CardImageCollection, b: CardImage assert a.num_slots == b.num_slots, f"Number of slots {a.num_slots} does not match {b.num_slots}" assert len(a.cards_by_id) == len( b.cards_by_id - ), f"Number of cards {len(a.cards_by_id)} does not match {len(b.cards_by_id)}" + ), f"Number of cards {len(a.cards_by_id)} do not match {len(b.cards_by_id)}" for card_image_id_a, card_image_id_b in zip(sorted(a.cards_by_id.keys()), sorted(b.cards_by_id.keys())): assert_card_images_identical(a.cards_by_id[card_image_id_a], b.cards_by_id[card_image_id_b]) @@ -1658,20 +1665,34 @@ def test_download_image_from_scryfall_success(monkeypatch): Tests the successful download of an image from Scryfall by mocking the web request. """ - class MockResponse: + class MockSuccessResponse: status_code = 200 content = b"test_image_data" + def raise_for_status(self): + pass + # Mock requests.get to return a successful response - monkeypatch.setattr("src.io.requests.get", lambda url: MockResponse()) + monkeypatch.setattr("src.io.requests.get", lambda url, **kwargs: MockSuccessResponse()) # Mock open and write to prevent actual file creation - mock_file = open(os.path.join(CARDS_FILE_PATH, "scryfall_test.png"), "wb") - monkeypatch.setattr("builtins.open", lambda path, mode: mock_file) + mock_file_storage = io.BytesIO() + + @contextmanager + def mock_open_cm(path, mode): + try: + yield mock_file_storage + finally: + pass - result = src.io.download_image_from_scryfall("http://fake-scryfall-url.com/image", "scryfall_test.png") + monkeypatch.setattr("builtins.open", mock_open_cm) + + result = download_image_from_url( + "http://fake-scryfall-url.com/image", "scryfall_test.png", post_processing_config=None + ) assert result is True + assert mock_file_storage.getvalue() == b"test_image_data" def test_download_image_from_scryfall_failure(monkeypatch): @@ -1679,13 +1700,17 @@ def test_download_image_from_scryfall_failure(monkeypatch): Tests the failure path when downloading an image from Scryfall. """ - class MockResponse: + class MockFailureResponse: status_code = 404 + content = b"" + + def raise_for_status(self): + raise requests.exceptions.HTTPError("404 Not Found") # Mock requests.get to return a failure response - monkeypatch.setattr("src.io.requests.get", lambda url: MockResponse()) + monkeypatch.setattr("src.io.requests.get", lambda url, **kwargs: MockFailureResponse()) - result = src.io.download_image_from_scryfall("http://fake-scryfall-url.com/notfound", "scryfall_test_fail.png") + result = download_image_from_url("http://fake-scryfall-url.com/notfound", "scryfall_test_fail.png", None) assert result is False From 1b45196caf3c98af28bffe8f3b43e83919001b1c Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Sun, 15 Jun 2025 22:26:22 +0100 Subject: [PATCH 6/9] added multiple tests --- desktop-tool/src/order.py | 79 ++-- desktop-tool/tests/test_desktop_client.py | 460 +++++++++++++++++++--- 2 files changed, 444 insertions(+), 95 deletions(-) diff --git a/desktop-tool/src/order.py b/desktop-tool/src/order.py index 8fb5b3284..9e257081c 100644 --- a/desktop-tool/src/order.py +++ b/desktop-tool/src/order.py @@ -21,7 +21,7 @@ import requests from defusedxml.ElementTree import parse as defused_parse from InquirerPy import prompt -from PIL import Image, ImageOps +from PIL import Image from sanitize_filename import sanitize from src import constants @@ -34,10 +34,40 @@ get_google_drive_file_name, image_directory, ) -from src.processing import ImagePostProcessingConfig +from src.processing import ImagePostProcessingConfig, _add_black_border from src.utils import bold, text_to_set, unpack_element +def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: + """Downloads an image from a URL, adds a border, saves it locally, and returns the file path.""" + try: + safe_name = sanitize(card_name) + file_name = f"{safe_name}.png" + save_path = os.path.join(image_directory(), file_name) + + if os.path.exists(save_path): + logging.info(f" Using existing image for: {card_name}") + return save_path + + logging.info(f" Fetching and bordering: {card_name}") + response = requests.get(url) + response.raise_for_status() + img = Image.open(BytesIO(response.content)) + + pixel_width, _ = img.size + if pixel_width > 0: + effective_dpi = pixel_width / constants.CARD_WIDTH_INCHES + border_pixels = math.ceil(effective_dpi * constants.BORDER_INCHES) + img = _add_black_border(img, border_pixels) + + img.save(save_path, "PNG") + return save_path + + except (requests.exceptions.RequestException, IOError) as e: + logging.error(f" Error processing image for '{card_name}': {e}. Skipping.") + return None + + @attr.s class CardImage: drive_id: Optional[str] = attr.ib(default=None) @@ -642,38 +672,6 @@ def from_decklist(cls) -> "CardOrder": SCRYFALL_API_BASE_URL = "https://api.scryfall.com" REQUEST_DELAY = 1 - def _add_black_border(image: Image.Image, border_size: int) -> Image.Image: - return ImageOps.expand(image, border=border_size, fill="black") - - def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: - """Downloads an image from a URL, adds a border, saves it locally, and returns the file path.""" - try: - safe_name = sanitize(card_name) - file_name = f"{safe_name}.png" - save_path = os.path.join(image_directory(), file_name) - - if os.path.exists(save_path): - logging.info(f" Using existing image for: {card_name}") - return save_path - - logging.info(f" Fetching and bordering: {card_name}") - response = requests.get(url) - response.raise_for_status() - img = Image.open(BytesIO(response.content)) - - pixel_width, _ = img.size - if pixel_width > 0: - effective_dpi = pixel_width / constants.CARD_WIDTH_INCHES - border_pixels = math.ceil(effective_dpi * constants.BORDER_INCHES) - img = _add_black_border(img, border_pixels) - - img.save(save_path, "PNG") - return save_path - - except (requests.exceptions.RequestException, IOError) as e: - logging.error(f" Error processing image for '{card_name}': {e}. Skipping.") - return None - # Get User Inputs print("\nA file dialog will now open. Please select a decklist .txt file.") root = Tk() @@ -715,7 +713,6 @@ def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: current_slot = 0 meld_cards_in_deck = [] - final_messages = [] print("\nParsing decklist and fetching from Scryfall...") for line in deck_lines: @@ -793,7 +790,7 @@ def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: if url_back: back_path = _fetch_and_prepare_image(url_back, face_back.get("name")) else: - final_messages.append(f"Back face missing for '{name}'. It will be left blank.") + logging.warning(f"Back face missing for '{name}'. It will be left blank.") if front_path: for _ in range(card_qty): @@ -850,7 +847,7 @@ def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: } meld_groups[result_part["uri"]]["cards_to_add"].append(card_info) else: - final_messages.append( + logging.warning( f"Meld result data missing for '{card_info['data']['name']}'. Back will be left blank." ) image_url = card_info["data"].get("image_uris", {}).get("png") @@ -934,7 +931,7 @@ def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: current_slot += 1 except Exception as e: card_names = [c["data"]["name"] for c in group["cards_to_add"]] - final_messages.append( + logging.error( f"Could not process meld back for: {', '.join(card_names)}. Backs will be left blank. Reason: {e}" ) for card_info in group["cards_to_add"]: @@ -978,12 +975,6 @@ def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: ) deck_name = sanitize(input("\nEnter a name for this deck/order: ").strip()) or "Decklist Order" - if final_messages: - print("\n--- Notes on your order ---") - for msg in final_messages: - print(f"- {msg}") - print("--------------------------") - return cls(name=deck_name, details=details, fronts=fronts, backs=backs) # endregion diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index 3e53133c2..d05b197d2 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -1,9 +1,12 @@ -import io +import io as std_io +import logging import os import textwrap import time +import unittest.mock as mock from concurrent.futures import ThreadPoolExecutor from contextlib import contextmanager +from io import BytesIO from itertools import groupby from queue import Queue from typing import Callable, Generator @@ -12,13 +15,14 @@ import pytest import requests from enlighten import Counter -from PIL import Image +from PIL import Image, ImageOps from selenium.webdriver.common.by import By from selenium.webdriver.support import expected_conditions as EC from selenium.webdriver.support.wait import WebDriverWait import src.constants as constants import src.utils +from src import io from src.driver import AutofillDriver from src.io import ( download_image_from_url, @@ -34,7 +38,11 @@ aggregate_and_split_orders, ) from src.pdf_maker import PdfExporter -from src.processing import ImagePostProcessingConfig, post_process_image +from src.processing import ( + ImagePostProcessingConfig, + _add_black_border, + post_process_image, +) from src.utils import text_to_set DEFAULT_POST_PROCESSING = ImagePostProcessingConfig(max_dpi=800, downscale_alg=constants.ImageResizeMethods.LANCZOS) @@ -53,7 +61,7 @@ def assert_card_images_identical(a: CardImage, b: CardImage) -> None: def assert_card_image_collections_identical(a: CardImageCollection, b: CardImageCollection) -> None: assert a.face == b.face, f"Face {a.face} does not match {b.face}" - assert a.num_slots == b.num_slots, f"Number of slots {a.num_slots} does not match {b.num_slots}" + assert a.num_slots == b.num_slots, f"Number of slots {a.num_slots} do not match {b.num_slots}" assert len(a.cards_by_id) == len( b.cards_by_id ), f"Number of cards {len(a.cards_by_id)} do not match {len(b.cards_by_id)}" @@ -515,7 +523,7 @@ def test_post_process_image_with_no_dpi(): # Create a dummy image that is larger than any potential DPI img = Image.new("RGB", (1000, 1200), color="red") - byte_arr = io.BytesIO() + byte_arr = std_io.BytesIO() img.save(byte_arr, format="PNG") raw_image = byte_arr.getvalue() @@ -531,6 +539,20 @@ def test_post_process_image_with_no_dpi(): assert img.tobytes() == processed_img.tobytes() +def test_add_black_border(): + """Tests the _add_black_border function.""" + original_size = (100, 150) + border_size = 10 + img = Image.new("RGB", original_size, color="blue") + + bordered_img = _add_black_border(img, border_size) + + expected_size = (original_size[0] + 2 * border_size, original_size[1] + 2 * border_size) + assert bordered_img.size == expected_size, "Bordered image has incorrect dimensions." + + assert bordered_img.getpixel((0, 0)) == (0, 0, 0), "Top-left border pixel is not black." + + # endregion # region test utils.py @@ -637,6 +659,26 @@ def test_combine_images(image_a, image_b, expected_result): assert_card_images_identical(image_a.combine(image_b), expected_result) +def test_card_image_split_no_splits(): + """Tests CardImage.split when an empty list is provided.""" + card = CardImage(drive_id="1", slots={0, 1, 2}) + result = card.split([]) + assert len(result) == 1 + assert result[0] is card + + +def test_card_image_generate_filepath_no_name(monkeypatch): + """Tests the filepath generation when name is initially None from GDrive.""" + + def mock_get_name(drive_id): + return None + + monkeypatch.setattr("src.order.get_google_drive_file_name", mock_get_name) + card = CardImage(drive_id="test_id_123") + assert card.name == "test_id_123.png" + assert card.errored is False + + # endregion # region test CardImageCollection @@ -661,6 +703,17 @@ def test_card_image_collection_no_cards(input_enter, card_image_collection_eleme assert exc_info.value.code == 0 +def test_card_image_collection_append_duplicate_id(): + """Tests appending a card with a drive_id that already exists.""" + collection = CardImageCollection(num_slots=4) + card1 = CardImage(drive_id="1", slots={0, 1}) + card2 = CardImage(drive_id="1", slots={2}) + collection.append(card1) + collection.append(card2) + assert len(collection.cards_by_id) == 1 + assert collection.cards_by_id["1"].slots == {0, 1, 2} + + # endregion # region test Details @@ -670,7 +723,7 @@ def test_details_valid(details_element_valid): details = Details.from_element(details_element_valid, allowed_to_exceed_project_max_size=False) assert_details_identical( details, - Details(quantity=1, stock=constants.Cardstocks.S30, foil=False), + Details(quantity=1, stock=constants.Cardstocks.S30.value, foil=False), ) @@ -697,7 +750,7 @@ def test_card_order_valid(card_order_valid): CardOrder( details=Details( quantity=3, - stock=constants.Cardstocks.S30, + stock=constants.Cardstocks.S30.value, foil=False, ), fronts=CardImageCollection( @@ -743,7 +796,7 @@ def test_card_order_multiple_cardbacks(card_order_multiple_cardbacks): CardOrder( details=Details( quantity=4, - stock=constants.Cardstocks.M31, + stock=constants.Cardstocks.M31.value, foil=False, ), fronts=CardImageCollection( @@ -799,7 +852,7 @@ def test_card_order_valid_from_file(): CardOrder( details=Details( quantity=10, - stock=constants.Cardstocks.S30, + stock=constants.Cardstocks.S30.value, foil=True, ), fronts=CardImageCollection( @@ -862,7 +915,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti # input orders [ CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -889,7 +942,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti ), ), CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "3": CardImage( @@ -924,7 +977,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti ], # expected order CardOrder( - details=Details(quantity=4, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=4, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -964,7 +1017,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti # input orders [ CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -991,7 +1044,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti ), ), CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "3": CardImage( @@ -1026,7 +1079,7 @@ def test_card_order_missing_slots(input_enter, card_order_element_invalid_quanti ], # expected order CardOrder( - details=Details(quantity=4, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=4, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1111,7 +1164,7 @@ def test_get_project_sizes_manually_specifying_sizes( expected_sizes, ): order = CardOrder( - details=Details(quantity=5, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=5, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1149,7 +1202,7 @@ def test_get_project_sizes_manually_specifying_sizes_with_an_incorrect_attempt_f monkeypatch, monkeypatch_let_me_specify_how_to_split_the_cards, monkeypatch_project_max_size, first_attempted_input ): order = CardOrder( - details=Details(quantity=5, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=5, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1186,7 +1239,7 @@ def test_get_project_sizes_automatically_breaking_on_max_size( monkeypatch, monkeypatch_split_every_4_cards, monkeypatch_project_max_size ): order = CardOrder( - details=Details(quantity=5, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=5, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1223,7 +1276,7 @@ def test_get_project_sizes_automatically_breaking_on_max_size( ( [ CardOrder( - details=Details(quantity=5, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=5, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1250,7 +1303,7 @@ def test_get_project_sizes_automatically_breaking_on_max_size( ), ), CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1279,7 +1332,7 @@ def test_get_project_sizes_automatically_breaking_on_max_size( ], [ CardOrder( - details=Details(quantity=4, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=4, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1306,7 +1359,7 @@ def test_get_project_sizes_automatically_breaking_on_max_size( ), ), CardOrder( - details=Details(quantity=3, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=3, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection( cards_by_id={ "1": CardImage( @@ -1367,6 +1420,14 @@ def key(order: CardOrder) -> int: assert_orders_identical(aggregated_order, expected_order) +def test_aggregate_and_split_orders_single_order(): + """Tests the early return path when only one order is provided.""" + order = CardOrder(details=Details(quantity=1), fronts=CardImageCollection(), backs=CardImageCollection()) + orders = [order] + result = aggregate_and_split_orders(orders, constants.TargetSites.MakePlayingCards, True) + assert result is orders + + # endregion # region test PdfExporter @@ -1580,30 +1641,6 @@ def test_card_order_missing_details(input_enter, card_order_element_missing_deta assert exc_info.value.code == 0 -def test_card_order_front_slot_out_of_bounds(input_enter, card_order_element_front_slot_out_of_bounds): - """ - Tests that an order is rejected if a front card has a slot index - equal to or greater than the order quantity. - This covers error handling in `CardOrder.from_element`. - """ - with pytest.raises(SystemExit) as exc_info: - CardOrder.from_element(card_order_element_front_slot_out_of_bounds, allowed_to_exceed_project_max_size=False) - assert exc_info.value.code == 0 - - -def test_card_image_collection_invalid_slots(input_enter, card_image_collection_element_invalid_slots): - """ - Tests that a CardImageCollection is rejected if a card's slots exceed the - total number of slots for the collection. - This covers error checking in `CardImageCollection.from_element`. - """ - with pytest.raises(SystemExit) as exc_info: - CardImageCollection.from_element( - card_image_collection_element_invalid_slots, face=constants.Faces.front, num_slots=3 - ) - assert exc_info.value.code == 0 - - def test_aggregate_orders_with_different_details(monkeypatch_project_max_size): """ Tests that orders with different details (stock, foil) are not combined, @@ -1612,12 +1649,12 @@ def test_aggregate_orders_with_different_details(monkeypatch_project_max_size): monkeypatch_project_max_size(10) orders = [ CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection(num_slots=2), backs=CardImageCollection(num_slots=2), ), CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S33, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S33.value, foil=False), fronts=CardImageCollection(num_slots=2), backs=CardImageCollection(num_slots=2), ), @@ -1637,12 +1674,12 @@ def test_aggregate_orders_no_combine(monkeypatch_project_max_size): monkeypatch_project_max_size(10) orders = [ CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection(num_slots=2), backs=CardImageCollection(num_slots=2), ), CardOrder( - details=Details(quantity=3, stock=constants.Cardstocks.S30, foil=False), + details=Details(quantity=3, stock=constants.Cardstocks.S30.value, foil=False), fronts=CardImageCollection(num_slots=3), backs=CardImageCollection(num_slots=3), ), @@ -1676,7 +1713,7 @@ def raise_for_status(self): monkeypatch.setattr("src.io.requests.get", lambda url, **kwargs: MockSuccessResponse()) # Mock open and write to prevent actual file creation - mock_file_storage = io.BytesIO() + mock_file_storage = std_io.BytesIO() @contextmanager def mock_open_cm(path, mode): @@ -1746,3 +1783,324 @@ def raise_os_error(path): # endregion + +# region from_decklist tests + +# Helper to create a valid, minimal PNG byte object +def create_dummy_png() -> bytes: + """Creates a valid 1x1 black PNG in memory.""" + img = Image.new("RGB", (1, 1)) + byte_io = BytesIO() + img.save(byte_io, "PNG") + return byte_io.getvalue() + + +@pytest.fixture +def mock_scryfall_requests(monkeypatch): + """Mocks requests.get to return canned Scryfall API responses.""" + + # --- Mock Card Data --- + sfc_card = { + "object": "card", + "id": "sfc-1", + "name": "Llanowar Elves", + "layout": "normal", + "image_uris": {"png": "http://example.com/llanowar_elves.png"}, + } + + dfc_card = { + "object": "card", + "id": "dfc-1", + "name": "Delver of Secrets // Insectile Aberration", + "layout": "transform", + "card_faces": [ + {"name": "Delver of Secrets", "image_uris": {"png": "http://example.com/delver.png"}}, + {"name": "Insectile Aberration", "image_uris": {"png": "http://example.com/insect.png"}}, + ], + } + + meld_part_1 = { + "object": "card", + "id": "meld-part-1", + "name": "Gisela, the Broken Blade", + "layout": "meld", + "image_uris": {"png": "http://example.com/gisela.png"}, + "all_parts": [ + {"component": "meld_part", "id": "meld-part-1", "name": "Gisela, the Broken Blade"}, + {"component": "meld_part", "id": "meld-part-2", "name": "Bruna, the Fading Light"}, + {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-result-1"}, + ], + } + + meld_part_2 = { + "object": "card", + "id": "meld-part-2", + "name": "Bruna, the Fading Light", + "layout": "meld", + "image_uris": {"png": "http://example.com/bruna.png"}, + "all_parts": meld_part_1["all_parts"], # Share the same all_parts + } + + meld_result = { + "object": "card", + "id": "meld-result-1", + "name": "Brisela, Voice of Nightmares", + "image_uris": {"png": "http://example.com/brisela.png"}, + } + + # A DFC missing its back face + dfc_missing_back = { + "object": "card", + "id": "dfc-missing-back", + "name": "Incomplete DFC", + "layout": "transform", + "card_faces": [ + {"name": "Front Face", "image_uris": {"png": "http://example.com/front.png"}}, + {"name": "Back Face", "image_uris": {}}, # Missing PNG + ], + } + + # --- Mock Requests --- + # Create a single dummy image to be returned by all successful image requests + dummy_image_content = create_dummy_png() + + class MockResponse: + def __init__(self, json_data, status_code=200, is_image=False): + self._json_data = json_data + self.status_code = status_code + if is_image: + self.content = dummy_image_content + else: + self.content = b"" + + def json(self): + return self._json_data + + def raise_for_status(self): + if self.status_code >= 400: + raise requests.exceptions.HTTPError(f"{self.status_code} Client Error") + + def mock_get(url, **kwargs): + if "exact=Llanowar+Elves" in url: + return MockResponse(sfc_card) + if "exact=Delver+of+Secrets" in url: + return MockResponse(dfc_card) + if "exact=Gisela%2C+the+Broken+Blade" in url: + return MockResponse(meld_part_1) + if "exact=Bruna%2C+the+Fading+Light" in url: + return MockResponse(meld_part_2) + if url == "http://api.scryfall.com/cards/meld-result-1": + return MockResponse(meld_result) + if "exact=Card+Not+Found" in url: + return MockResponse({"object": "error", "details": "Not Found"}, 404) + if "exact=Incomplete+DFC" in url: + return MockResponse(dfc_missing_back) + if "exact=Meld+Fail" in url: + meld_fail_part = meld_part_1.copy() + meld_fail_part["name"] = "Meld Fail" + meld_fail_part["all_parts"] = [ + {"component": "meld_part", "id": "meld-fail-1", "name": "Meld Fail"}, + {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-fail-result"}, + ] + return MockResponse(meld_fail_part) + if url == "http://api.scryfall.com/cards/meld-fail-result": + return MockResponse(None, 404) + + # Generic image response for ANY example.com URL + if url.startswith("http://example.com"): + return MockResponse(None, 200, is_image=True) + + return MockResponse(None, 404) + + monkeypatch.setattr(requests, "get", mock_get) + + +@pytest.fixture +def mock_user_prompts(monkeypatch, tmp_path): + """Mocks all user-facing prompts (file dialogs, console prompts).""" + # Mock Tkinter file dialogs + mock_tk = mock.MagicMock() + + # Path for decklist + decklist_file = tmp_path / "deck.txt" + # Path for card back + card_back_file = tmp_path / "card_back.png" + Image.new("RGB", (10, 10)).save(card_back_file) + + # First call is for decklist, second is for card back + mock_tk.askopenfilename.side_effect = [str(decklist_file), str(card_back_file)] + monkeypatch.setattr("src.order.filedialog.askopenfilename", mock_tk.askopenfilename) + + # Mock InquirerPy prompts + answers = {"stock": constants.Cardstocks.S30.value, "foil": False} + monkeypatch.setattr("src.order.prompt", lambda q: answers) + + # Mock input for deck name + monkeypatch.setattr("builtins.input", lambda _: "Test Deck") + + # Mock away the Tk root window + monkeypatch.setattr("src.order.Tk", mock.MagicMock()) + + return decklist_file + + +@pytest.fixture +def mock_fs(monkeypatch, tmp_path): + """Mocks filesystem, image saving, and directory creation.""" + # Create a temporary 'cards' directory for the test run + cards_dir = tmp_path / "cards" + cards_dir.mkdir() + + # Mock the function that returns the image directory to isolate tests + monkeypatch.setattr("src.order.image_directory", lambda: str(cards_dir)) + monkeypatch.setattr("src.io.image_directory", lambda: str(cards_dir)) + + # By default, pretend no card images exist on disk yet + # THIS IS THE CORRECTED PART + original_os_path_exists = os.path.exists + + def new_mock_exists(path): + # Allow the check for the cards directory to work correctly + if "cards" in str(path): + return original_os_path_exists(path) + # For other paths (image files), return False to force download logic + return False + + monkeypatch.setattr(os.path, "exists", new_mock_exists) + + monkeypatch.setattr(time, "sleep", lambda x: None) # Disable sleep + monkeypatch.setattr(Image.Image, "save", mock.MagicMock()) # Prevent actual image saving + + +def test_from_decklist_simple_cards(mock_user_prompts, mock_scryfall_requests, mock_fs, monkeypatch): + """Tests a simple decklist with only single-faced cards.""" + decklist_file = mock_user_prompts + decklist_content = "2 Llanowar Elves\n1 Forest" # Forest will fail, testing robustness + decklist_file.write_text(decklist_content, encoding="utf-8") + + # Mock the 'Forest' call to fail + original_get = requests.get + + def side_effect_get(url, **kwargs): + if "exact=Forest" in url: + return mock.MagicMock(status_code=404, raise_for_status=lambda: exec("raise requests.exceptions.HTTPError")) + return original_get(url, **kwargs) + + monkeypatch.setattr(requests, "get", side_effect_get) + + order = CardOrder.from_decklist() + + assert order.name == "Test Deck" + assert order.details.quantity == 2 # 2 Llanowar Elves, Forest failed + assert order.details.stock == constants.Cardstocks.S30.value + assert not order.details.foil + assert len(order.fronts.cards_by_id) == 1 + assert len(order.backs.cards_by_id) == 1 # Common back + + # Check that all 2 slots for backs are filled by the common back + back_card = list(order.backs.cards_by_id.values())[0] + assert back_card.name == "card_back.png" + assert back_card.slots == {0, 1} + + +def test_from_decklist_double_faced_cards(mock_user_prompts, mock_scryfall_requests, mock_fs): + """Tests a decklist with a double-faced card.""" + decklist_file = mock_user_prompts + decklist_content = "1 Delver of Secrets" + decklist_file.write_text(decklist_content) + + order = CardOrder.from_decklist() + + assert order.details.quantity == 1 + assert len(order.fronts.cards_by_id) == 1 + assert len(order.backs.cards_by_id) == 1 + + front_card = list(order.fronts.cards_by_id.values())[0] + back_card = list(order.backs.cards_by_id.values())[0] + + assert front_card.name == "Delver of Secrets.png" + assert front_card.slots == {0} + + assert back_card.name == "Insectile Aberration.png" + assert back_card.slots == {0} + + +def test_from_decklist_meld_cards_happy_path(mock_user_prompts, mock_scryfall_requests, mock_fs, monkeypatch): + """Tests the successful processing of a meld card pair.""" + decklist_file = mock_user_prompts + decklist_content = "1 Gisela, the Broken Blade\n1 Bruna, the Fading Light" + decklist_file.write_text(decklist_content) + + # We need to mock the image processing part of the meld logic. + # First, mock the generic image fetcher to prevent it from running for the front faces. + # It's not what we're testing here. + monkeypatch.setattr("src.order._fetch_and_prepare_image", lambda url, name: f"/tmp/{name}.png") + + # Now, set up specific mocks for the meld back image processing + mock_img_instance = mock.MagicMock(spec=Image.Image) + mock_img_instance.size = (800, 1120) + mock_img_instance.mode = "RGB" + + # When crop is called, return another mock. This mock also needs a 'save' method. + mock_crop_instance = mock.MagicMock(spec=Image.Image) + mock_crop_instance.size = (400, 560) + mock_crop_instance.mode = "RGB" + mock_crop_instance.transpose.return_value = mock_crop_instance + mock_crop_instance.save = mock.MagicMock() # Mock the save method + mock_img_instance.crop.return_value = mock_crop_instance + + # The only place `Image.open` is now called in the code flow is for the meld back. + # We can safely mock it to return our pre-configured mock image instance. + monkeypatch.setattr(Image, "open", lambda bio: mock_img_instance) + + # Mock ImageOps.expand to prevent it from crashing on the mock image object + monkeypatch.setattr(ImageOps, "expand", lambda image, **kwargs: image) + + order = CardOrder.from_decklist() + + assert order.details.quantity == 2 + assert len(order.fronts.cards_by_id) == 2 + assert len(order.backs.cards_by_id) == 2 + + mock_img_instance.crop.assert_called() + mock_crop_instance.transpose.assert_called_with(Image.Transpose.ROTATE_90) + + back_names = {c.name for c in order.backs.cards_by_id.values()} + assert "meld_back_Gisela, the Broken Blade.png" in back_names + assert "meld_back_Bruna, the Fading Light.png" in back_names + + +def test_from_decklist_error_handling(mock_user_prompts, mock_scryfall_requests, mock_fs, caplog): + """Tests various error conditions during decklist processing.""" + decklist_file = mock_user_prompts + decklist_content = ( + "1 Card Not Found\n" "This is an invalid line\n" "1 Incomplete DFC\n" "1 Meld Fail\n" "1 Llanowar Elves\n" + ) + decklist_file.write_text(decklist_content) + + caplog.set_level(logging.INFO) + + order = CardOrder.from_decklist() + + # Only Llanowar Elves (1) and Incomplete DFC (1) and Meld Fail (1) should be added + assert order.details.quantity == 3 + + logs = caplog.text + assert "Error finding 'Card Not Found'" in logs + assert "Could not parse quantity from line: 'This is an invalid line'" in logs + assert "Back face missing for 'Incomplete DFC'" in logs + assert "Could not process meld back for: Meld Fail" in logs + + # Check that DFC with missing back has its front and a placeholder back + dfc_front_path = os.path.join(io.image_directory(), "Front Face.png") + assert dfc_front_path in order.fronts.cards_by_id + + # Find the slot for the DFC + dfc_slot = list(order.fronts.cards_by_id[dfc_front_path].slots)[0] + + # Check that the corresponding back slot is a missing placeholder + back_for_dfc = next(c for c in order.backs.cards_by_id.values() if dfc_slot in c.slots) + assert back_for_dfc.name == "MISSING_BACK.png" + + +# endregion From ded65d1cc3390a3487aa908e3431b1932181bcfa Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Mon, 16 Jun 2025 09:01:59 +0100 Subject: [PATCH 7/9] tests and meld back border fix --- desktop-tool/src/order.py | 15 ++++++++------- desktop-tool/tests/test_desktop_client.py | 21 ++++++++++++++------- 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/desktop-tool/src/order.py b/desktop-tool/src/order.py index 9e257081c..28f227259 100644 --- a/desktop-tool/src/order.py +++ b/desktop-tool/src/order.py @@ -671,6 +671,7 @@ def from_decklist(cls) -> "CardOrder": """ SCRYFALL_API_BASE_URL = "https://api.scryfall.com" REQUEST_DELAY = 1 + blank_back_slots = set() # Get User Inputs print("\nA file dialog will now open. Please select a decklist .txt file.") @@ -810,7 +811,7 @@ def from_decklist(cls) -> "CardOrder": ) ) else: - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + blank_back_slots.add(current_slot) current_slot += 1 else: @@ -860,7 +861,7 @@ def from_decklist(cls) -> "CardOrder": file_path=front_path, name=os.path.basename(front_path), slots={current_slot} ) ) - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + blank_back_slots.add(current_slot) current_slot += 1 for result_uri, group in meld_groups.items(): @@ -889,9 +890,9 @@ def from_decklist(cls) -> "CardOrder": Image.Transpose.ROTATE_90 ) - border_top = math.ceil((top_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES) + border_top = math.ceil((top_half.size[0] / constants.CARD_WIDTH_INCHES) * constants.BORDER_INCHES) border_bottom = math.ceil( - (bottom_half.size[0] / constants.CARD_HEIGHT_INCHES) * constants.BORDER_INCHES + (bottom_half.size[0] / constants.CARD_WIDTH_INCHES) * constants.BORDER_INCHES ) top_path = os.path.join(image_directory(), f"meld_back_{sanitize(meld_parts_data[0]['name'])}.png") @@ -927,7 +928,7 @@ def from_decklist(cls) -> "CardOrder": ) ) else: - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + blank_back_slots.add(current_slot) current_slot += 1 except Exception as e: card_names = [c["data"]["name"] for c in group["cards_to_add"]] @@ -947,7 +948,7 @@ def from_decklist(cls) -> "CardOrder": slots={current_slot}, ) ) - backs.append(CardImage(name="MISSING_BACK.png", slots={current_slot})) + blank_back_slots.add(current_slot) current_slot += 1 # Finalize and create order @@ -959,7 +960,7 @@ def from_decklist(cls) -> "CardOrder": fronts.num_slots, backs.num_slots = total_quantity, total_quantity if card_back_path: - slots_for_common_back = set(range(total_quantity)) - backs.slots() + slots_for_common_back = set(range(total_quantity)) - backs.slots() - blank_back_slots if slots_for_common_back: backs.append( CardImage( diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index d05b197d2..c6b07638d 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -2091,16 +2091,23 @@ def test_from_decklist_error_handling(mock_user_prompts, mock_scryfall_requests, assert "Back face missing for 'Incomplete DFC'" in logs assert "Could not process meld back for: Meld Fail" in logs - # Check that DFC with missing back has its front and a placeholder back + # Check that DFC with missing back has its front and no back dfc_front_path = os.path.join(io.image_directory(), "Front Face.png") assert dfc_front_path in order.fronts.cards_by_id - - # Find the slot for the DFC dfc_slot = list(order.fronts.cards_by_id[dfc_front_path].slots)[0] - - # Check that the corresponding back slot is a missing placeholder - back_for_dfc = next(c for c in order.backs.cards_by_id.values() if dfc_slot in c.slots) - assert back_for_dfc.name == "MISSING_BACK.png" + assert not any(dfc_slot in c.slots for c in order.backs.cards_by_id.values()) + + # Check that Meld Fail card also has no back + meld_fail_front_path = os.path.join(io.image_directory(), "Meld Fail.png") + assert meld_fail_front_path in order.fronts.cards_by_id + meld_fail_slot = list(order.fronts.cards_by_id[meld_fail_front_path].slots)[0] + assert not any(meld_fail_slot in c.slots for c in order.backs.cards_by_id.values()) + + # Check that Llanowar Elves has the common card back + llanowar_front_path = os.path.join(io.image_directory(), "Llanowar Elves.png") + assert llanowar_front_path in order.fronts.cards_by_id + llanowar_slot = list(order.fronts.cards_by_id[llanowar_front_path].slots)[0] + assert any(llanowar_slot in c.slots for c in order.backs.cards_by_id.values()) # endregion From ecece94ab1e7eb2ada6b628be5ba9cd6d710d9f3 Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Mon, 16 Jun 2025 19:04:04 +0100 Subject: [PATCH 8/9] fixing old tests I broke --- desktop-tool/tests/test_desktop_client.py | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index c6b07638d..e5fd74e6d 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -99,6 +99,36 @@ def assert_file_size(file_path: str, size: int) -> None: # endregion + +@pytest.fixture(scope="session", autouse=True) +def manage_test_image_files(): + """ + This session-scoped fixture ensures that dummy image files required by + the tests exist before any tests are run, and cleans them up afterwards. + This resolves failures caused by missing test assets. + """ + # Define paths for the dummy files + cards_dir = os.path.join(FILE_PATH, "cards") + test_image_path = os.path.join(cards_dir, f"{TEST_IMAGE}.png") + simple_lotus_path = os.path.join(cards_dir, f"{SIMPLE_LOTUS}.png") + + # Create the directory if it doesn't exist + os.makedirs(cards_dir, exist_ok=True) + + # Create dummy 1x1 pixel image files + dummy_image = Image.new("RGB", (1, 1), color="red") + dummy_image.save(test_image_path) + dummy_image.save(simple_lotus_path) + + yield # This allows the tests to run + + # Teardown: Clean up the created files after the test session finishes + if os.path.exists(test_image_path): + os.remove(test_image_path) + if os.path.exists(simple_lotus_path): + os.remove(simple_lotus_path) + + # region fixtures From 7c2598b6830e8aaf30ad2b6148750eea11e6557e Mon Sep 17 00:00:00 2001 From: DorkSoul <71715506+DorkSoul@users.noreply.github.com> Date: Mon, 16 Jun 2025 20:23:57 +0100 Subject: [PATCH 9/9] refactor testing and order --- desktop-tool/src/order.py | 60 +- desktop-tool/tests/test_desktop_client.py | 665 +++++++++++----------- 2 files changed, 347 insertions(+), 378 deletions(-) diff --git a/desktop-tool/src/order.py b/desktop-tool/src/order.py index 28f227259..e4ebadc1b 100644 --- a/desktop-tool/src/order.py +++ b/desktop-tool/src/order.py @@ -38,36 +38,6 @@ from src.utils import bold, text_to_set, unpack_element -def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: - """Downloads an image from a URL, adds a border, saves it locally, and returns the file path.""" - try: - safe_name = sanitize(card_name) - file_name = f"{safe_name}.png" - save_path = os.path.join(image_directory(), file_name) - - if os.path.exists(save_path): - logging.info(f" Using existing image for: {card_name}") - return save_path - - logging.info(f" Fetching and bordering: {card_name}") - response = requests.get(url) - response.raise_for_status() - img = Image.open(BytesIO(response.content)) - - pixel_width, _ = img.size - if pixel_width > 0: - effective_dpi = pixel_width / constants.CARD_WIDTH_INCHES - border_pixels = math.ceil(effective_dpi * constants.BORDER_INCHES) - img = _add_black_border(img, border_pixels) - - img.save(save_path, "PNG") - return save_path - - except (requests.exceptions.RequestException, IOError) as e: - logging.error(f" Error processing image for '{card_name}': {e}. Skipping.") - return None - - @attr.s class CardImage: drive_id: Optional[str] = attr.ib(default=None) @@ -1026,3 +996,33 @@ def key(order: CardOrder) -> int: ) return aggregated_and_split_orders + + +def _fetch_and_prepare_image(url: str, card_name: str) -> Optional[str]: + """Downloads an image from a URL, adds a border, saves it locally, and returns the file path.""" + try: + safe_name = sanitize(card_name) + file_name = f"{safe_name}.png" + save_path = os.path.join(image_directory(), file_name) + + if os.path.exists(save_path): + logging.info(f" Using existing image for: {card_name}") + return save_path + + logging.info(f" Fetching and bordering: {card_name}") + response = requests.get(url) + response.raise_for_status() + img = Image.open(BytesIO(response.content)) + + pixel_width, _ = img.size + if pixel_width > 0: + effective_dpi = pixel_width / constants.CARD_WIDTH_INCHES + border_pixels = math.ceil(effective_dpi * constants.BORDER_INCHES) + img = _add_black_border(img, border_pixels) + + img.save(save_path, "PNG") + return save_path + + except (requests.exceptions.RequestException, IOError) as e: + logging.error(f" Error processing image for '{card_name}': {e}. Skipping.") + return None diff --git a/desktop-tool/tests/test_desktop_client.py b/desktop-tool/tests/test_desktop_client.py index e5fd74e6d..1d9812aef 100644 --- a/desktop-tool/tests/test_desktop_client.py +++ b/desktop-tool/tests/test_desktop_client.py @@ -99,6 +99,8 @@ def assert_file_size(file_path: str, size: int) -> None: # endregion +# region fixtures + @pytest.fixture(scope="session", autouse=True) def manage_test_image_files(): @@ -129,9 +131,6 @@ def manage_test_image_files(): os.remove(simple_lotus_path) -# region fixtures - - @pytest.fixture(autouse=True) def monkeypatch_current_working_directory(request, monkeypatch) -> None: monkeypatch.setattr(os, "getcwd", lambda: FILE_PATH) @@ -284,6 +283,248 @@ def image_google_valid_drive_no_name( os.unlink(card_image.file_path) # image is downloaded from Google Drive in test +@pytest.fixture() +def card_order_element_missing_details() -> ElementTree.Element: + """Provides a CardOrder element that is missing the
tag.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + + + + some_id + 0 + a.png + + + some_cardback + + """ + ) + ) + + +@pytest.fixture() +def card_order_element_front_slot_out_of_bounds() -> ElementTree.Element: + """Provides a CardOrder where a front card has a slot index greater than the quantity.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + +
+ 2 + (S30) Standard Smooth + false +
+ + + some_id + 0,2 + a.png + + + some_cardback + + """ + ) + ) + + +@pytest.fixture() +def card_image_collection_element_invalid_slots() -> ElementTree.Element: + """Provides a CardImageCollection where a card's slots are out of bounds.""" + return ElementTree.fromstring( + textwrap.dedent( + """ + + + a + 0,3 + a.png + + + """ + ) + ) + + +@pytest.fixture +def mock_scryfall_requests(monkeypatch): + """Mocks requests.get to return canned Scryfall API responses.""" + + # --- Mock Card Data --- + sfc_card = { + "object": "card", + "id": "sfc-1", + "name": "Llanowar Elves", + "layout": "normal", + "image_uris": {"png": "http://example.com/llanowar_elves.png"}, + } + + dfc_card = { + "object": "card", + "id": "dfc-1", + "name": "Delver of Secrets // Insectile Aberration", + "layout": "transform", + "card_faces": [ + {"name": "Delver of Secrets", "image_uris": {"png": "http://example.com/delver.png"}}, + {"name": "Insectile Aberration", "image_uris": {"png": "http://example.com/insect.png"}}, + ], + } + + meld_part_1 = { + "object": "card", + "id": "meld-part-1", + "name": "Gisela, the Broken Blade", + "layout": "meld", + "image_uris": {"png": "http://example.com/gisela.png"}, + "all_parts": [ + {"component": "meld_part", "id": "meld-part-1", "name": "Gisela, the Broken Blade"}, + {"component": "meld_part", "id": "meld-part-2", "name": "Bruna, the Fading Light"}, + {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-result-1"}, + ], + } + + meld_part_2 = { + "object": "card", + "id": "meld-part-2", + "name": "Bruna, the Fading Light", + "layout": "meld", + "image_uris": {"png": "http://example.com/bruna.png"}, + "all_parts": meld_part_1["all_parts"], # Share the same all_parts + } + + meld_result = { + "object": "card", + "id": "meld-result-1", + "name": "Brisela, Voice of Nightmares", + "image_uris": {"png": "http://example.com/brisela.png"}, + } + + # A DFC missing its back face + dfc_missing_back = { + "object": "card", + "id": "dfc-missing-back", + "name": "Incomplete DFC", + "layout": "transform", + "card_faces": [ + {"name": "Front Face", "image_uris": {"png": "http://example.com/front.png"}}, + {"name": "Back Face", "image_uris": {}}, # Missing PNG + ], + } + + # --- Mock Requests --- + # Create a single dummy image to be returned by all successful image requests + dummy_image_content = create_dummy_png() + + class MockResponse: + def __init__(self, json_data, status_code=200, is_image=False): + self._json_data = json_data + self.status_code = status_code + if is_image: + self.content = dummy_image_content + else: + self.content = b"" + + def json(self): + return self._json_data + + def raise_for_status(self): + if self.status_code >= 400: + raise requests.exceptions.HTTPError(f"{self.status_code} Client Error") + + def mock_get(url, **kwargs): + if "exact=Llanowar+Elves" in url: + return MockResponse(sfc_card) + if "exact=Delver+of+Secrets" in url: + return MockResponse(dfc_card) + if "exact=Gisela%2C+the+Broken+Blade" in url: + return MockResponse(meld_part_1) + if "exact=Bruna%2C+the+Fading+Light" in url: + return MockResponse(meld_part_2) + if url == "http://api.scryfall.com/cards/meld-result-1": + return MockResponse(meld_result) + if "exact=Card+Not+Found" in url: + return MockResponse({"object": "error", "details": "Not Found"}, 404) + if "exact=Incomplete+DFC" in url: + return MockResponse(dfc_missing_back) + if "exact=Meld+Fail" in url: + meld_fail_part = meld_part_1.copy() + meld_fail_part["name"] = "Meld Fail" + meld_fail_part["all_parts"] = [ + {"component": "meld_part", "id": "meld-fail-1", "name": "Meld Fail"}, + {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-fail-result"}, + ] + return MockResponse(meld_fail_part) + if url == "http://api.scryfall.com/cards/meld-fail-result": + return MockResponse(None, 404) + + # Generic image response for ANY example.com URL + if url.startswith("http://example.com"): + return MockResponse(None, 200, is_image=True) + + return MockResponse(None, 404) + + monkeypatch.setattr(requests, "get", mock_get) + + +@pytest.fixture +def mock_user_prompts(monkeypatch, tmp_path): + """Mocks all user-facing prompts (file dialogs, console prompts).""" + # Mock Tkinter file dialogs + mock_tk = mock.MagicMock() + + # Path for decklist + decklist_file = tmp_path / "deck.txt" + # Path for card back + card_back_file = tmp_path / "card_back.png" + Image.new("RGB", (10, 10)).save(card_back_file) + + # First call is for decklist, second is for card back + mock_tk.askopenfilename.side_effect = [str(decklist_file), str(card_back_file)] + monkeypatch.setattr("src.order.filedialog.askopenfilename", mock_tk.askopenfilename) + + # Mock InquirerPy prompts + answers = {"stock": constants.Cardstocks.S30.value, "foil": False} + monkeypatch.setattr("src.order.prompt", lambda q: answers) + + # Mock input for deck name + monkeypatch.setattr("builtins.input", lambda _: "Test Deck") + + # Mock away the Tk root window + monkeypatch.setattr("src.order.Tk", mock.MagicMock()) + + return decklist_file + + +@pytest.fixture +def mock_fs(monkeypatch, tmp_path): + """Mocks filesystem, image saving, and directory creation.""" + # Create a temporary 'cards' directory for the test run + cards_dir = tmp_path / "cards" + cards_dir.mkdir() + + # Mock the function that returns the image directory to isolate tests + monkeypatch.setattr("src.order.image_directory", lambda: str(cards_dir)) + monkeypatch.setattr("src.io.image_directory", lambda: str(cards_dir)) + + # By default, pretend no card images exist on disk yet + # THIS IS THE CORRECTED PART + original_os_path_exists = os.path.exists + + def new_mock_exists(path): + # Allow the check for the cards directory to work correctly + if "cards" in str(path): + return original_os_path_exists(path) + # For other paths (image files), return False to force download logic + return False + + monkeypatch.setattr(os.path, "exists", new_mock_exists) + + monkeypatch.setattr(time, "sleep", lambda x: None) # Disable sleep + monkeypatch.setattr(Image.Image, "save", mock.MagicMock()) # Prevent actual image saving + + # endregion # region CardImageCollection @@ -537,8 +778,6 @@ def card_order_element_missing_front_image() -> Generator[ElementTree.Element, N ) -# endregion - # endregion # region test processing.py @@ -547,8 +786,7 @@ def card_order_element_missing_front_image() -> Generator[ElementTree.Element, N def test_post_process_image_with_no_dpi(): """ Tests that post_process_image returns the image unchanged when the - config is provided but max_dpi is None. This covers the final - untested branch in the function. + config is provided but max_dpi is None. """ # Create a dummy image that is larger than any potential DPI img = Image.new("RGB", (1000, 1200), color="red") @@ -1458,13 +1696,72 @@ def test_aggregate_and_split_orders_single_order(): assert result is orders -# endregion - -# region test PdfExporter - - -def test_pdf_export_complete_3_cards_single_file(monkeypatch, card_order_valid): - def do_nothing(_): +def test_card_order_missing_details(input_enter, card_order_element_missing_details): + """ + Tests that parsing a CardOrder with a missing
tag exits gracefully. + """ + with pytest.raises(SystemExit) as exc_info: + CardOrder.from_element(card_order_element_missing_details, allowed_to_exceed_project_max_size=False) + assert exc_info.value.code == 0 + + +def test_aggregate_orders_with_different_details(monkeypatch_project_max_size): + """ + Tests that orders with different details (stock, foil) are not combined, + even if `combine_orders` is True. + """ + monkeypatch_project_max_size(10) + orders = [ + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S33.value, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + ] + aggregated = aggregate_and_split_orders( + orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=True + ) + # The orders should not have been combined + assert len(aggregated) == 2 + assert sorted([o.details.quantity for o in aggregated]) == [2, 2] + + +def test_aggregate_orders_no_combine(monkeypatch_project_max_size): + """ + Tests that orders are not combined when `combine_orders` is False. + """ + monkeypatch_project_max_size(10) + orders = [ + CardOrder( + details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), + fronts=CardImageCollection(num_slots=2), + backs=CardImageCollection(num_slots=2), + ), + CardOrder( + details=Details(quantity=3, stock=constants.Cardstocks.S30.value, foil=False), + fronts=CardImageCollection(num_slots=3), + backs=CardImageCollection(num_slots=3), + ), + ] + aggregated = aggregate_and_split_orders( + orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=False + ) + assert len(aggregated) == 2 + assert sorted([o.details.quantity for o in aggregated]) == [2, 3] + + +# endregion + +# region test PdfExporter + + +def test_pdf_export_complete_3_cards_single_file(monkeypatch, card_order_valid): + def do_nothing(_): return None monkeypatch.setattr("src.pdf_maker.PdfExporter.ask_questions", do_nothing) @@ -1588,143 +1885,7 @@ def test_card_order_complete_run_multiple_cardbacks(browser, site, input_enter, # endregion -# region Fixtures for New Tests - - -@pytest.fixture() -def card_order_element_missing_details() -> ElementTree.Element: - """Provides a CardOrder element that is missing the
tag.""" - return ElementTree.fromstring( - textwrap.dedent( - """ - - - - some_id - 0 - a.png - - - some_cardback - - """ - ) - ) - - -@pytest.fixture() -def card_order_element_front_slot_out_of_bounds() -> ElementTree.Element: - """Provides a CardOrder where a front card has a slot index greater than the quantity.""" - return ElementTree.fromstring( - textwrap.dedent( - """ - -
- 2 - (S30) Standard Smooth - false -
- - - some_id - 0,2 - a.png - - - some_cardback - - """ - ) - ) - - -@pytest.fixture() -def card_image_collection_element_invalid_slots() -> ElementTree.Element: - """Provides a CardImageCollection where a card's slots are out of bounds.""" - return ElementTree.fromstring( - textwrap.dedent( - """ - - - a - 0,3 - a.png - - - """ - ) - ) - - -# endregion - -# region New Test Cases - - -def test_card_order_missing_details(input_enter, card_order_element_missing_details): - """ - Tests that parsing a CardOrder with a missing
tag exits gracefully. - This covers the error handling path in `CardOrder.from_element`. - """ - with pytest.raises(SystemExit) as exc_info: - CardOrder.from_element(card_order_element_missing_details, allowed_to_exceed_project_max_size=False) - assert exc_info.value.code == 0 - - -def test_aggregate_orders_with_different_details(monkeypatch_project_max_size): - """ - Tests that orders with different details (stock, foil) are not combined, - even if `combine_orders` is True. - """ - monkeypatch_project_max_size(10) - orders = [ - CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), - fronts=CardImageCollection(num_slots=2), - backs=CardImageCollection(num_slots=2), - ), - CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S33.value, foil=False), - fronts=CardImageCollection(num_slots=2), - backs=CardImageCollection(num_slots=2), - ), - ] - aggregated = aggregate_and_split_orders( - orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=True - ) - # The orders should not have been combined - assert len(aggregated) == 2 - assert sorted([o.details.quantity for o in aggregated]) == [2, 2] - - -def test_aggregate_orders_no_combine(monkeypatch_project_max_size): - """ - Tests that orders are not combined when `combine_orders` is False. - """ - monkeypatch_project_max_size(10) - orders = [ - CardOrder( - details=Details(quantity=2, stock=constants.Cardstocks.S30.value, foil=False), - fronts=CardImageCollection(num_slots=2), - backs=CardImageCollection(num_slots=2), - ), - CardOrder( - details=Details(quantity=3, stock=constants.Cardstocks.S30.value, foil=False), - fronts=CardImageCollection(num_slots=3), - backs=CardImageCollection(num_slots=3), - ), - ] - aggregated = aggregate_and_split_orders( - orders, target_site=constants.TargetSites.MakePlayingCards, combine_orders=False - ) - # The orders should remain separate - assert len(aggregated) == 2 - assert sorted([o.details.quantity for o in aggregated]) == [2, 3] - - -# endregion - -# region test io.py +# region test scryfall downloader def test_download_image_from_scryfall_success(monkeypatch): @@ -1790,10 +1951,8 @@ def test_remove_files_os_error(monkeypatch): def raise_os_error(path): raise OSError("Permission denied") - # Make os.remove raise an OSError monkeypatch.setattr("src.io.os.remove", raise_os_error) - # This should now execute without crashing remove_files(["non_existent_file.txt"]) @@ -1805,16 +1964,14 @@ def test_remove_directories_os_error(monkeypatch): def raise_os_error(path): raise OSError("Directory not empty") - # Make os.rmdir raise an OSError monkeypatch.setattr("src.io.os.rmdir", raise_os_error) - # This should now execute without crashing remove_directories(["non_existent_dir"]) # endregion -# region from_decklist tests +# region test from_decklist # Helper to create a valid, minimal PNG byte object def create_dummy_png() -> bytes: @@ -1825,190 +1982,12 @@ def create_dummy_png() -> bytes: return byte_io.getvalue() -@pytest.fixture -def mock_scryfall_requests(monkeypatch): - """Mocks requests.get to return canned Scryfall API responses.""" - - # --- Mock Card Data --- - sfc_card = { - "object": "card", - "id": "sfc-1", - "name": "Llanowar Elves", - "layout": "normal", - "image_uris": {"png": "http://example.com/llanowar_elves.png"}, - } - - dfc_card = { - "object": "card", - "id": "dfc-1", - "name": "Delver of Secrets // Insectile Aberration", - "layout": "transform", - "card_faces": [ - {"name": "Delver of Secrets", "image_uris": {"png": "http://example.com/delver.png"}}, - {"name": "Insectile Aberration", "image_uris": {"png": "http://example.com/insect.png"}}, - ], - } - - meld_part_1 = { - "object": "card", - "id": "meld-part-1", - "name": "Gisela, the Broken Blade", - "layout": "meld", - "image_uris": {"png": "http://example.com/gisela.png"}, - "all_parts": [ - {"component": "meld_part", "id": "meld-part-1", "name": "Gisela, the Broken Blade"}, - {"component": "meld_part", "id": "meld-part-2", "name": "Bruna, the Fading Light"}, - {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-result-1"}, - ], - } - - meld_part_2 = { - "object": "card", - "id": "meld-part-2", - "name": "Bruna, the Fading Light", - "layout": "meld", - "image_uris": {"png": "http://example.com/bruna.png"}, - "all_parts": meld_part_1["all_parts"], # Share the same all_parts - } - - meld_result = { - "object": "card", - "id": "meld-result-1", - "name": "Brisela, Voice of Nightmares", - "image_uris": {"png": "http://example.com/brisela.png"}, - } - - # A DFC missing its back face - dfc_missing_back = { - "object": "card", - "id": "dfc-missing-back", - "name": "Incomplete DFC", - "layout": "transform", - "card_faces": [ - {"name": "Front Face", "image_uris": {"png": "http://example.com/front.png"}}, - {"name": "Back Face", "image_uris": {}}, # Missing PNG - ], - } - - # --- Mock Requests --- - # Create a single dummy image to be returned by all successful image requests - dummy_image_content = create_dummy_png() - - class MockResponse: - def __init__(self, json_data, status_code=200, is_image=False): - self._json_data = json_data - self.status_code = status_code - if is_image: - self.content = dummy_image_content - else: - self.content = b"" - - def json(self): - return self._json_data - - def raise_for_status(self): - if self.status_code >= 400: - raise requests.exceptions.HTTPError(f"{self.status_code} Client Error") - - def mock_get(url, **kwargs): - if "exact=Llanowar+Elves" in url: - return MockResponse(sfc_card) - if "exact=Delver+of+Secrets" in url: - return MockResponse(dfc_card) - if "exact=Gisela%2C+the+Broken+Blade" in url: - return MockResponse(meld_part_1) - if "exact=Bruna%2C+the+Fading+Light" in url: - return MockResponse(meld_part_2) - if url == "http://api.scryfall.com/cards/meld-result-1": - return MockResponse(meld_result) - if "exact=Card+Not+Found" in url: - return MockResponse({"object": "error", "details": "Not Found"}, 404) - if "exact=Incomplete+DFC" in url: - return MockResponse(dfc_missing_back) - if "exact=Meld+Fail" in url: - meld_fail_part = meld_part_1.copy() - meld_fail_part["name"] = "Meld Fail" - meld_fail_part["all_parts"] = [ - {"component": "meld_part", "id": "meld-fail-1", "name": "Meld Fail"}, - {"component": "meld_result", "uri": "http://api.scryfall.com/cards/meld-fail-result"}, - ] - return MockResponse(meld_fail_part) - if url == "http://api.scryfall.com/cards/meld-fail-result": - return MockResponse(None, 404) - - # Generic image response for ANY example.com URL - if url.startswith("http://example.com"): - return MockResponse(None, 200, is_image=True) - - return MockResponse(None, 404) - - monkeypatch.setattr(requests, "get", mock_get) - - -@pytest.fixture -def mock_user_prompts(monkeypatch, tmp_path): - """Mocks all user-facing prompts (file dialogs, console prompts).""" - # Mock Tkinter file dialogs - mock_tk = mock.MagicMock() - - # Path for decklist - decklist_file = tmp_path / "deck.txt" - # Path for card back - card_back_file = tmp_path / "card_back.png" - Image.new("RGB", (10, 10)).save(card_back_file) - - # First call is for decklist, second is for card back - mock_tk.askopenfilename.side_effect = [str(decklist_file), str(card_back_file)] - monkeypatch.setattr("src.order.filedialog.askopenfilename", mock_tk.askopenfilename) - - # Mock InquirerPy prompts - answers = {"stock": constants.Cardstocks.S30.value, "foil": False} - monkeypatch.setattr("src.order.prompt", lambda q: answers) - - # Mock input for deck name - monkeypatch.setattr("builtins.input", lambda _: "Test Deck") - - # Mock away the Tk root window - monkeypatch.setattr("src.order.Tk", mock.MagicMock()) - - return decklist_file - - -@pytest.fixture -def mock_fs(monkeypatch, tmp_path): - """Mocks filesystem, image saving, and directory creation.""" - # Create a temporary 'cards' directory for the test run - cards_dir = tmp_path / "cards" - cards_dir.mkdir() - - # Mock the function that returns the image directory to isolate tests - monkeypatch.setattr("src.order.image_directory", lambda: str(cards_dir)) - monkeypatch.setattr("src.io.image_directory", lambda: str(cards_dir)) - - # By default, pretend no card images exist on disk yet - # THIS IS THE CORRECTED PART - original_os_path_exists = os.path.exists - - def new_mock_exists(path): - # Allow the check for the cards directory to work correctly - if "cards" in str(path): - return original_os_path_exists(path) - # For other paths (image files), return False to force download logic - return False - - monkeypatch.setattr(os.path, "exists", new_mock_exists) - - monkeypatch.setattr(time, "sleep", lambda x: None) # Disable sleep - monkeypatch.setattr(Image.Image, "save", mock.MagicMock()) # Prevent actual image saving - - def test_from_decklist_simple_cards(mock_user_prompts, mock_scryfall_requests, mock_fs, monkeypatch): """Tests a simple decklist with only single-faced cards.""" decklist_file = mock_user_prompts - decklist_content = "2 Llanowar Elves\n1 Forest" # Forest will fail, testing robustness + decklist_content = "2 Llanowar Elves\n1 Forest" decklist_file.write_text(decklist_content, encoding="utf-8") - # Mock the 'Forest' call to fail original_get = requests.get def side_effect_get(url, **kwargs): @@ -2021,13 +2000,12 @@ def side_effect_get(url, **kwargs): order = CardOrder.from_decklist() assert order.name == "Test Deck" - assert order.details.quantity == 2 # 2 Llanowar Elves, Forest failed + assert order.details.quantity == 2 assert order.details.stock == constants.Cardstocks.S30.value assert not order.details.foil assert len(order.fronts.cards_by_id) == 1 - assert len(order.backs.cards_by_id) == 1 # Common back + assert len(order.backs.cards_by_id) == 1 - # Check that all 2 slots for backs are filled by the common back back_card = list(order.backs.cards_by_id.values())[0] assert back_card.name == "card_back.png" assert back_card.slots == {0, 1} @@ -2061,29 +2039,21 @@ def test_from_decklist_meld_cards_happy_path(mock_user_prompts, mock_scryfall_re decklist_content = "1 Gisela, the Broken Blade\n1 Bruna, the Fading Light" decklist_file.write_text(decklist_content) - # We need to mock the image processing part of the meld logic. - # First, mock the generic image fetcher to prevent it from running for the front faces. - # It's not what we're testing here. monkeypatch.setattr("src.order._fetch_and_prepare_image", lambda url, name: f"/tmp/{name}.png") - # Now, set up specific mocks for the meld back image processing mock_img_instance = mock.MagicMock(spec=Image.Image) mock_img_instance.size = (800, 1120) mock_img_instance.mode = "RGB" - # When crop is called, return another mock. This mock also needs a 'save' method. mock_crop_instance = mock.MagicMock(spec=Image.Image) mock_crop_instance.size = (400, 560) mock_crop_instance.mode = "RGB" mock_crop_instance.transpose.return_value = mock_crop_instance - mock_crop_instance.save = mock.MagicMock() # Mock the save method + mock_crop_instance.save = mock.MagicMock() mock_img_instance.crop.return_value = mock_crop_instance - # The only place `Image.open` is now called in the code flow is for the meld back. - # We can safely mock it to return our pre-configured mock image instance. monkeypatch.setattr(Image, "open", lambda bio: mock_img_instance) - # Mock ImageOps.expand to prevent it from crashing on the mock image object monkeypatch.setattr(ImageOps, "expand", lambda image, **kwargs: image) order = CardOrder.from_decklist() @@ -2112,7 +2082,6 @@ def test_from_decklist_error_handling(mock_user_prompts, mock_scryfall_requests, order = CardOrder.from_decklist() - # Only Llanowar Elves (1) and Incomplete DFC (1) and Meld Fail (1) should be added assert order.details.quantity == 3 logs = caplog.text