Andrew/test utils

src/py/CHANGELOG.txt

@@ -1,4 +1,13 @@
+- Add testing
+- Fix a variety of type bugs
+- Change order of browser closer to fix hang
+- Explicitly handle certain argument options better
+- Move temp file creation to .open() out of __init__()
+- Reduce mathjax version to plotly.py
+- Fix hang and add automatic close with stop_sync_server
+- Add option to silence warnings in start/stop_sync_server
 - Fix bug where attribute was inconsistently named
+
 v1.1.0rc0
 - Improve verbosity of errors when starting kaleido improperly
 - Add new api functions start/stop_sync_server

src/py/kaleido/_fig_tools.py

@@ -1,36 +1,53 @@
+"""
+Adapted from old code, it 1. validates, 2. write defaults, 3. packages object.
+
+Its a bit complicated and mixed in order.
+"""
+
 from __future__ import annotations
 
 import glob
 import re
 from pathlib import Path
-from typing import TYPE_CHECKING, Any, Literal, TypedDict
+from typing import TYPE_CHECKING, Literal, TypedDict
 
 import logistro
 
 if TYPE_CHECKING:
+    from typing import Any
+
     from typing_extensions import TypeGuard
 
+    Figurish = Any  # Be nice to make it more specific, dictionary or something
+    FormatString = Literal["png", "jpg", "jpeg", "webp", "svg", "json", "pdf"]
+
 _logger = logistro.getLogger(__name__)
 
 # constants
 DEFAULT_EXT = "png"
 DEFAULT_SCALE = 1
 DEFAULT_WIDTH = 700
 DEFAULT_HEIGHT = 500
-SUPPORTED_FORMATS = ("png", "jpg", "jpeg", "webp", "svg", "json", "pdf")
-FormatString = Literal["png", "jpg", "jpeg", "webp", "svg", "json", "pdf"]
+SUPPORTED_FORMATS: tuple[FormatString, ...] = (
+    "png",
+    "jpg",
+    "jpeg",
+    "webp",
+    "svg",
+    "json",
+    "pdf",
+)
 
 
 def _assert_format(ext: str) -> TypeGuard[FormatString]:
     if ext not in SUPPORTED_FORMATS:
-        raise ValueError(f"File format {ext} is not supported.")
+        raise ValueError(
+            f"Invalid format '{ext}'.\n    Supported formats: {SUPPORTED_FORMATS!s}",
+        )
     return True
 
 
-Figurish = Any  # Be nice to make it more specific, dictionary or something
-
-
-def _is_figurish(o) -> TypeGuard[Figurish]:
+def _is_figurish(o: Any) -> TypeGuard[Figurish]:
     valid = hasattr(o, "to_dict") or (isinstance(o, dict) and "data" in o)
     if not valid:
         _logger.debug(
@@ -41,7 +58,11 @@ def _is_figurish(o) -> TypeGuard[Figurish]:
     return valid
 
 
-def _get_figure_dimensions(layout, width, height):
+def _get_figure_dimensions(
+    layout: dict,
+    width: float | None,
+    height: float | None,
+) -> tuple[float, float]:
     # Compute image width / height with fallbacks
     width = (
         width
@@ -58,29 +79,25 @@ def _get_figure_dimensions(layout, width, height):
     return width, height
 
 
-def _get_format(extension):
-    original_format = extension
-    extension = extension.lower()
-    if extension == "jpg":
+def _get_format(extension: str) -> FormatString:
+    formatted_extension = extension.lower()
+    if formatted_extension == "jpg":
         return "jpeg"
-
-    if extension not in SUPPORTED_FORMATS:
-        raise ValueError(
-            f"Invalid format '{original_format}'.\n"
-            f"    Supported formats: {SUPPORTED_FORMATS!s}",
-        )
-    return extension
+    if not _assert_format(formatted_extension):
+        raise ValueError  # this line will never be reached its for typer
+    return formatted_extension
 
 
-# Input of to_spec
+# Input of to_spec (user gives us this)
 class LayoutOpts(TypedDict, total=False):
     format: FormatString | None
     scale: int | float
     height: int | float
     width: int | float
 
 
-# Output of to_spec
+# Output of to_spec (we give kaleido_scopes.js this)
+# refactor note: this could easily be right before send
 class Spec(TypedDict):
     format: FormatString
     width: int | float
@@ -89,7 +106,8 @@ class Spec(TypedDict):
     data: Figurish
 
 
-def to_spec(figure, layout_opts: LayoutOpts) -> Spec:
+# validate configuration options for kaleido.js and package like its wants
+def to_spec(figure: Figurish, layout_opts: LayoutOpts) -> Spec:
     # Get figure layout
     layout = figure.get("layout", {})
 
@@ -107,6 +125,7 @@ def to_spec(figure, layout_opts: LayoutOpts) -> Spec:
 
     # Extract info
     extension = _get_format(layout_opts.get("format") or DEFAULT_EXT)
+
     width, height = _get_figure_dimensions(
         layout,
         layout_opts.get("width"),
@@ -123,7 +142,9 @@ def to_spec(figure, layout_opts: LayoutOpts) -> Spec:
     }
 
 
-def _next_filename(path, prefix, ext) -> str:
+# if we need to suffix the filename automatically:
+def _next_filename(path: Path | str, prefix: str, ext: str) -> str:
+    path = path if isinstance(path, Path) else Path(path)
     default = 1 if (path / f"{prefix}.{ext}").exists() else 0
     re_number = re.compile(
         r"^" + re.escape(prefix) + r"\-(\d+)\." + re.escape(ext) + r"$",
@@ -139,36 +160,16 @@ def _next_filename(path, prefix, ext) -> str:
     return f"{prefix}.{ext}" if n == 1 else f"{prefix}-{n}.{ext}"
 
 
-def build_fig_spec(  #  noqa: C901, PLR0912
+# validate and build full route if needed:
+def _build_full_path(
+    path: Path | None,
     fig: Figurish,
-    path: Path | str | None,
-    opts: LayoutOpts | None,
-) -> tuple[Spec, Path]:
-    if not opts:
-        opts = {}
-
-    if not _is_figurish(fig):
-        raise TypeError("Figure supplied doesn't seem to be a valid plotly figure.")
-
-    if hasattr(fig, "to_dict"):
-        fig = fig.to_dict()
-
-    if isinstance(path, str):
-        path = Path(path)
-    elif path and not isinstance(path, Path):
-        raise TypeError("Path should be a string or `pathlib.Path` object (or None)")
-
-    if path and path.suffix and not opts.get("format"):
-        ext = path.suffix.lstrip(".")
-        if _assert_format(ext):  # not strict necessary if but helps typeguard
-            opts["format"] = ext
-
-    spec = to_spec(fig, opts)
-
-    ext = spec["format"]
-
+    ext: FormatString,
+) -> Path:
     full_path: Path | None = None
+
     directory: Path
+
     if not path:
         directory = Path()  # use current Path
     elif path and (not path.suffix or path.is_dir()):
@@ -181,6 +182,7 @@ def build_fig_spec(  #  noqa: C901, PLR0912
             raise RuntimeError(
                 f"Cannot reach path {path.parent}. Are all directories created?",
             )
+
     if not full_path:
         _logger.debug("Looking for title")
         prefix = fig.get("layout", {}).get("title", {}).get("text", "fig")
@@ -190,5 +192,36 @@ def build_fig_spec(  #  noqa: C901, PLR0912
         _logger.debug(f"Found: {prefix}")
         name = _next_filename(directory, prefix, ext)
         full_path = directory / name
+    return full_path
+
+
+# call all validators/automatic config fill-in/packaging in expected format
+def build_fig_spec(
+    fig: Figurish,
+    path: Path | str | None,
+    opts: LayoutOpts | None,
+) -> tuple[Spec, Path]:
+    if not opts:
+        opts = {}
+
+    if not _is_figurish(fig):
+        raise TypeError("Figure supplied doesn't seem to be a valid plotly figure.")
+
+    if hasattr(fig, "to_dict"):
+        fig = fig.to_dict()
+
+    if isinstance(path, str):
+        path = Path(path)
+    elif path and not isinstance(path, Path):
+        raise TypeError("Path should be a string or `pathlib.Path` object (or None)")
+
+    if not opts.get("format") and path and path.suffix:
+        ext = path.suffix.lstrip(".")
+        if _assert_format(ext):  # not strict necessary if but helps typeguard
+            opts["format"] = ext
+
+    spec = to_spec(fig, opts)
+
+    full_path = _build_full_path(path, fig, spec["format"])
 
     return spec, full_path

src/py/pyproject.toml

@@ -30,6 +30,7 @@ dependencies = [
   "logistro>=1.0.8",
   "orjson>=3.10.15",
   "packaging",
+  "pytest-timeout>=2.4.0",
 ]
 
 [project.urls]
@@ -94,7 +95,7 @@ log_cli = false
 # name = cmd
 
 [tool.poe.tasks.test]
-cmd = "pytest --log-level=1 -W error -n auto -v -rfE --capture=fd"
+cmd = "pytest --timeout=90 --log-level=1 -W error -n auto -v -rfE --capture=fd"
 help = "Run all tests quickly"
 
 [tool.poe.tasks.debug-test]

src/py/tests/conftest.py

@@ -20,7 +20,7 @@
 )
 
 is_ci = os.getenv("GITHUB_ACTIONS") == "true" or os.getenv("CI") == "true"
-if is_ci and platform.system in {"Windows", "Darwin"}:
+if is_ci and platform.system() in {"Windows", "Darwin"}:
     settings.load_profile("ci")