Add Robot Framework support via PercyLibrary (Browser library)#138
Add Robot Framework support via PercyLibrary (Browser library)#138neha-sanse wants to merge 9 commits intomainfrom
Conversation
Adds native Robot Framework keywords for Percy visual testing using the Browser library (robotframework-browser / Playwright): - Percy Snapshot — capture DOM snapshots with all options - Create Percy Region — build ignore/consider region definitions - Percy Is Running — check Percy CLI availability Robot Framework and Playwright imports are wrapped for graceful degradation — existing percy-playwright behavior is unchanged when robotframework is not installed. Uses lazy imports so playwright is only loaded when keywords are actually called. Includes _BrowserLibraryPageAdapter that bridges Browser library keywords to the Playwright page interface percy_snapshot() expects. 12 new tests covering parse helpers, keyword dispatch, and lazy imports. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
23 end-to-end Robot tests using Browser library (Playwright) covering all Percy keywords: basic snapshots, responsive widths, min height, Percy CSS, scoped snapshots, JS/layout/shadow DOM, labels, test case metadata, ignore/consider/intelliignore regions, all options combined, and utility keywords. Run with: percy exec -- robot tests/test_robot_integration.robot Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ror, exclude robot_library from coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…port Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cess, import-outside-toplevel Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cy_snapshot Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Show full workflow for using Create Percy Region with Percy Snapshot, including multiple regions, padding, and bounding box examples. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Shorten docstring examples to stay within 100-char limit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
amandeepsingh333
left a comment
There was a problem hiding this comment.
Review: Robot Framework support (Playwright backend)
Reviewed alongside the sister PR percy-selenium-python#220. The overall approach is sound — lazy imports, graceful degradation, adapter pattern for the Browser library. A few issues below, most of which are shared with the Selenium PR.
Summary of findings:
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | HIGH | robot_library.py |
labels is joined back into a string after parsing — likely a bug (test asserts a list) |
| 2 | MEDIUM | robot_library.py |
Uses private _is_percy_enabled() instead of public API |
| 3 | MEDIUM | robot_library.py |
No CLIENT_INFO/ENV_INFO override — Percy analytics won't attribute snapshots to Robot Framework (Selenium PR has this) |
| 4 | LOW | robot_library.py |
_parse_json doesn't catch json.JSONDecodeError — malformed JSON from Robot users gets an opaque stack trace |
| 5 | LOW | .pylintrc |
Globally disabling import-error suppresses legitimate import issues across the entire codebase |
| 6 | LOW | .coveragerc |
Entire robot_library.py excluded from coverage — unit tests in this PR should count |
| 7 | LOW | test_robot_library.py |
No test coverage for _BrowserLibraryPageAdapter |
| 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), |
There was a problem hiding this comment.
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.
| adsEnabled=_parse_bool(ads_enabled), | ||
| diffIgnoreThreshold=float(diff_ignore_threshold) if diff_ignore_threshold else None, | ||
| ) | ||
|
|
There was a problem hiding this comment.
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.
| return BuiltIn().run_keyword("Browser.Get Page Source") | ||
|
|
||
|
|
||
| if ROBOT_AVAILABLE: |
There was a problem hiding this comment.
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()}',
]| if not val: | ||
| return None | ||
| if isinstance(val, str): | ||
| return json.loads(val) |
There was a problem hiding this comment.
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 excSame issue exists in the Selenium PR.
| broad-except, | ||
| broad-exception-raised, | ||
| global-statement, | ||
| import-error, |
There was a problem hiding this comment.
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-errorOr scope it to the specific file in .pylintrc:
[MESSAGES CONTROL]
# Only for robot_library.py| */__pycache__/* | ||
| */site-packages/* | ||
| */.venv/* | ||
| percy/robot_library.py |
There was a problem hiding this comment.
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).
Summary
Adds native Robot Framework keywords for Percy visual testing using the Browser library (robotframework-browser / Playwright backend). Mirrors the same PR on percy-selenium-python (#220) for the Selenium backend.
What's added
percy/robot_library.py— Robot Framework library with keywords and Browser library adaptertests/test_robot_library.py— 12 testspercy/__init__.pydevelopment.txt— added robotframework and robotframework-browserKeywords
Key design decisions
import robotwrapped intry/except— zero impact when robotframework is not installedpercy.screenshot— playwright only loaded when keywords are called, not at import time_BrowserLibraryPageAdapterbridges Browser library keywords to the Playwright page interface thatpercy_snapshot()expectsImportErrorwith clear messageUsage
percy exec -- robot tests/Test plan
Related
🤖 Generated with Claude Code