Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .coveragerc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ omit =
*/__pycache__/*
*/site-packages/*
*/.venv/*
percy/robot_library.py
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — Entire robot_library.py excluded from coverage

The 12 unit tests in test_robot_library.py exercise the parse helpers and keyword dispatch, but excluding the file from coverage means those tests don't contribute to coverage metrics. This also means future regressions in robot_library.py won't be caught by coverage thresholds.

Consider removing this exclusion and using # pragma: no cover on specific lines that genuinely can't be covered (e.g., the else: stub class, the if ROBOT_AVAILABLE: branch boundary).


[report]
# Regexes for lines to exclude from consideration
Expand Down
2 changes: 2 additions & 0 deletions .pylintrc
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
[MASTER]
fail-under=10.0
ignore-patterns=.*\.robot$
disable=
broad-except,
broad-exception-raised,
global-statement,
import-error,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — Globally disabling import-error is too broad

This suppresses legitimate import errors across the entire codebase, not just for the optional robotframework imports. The Robot-specific imports are already wrapped in try/except ImportError in the code.

Consider using a targeted inline disable instead:

from robot.api.deco import keyword, library  # pylint: disable=import-error

Or scope it to the specific file in .pylintrc:

[MESSAGES CONTROL]
# Only for robot_library.py

invalid-name,
missing-class-docstring,
missing-function-docstring,
Expand Down
2 changes: 2 additions & 0 deletions development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ httpretty==1.0.*
pylint==2.*
twine
coverage==7.*
robotframework>=5.0
robotframework-browser>=16.0
6 changes: 6 additions & 0 deletions percy/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
from percy.version import __version__
from percy.screenshot import percy_automate_screenshot

# Robot Framework support — graceful when robotframework is not installed
try:
from percy.robot_library import PercyLibrary # pragma: no cover
except ImportError: # pragma: no cover
pass

# import snapshot command
try:
from percy.screenshot import percy_snapshot
Expand Down
267 changes: 267 additions & 0 deletions percy/robot_library.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,267 @@
"""Robot Framework library for Percy visual testing with Playwright.

Provides keywords to capture Percy snapshots from Robot Framework tests
using the Browser library (robotframework-browser) as the Playwright backend.
All robot-specific imports are wrapped in try/except for graceful degradation
when robotframework is not installed.

Usage in Robot Framework:
*** Settings ***
Library Browser
Library percy.robot_library.PercyLibrary

*** Test Cases ***
Homepage Visual Test
New Browser chromium headless=true
New Page https://example.com
Percy Snapshot Homepage
Close Browser

Run with:
percy exec -- robot tests/
"""

import json

try:
from robot.api.deco import keyword, library
from robot.libraries.BuiltIn import BuiltIn
ROBOT_AVAILABLE = True
except ImportError:
ROBOT_AVAILABLE = False

# Lazy imports — percy.screenshot requires playwright at import time,
# so we defer the import to when keywords are actually called.
_screenshot_module = None

def _get_screenshot_module():
global _screenshot_module # pylint: disable=global-statement
if _screenshot_module is None:
from percy import screenshot as _mod # pylint: disable=import-outside-toplevel
_screenshot_module = _mod
return _screenshot_module


def _parse_bool(val):
if val is None:
return None
return str(val).lower() in ("true", "1", "yes")


def _parse_widths(widths):
if not widths:
return None
if isinstance(widths, str):
return [int(w.strip()) for w in widths.split(",")]
if isinstance(widths, list):
return [int(w) for w in widths]
return None


def _parse_csv(val):
if not val:
return None
if isinstance(val, str):
return [v.strip() for v in val.split(",")]
if isinstance(val, list):
return val
return None


def _parse_json(val):
if not val:
return None
if isinstance(val, str):
return json.loads(val)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOW — json.loads() can throw json.JSONDecodeError on malformed input

Robot Framework users pass all values as strings, so a typo like scope_options={not-valid-json} will produce an opaque json.JSONDecodeError stack trace. Consider wrapping in try/except with a Robot-friendly error message:

if isinstance(val, str):
    try:
        return json.loads(val)
    except json.JSONDecodeError as exc:
        raise ValueError(f"Invalid JSON: {val!r}") from exc

Same issue exists in the Selenium PR.

if isinstance(val, (dict, list)):
return val
return None


class _BrowserLibraryPageAdapter:
"""Adapter to make Browser library (robotframework-browser) look like a Playwright page.

Percy's playwright SDK expects a Playwright page object with methods like
evaluate(), url, and content(). This adapter bridges the gap by calling
Browser library keywords via Robot Framework's BuiltIn.
"""

@property
def url(self):
return BuiltIn().run_keyword("Browser.Get Url")

def evaluate(self, expression):
return BuiltIn().run_keyword("Browser.Evaluate JavaScript", "", expression)

def content(self):
return BuiltIn().run_keyword("Browser.Get Page Source")


if ROBOT_AVAILABLE:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — Missing CLIENT_INFO/ENV_INFO override (inconsistency with Selenium PR)

The Selenium PR (percy-selenium-python#220) overrides _snapshot_module.CLIENT_INFO and ENV_INFO with Robot Framework-specific values (percy-robotframework-selenium/..., robotframework/VERSION, etc.) so Percy analytics can attribute snapshots to the Robot Framework integration.

This PR doesn't set any client/env info, so Percy will see these snapshots as coming from the base percy-playwright-python SDK with no Robot Framework attribution. Consider adding the same pattern:

_ROBOT_CLIENT_INFO = f'percy-robotframework-playwright/{SDK_VERSION}'
_ROBOT_ENV_INFO = [
    f'robotframework/{robot.version.VERSION}',
    f'python/{platform.python_version()}',
]


@library(scope="GLOBAL")
class PercyLibrary:
"""Percy visual testing library for Robot Framework (Playwright/Browser backend).

Provides keywords to capture visual snapshots using Percy.
Requires Browser library (robotframework-browser) to be imported.

Tests must be run under ``percy exec``:
| percy exec -- robot tests/
"""

def _get_page(self):
"""Get a page adapter for the Browser library."""
try:
BuiltIn().get_library_instance("Browser")
return _BrowserLibraryPageAdapter()
except RuntimeError as exc:
raise RuntimeError(
"PercyLibrary requires Browser library to be imported"
) from exc

# --------------------------------------------------------------
# Percy Snapshot
# --------------------------------------------------------------

@keyword("Percy Snapshot")
def percy_snapshot_keyword( # pylint: disable=too-many-arguments,too-many-locals
self, name, widths=None, min_height=None,
percy_css=None, scope=None, scope_options=None,
enable_javascript=None, enable_layout=None,
disable_shadow_dom=None, labels=None,
test_case=None, sync=None, regions=None,
responsive_snapshot_capture=None,
):
"""Capture a Percy visual snapshot of the current page.

``name`` is the snapshot name shown in the Percy dashboard.

``widths`` is a comma-separated string of responsive widths
(e.g., ``375,768,1280``).

``min_height`` is the minimum screenshot height in pixels.

``percy_css`` is custom CSS injected before the snapshot.

``scope`` is a CSS selector to limit the snapshot area.

``enable_javascript`` enables JS execution in Percy rendering.

``enable_layout`` enables layout comparison mode.

``labels`` is a comma-separated string of tags/labels.

``regions`` is a JSON string of region definitions.

``responsive_snapshot_capture`` enables responsive capture mode.

Examples:
| Percy Snapshot Homepage
| Percy Snapshot Login widths=375,1280 min_height=1024
| Percy Snapshot Dashboard labels=dashboard,admin enable_layout=True
"""
page = self._get_page()
mod = _get_screenshot_module()
mod.percy_snapshot(
page,
name,
widths=_parse_widths(widths),
min_height=int(min_height) if min_height else None,
percy_css=percy_css,
scope=scope,
scope_options=_parse_json(scope_options),
enable_javascript=_parse_bool(enable_javascript),
enable_layout=_parse_bool(enable_layout),
disable_shadow_dom=_parse_bool(disable_shadow_dom),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HIGH — labels is joined back into a string after being parsed into a list

labels=",".join(_parse_csv(labels)) if labels else None,

_parse_csv("regression,v2") returns ["regression", "v2"], then ",".join(...) converts it back to "regression,v2" — a string. But the test at test_robot_library.py:75 asserts:

assert call_kwargs["labels"] == ["regression", "v2"]  # a list!

Either the code or the test is wrong. If percy_snapshot() expects a list, the fix is:

labels=_parse_csv(labels),

If it expects a comma-separated string, the test assertion should be:

assert call_kwargs["labels"] == "regression,v2"

This same issue exists in the Selenium PR (percy-selenium-python#220). Please verify what percy_snapshot() expects and fix both PRs consistently.

labels=",".join(_parse_csv(labels)) if labels else None,
test_case=test_case,
sync=_parse_bool(sync),
regions=_parse_json(regions),
responsive_snapshot_capture=_parse_bool(responsive_snapshot_capture),
)

# --------------------------------------------------------------
# Region helpers
# --------------------------------------------------------------

@keyword("Create Percy Region")
def create_percy_region_keyword( # pylint: disable=too-many-arguments
self, algorithm="ignore",
bounding_box=None, element_xpath=None,
element_css=None, padding=None,
diff_sensitivity=None,
image_ignore_threshold=None,
carousels_enabled=None,
banners_enabled=None, ads_enabled=None,
diff_ignore_threshold=None,
):
"""Create a region definition for Percy ignore/consider regions.

``algorithm`` is one of ``ignore``, ``standard``, or ``intelliignore``.

``element_css`` is a CSS selector for the region.

``element_xpath`` is an XPath selector for the region.

``bounding_box`` is a JSON string with x, y, width, height.

``padding`` is padding in pixels around the element.

Returns a region dict to pass to ``Percy Snapshot`` via ``regions``.

== Usage with Percy Snapshot ==
| ${region}= Create Percy Region algorithm=ignore element_css=.ad-banner
| Percy Snapshot Homepage regions=${{json.dumps([${region}])}}

== Multiple regions ==
| ${ignore}= Create Percy Region element_css=h1
| ${consider}= Create Percy Region
| ... algorithm=standard element_css=.content
| Percy Snapshot Mixed
| ... regions=${{json.dumps([${ignore}, ...])}}

== With padding and bounding box ==
| ${region}= Create Percy Region
| ... element_css=.banner padding=10
| ${region}= Create Percy Region
| ... bounding_box={"x":0,"y":0,"width":200}
"""
mod = _get_screenshot_module()
return mod.create_region(
boundingBox=_parse_json(bounding_box),
elementXpath=element_xpath,
elementCSS=element_css,
padding=int(padding) if padding else None,
algorithm=algorithm,
diffSensitivity=int(diff_sensitivity) if diff_sensitivity else None,
imageIgnoreThreshold=(
float(image_ignore_threshold) if image_ignore_threshold else None
),
carouselsEnabled=_parse_bool(carousels_enabled),
bannersEnabled=_parse_bool(banners_enabled),
adsEnabled=_parse_bool(ads_enabled),
diffIgnoreThreshold=float(diff_ignore_threshold) if diff_ignore_threshold else None,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEDIUM — Uses private _is_percy_enabled() instead of public API

return bool(mod._is_percy_enabled())

The Selenium PR (percy-selenium-python#220) uses the public is_percy_enabled() function. This PR accesses _is_percy_enabled() (prefixed with underscore = private/internal). Private APIs can change without notice.

Suggestion — check if this module exports a public equivalent:

return bool(mod.is_percy_enabled())

Or import the public function directly as the Selenium PR does.

# --------------------------------------------------------------
# Utility
# --------------------------------------------------------------

@keyword("Percy Is Running")
def percy_is_running_keyword(self):
"""Check if the Percy CLI server is running.

Returns ``True`` if Percy is available, ``False`` otherwise.
"""
mod = _get_screenshot_module()
return bool(mod._is_percy_enabled()) # pylint: disable=protected-access

else:
class PercyLibrary: # pylint: disable=function-redefined,too-few-public-methods
"""Stub -- robotframework is not installed."""
def __init__(self):
raise ImportError(
"robotframework is not installed. "
"Install it with: pip install robotframework robotframework-browser"
)
2 changes: 1 addition & 1 deletion percy/version.py
Original file line number Diff line number Diff line change
@@ -1 +1 @@
__version__ = "1.0.2"
__version__ = "1.1.0"
Loading
Loading