feat(tests): marketing screenshot testbed via integration suite#2895
Merged
Conversation
Unifies marketing-asset generation with the existing Playwright integration tests rather than forking a parallel suite. Setting MARKETING_SCREENSHOTS=1 turns the new ``marketing_screenshot`` fixture in tests/conftest.py from a no-op into a capture step that emits @1x/@2x/@3x sibling PNGs under test-artifacts/marketing/ — matching the website's existing overview*.png convention. Seed data moves to tests/_seed_data.py so test_app.py and test_migrate_to_screenly.py render the same Office Space parody content (Initech / TPS / Lumbergh / Chotchkie's / Milton). Two new integration tests — test_home_renders_with_full_schedule and test_add_asset_modal_layers_over_full_schedule — exercise a populated asset table that the per-row smoke tests miss, and double as the ``home`` and ``add-asset`` marketing captures. The new .github/workflows/marketing-screenshots.yaml is workflow_dispatch only, re-runs the integration suite with the env var set, and uploads test-artifacts/marketing/ as a 90-day artifact. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in marketing screenshot pipeline on top of the Playwright integration suite, sharing seeded UI data across home-page and migration-wizard tests and adding a manual GitHub Actions workflow to produce screenshot artifacts.
Changes:
- Adds shared integration seed data for populated schedule and migration wizard scenarios.
- Consolidates Playwright viewport/launch fixtures and adds an opt-in
marketing_screenshotfixture. - Adds/updates integration tests and a manual workflow for marketing screenshot capture.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/conftest.py |
Centralizes Playwright browser configuration and adds marketing screenshot generation. |
tests/_seed_data.py |
Introduces shared sample asset data for integration tests. |
tests/test_app.py |
Uses shared seed data and adds screenshot-driving home/settings scenarios. |
tests/test_migrate_to_screenly.py |
Reuses shared wizard seed data and constants. |
.github/workflows/marketing-screenshots.yaml |
Adds manual workflow to run integration tests with screenshot capture enabled. |
Comments suppressed due to low confidence (1)
tests/test_app.py:356
- The modal layering regression described here is not actually asserted: the test only waits for the heading and then calls
marketing_screenshot, which is a no-op outside the manual screenshot run and performs no comparison. A z-index/backdrop regression could pass the default suite; add an automated check for the overlay/card geometry or visibility if this is meant to guard layering.
expect(page.get_by_role('heading', name='Add asset')).to_be_visible()
# full_page=False because the modal is position: fixed; Playwright's
# full-page mode would push the modal card off-frame and capture
# only the dimmed backdrop over the underlying page.
marketing_screenshot('add-asset', full_page=False)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- _seed_data.py: clarify that the module-level seed singletons capture their schedule window at import time (only home_seed_assets() is a factory); fix a stale comment that referenced the old WIZARD_LOCAL_VIDEO_BASENAME name. - conftest.py + workflow: docstrings + section comment now describe the actual artefact naming (<name>.png + <name>@2x.png + <name>@3x.png) instead of an inaccurate "@1x/@2x/@3x" shorthand. Matches the existing website/assets/images/overview*.png convention, which is what Hugo's image-set picker resolves. - test_app.py: the two new tests now assert concrete layout properties (each row's name cell has nonzero width; the add-asset modal card has a real bounding box inside the viewport) so a layout regression fails CI without requiring MARKETING_SCREENSHOTS=1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _seed_data.py: home_seed_assets() now wraps the module-level singletons through a new _with_fresh_window() helper so every dict in the returned list carries a window computed at call time. Honours the "fresh schedule on each call" contract the docstring promises; a time-travelling test no longer gets the import-time freeze for the first / second / last rows. - test_app.py: test_home_renders_with_full_schedule now asserts each row's right edge stays inside the viewport AND that the rightmost action button (Delete) is visible + clickable — a layout regression that pushes the action cluster off-screen now fails CI instead of being caught only by the marketing capture. - test_app.py: test_add_asset_modal_layers_over_full_schedule now calls document.elementFromPoint() at the modal centre and asserts the topmost element lives inside .modal-card. A z-index regression that leaves an asset row floating above the modal is now caught at the assertion layer instead of relying on a visual diff. - marketing-screenshots.yaml: the pytest step is wrapped in nick-fields/retry@v4 (timeout 8 min, 3 attempts) to mirror the existing test-runner.yml integration step. A transient Playwright / SQLite WAL flake no longer aborts the manual artifact pipeline. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- _seed_data.py: docstring no longer claims the parody avoids
registered/copyrighted material — every name (Initech, Lumbergh,
Chotchkie's, Milton, Peter Gibbons) is an Office Space reference.
Now describes the content honestly as parody/fair-use of character
and company names; the file remains the single swap point for
downstream uses that need fully generic naming.
- test_migrate_to_screenly.py: the failure-flow test no longer
hardcodes abc1.mp4. Both the mocked backend error and the assertion
reuse WIZARD_VIDEO_BASENAME, so a future seed rename only has to
touch one place. The wizard test's docstring also points at the
constant instead of the stale literal.
- test_app.py (home): viewport-bound assertion on the Delete button
now uses the same +1px tolerance as the row-edge check — Playwright
reports floating-point CSS pixels and the strict comparison flaked
intermittently under the 3× marketing device scale.
- test_app.py (modal): wait for Element.getAnimations({subtree:true})
to settle before asserting the modal card's bbox. The .modal-card
has a 220ms modal-in keyframe animation; without explicitly waiting,
the marketing screenshot can land mid-fade. Same +1px tolerance
applied to the modal viewport-bounds check.
- marketing-screenshots.yaml: explicit `permissions: contents: read`
block so a workflow_dispatch run against an arbitrary ref cannot
inherit broader repo write scopes. The retry command now clears
test-artifacts/marketing before each attempt so an earlier
partial-success can't stamp stale screenshots onto the artifact
upload if a later attempt aborts before the capture tests run.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
tests/test_app.py:363
- The assertion only verifies that the Delete button is visible and within the viewport; it does not prove the button is actionable/clickable as the test comment describes. If an overlay or CSS pointer-events regression blocks the action cluster, this test would still pass.
delete_btn = row.locator('button[title="Delete"]')
expect(delete_btn).to_be_visible()
- conftest.py: new session-scoped autouse _reset_marketing_dir fixture clears test-artifacts/marketing/ from inside the container when MARKETING_SCREENSHOTS=1. Replaces the workflow's host-side rm -rf, which failed with permission-denied on a retry because the bind-mounted artefact dir is owned by the root-running anthias-test container. Verified locally: a STALE.png planted before the run is removed before any test executes, then all 9 fresh captures land. - test_app.py (home): adds an explicit .drag-handle visibility + width check on each enabled row. The "drag handle stays reachable" promise in the docstring is now actually asserted, not just implied by the row/Delete-button checks. - test_migrate_to_screenly.py: the mocked migration failure on asset a2 (the URL-backed webpage seed) now uses a URL-fetch error string referencing WIZARD_WEBPAGE_URL — the previous "File not found on device" wording with WIZARD_VIDEO_BASENAME was internally inconsistent (would only make sense for a1/a3, the local-file seeds) and could hide a row-association mistake. - marketing-screenshots.yaml: drops the host-side rm -rf test-artifacts/marketing (now handled inside the container by the fixture above), updates the comment to point at the fixture. PR body also updated to describe the artifact set precisely (<name>.png + <name>@2x.png + <name>@3x.png — no @1x suffix on the base image; matches the existing website overview*.png convention). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds a marketing-asset generation pipeline that piggybacks on the existing Playwright integration suite instead of forking a parallel one.
tests/conftest.py— newmarketing_screenshotfixture. No-op by default; whenMARKETING_SCREENSHOTS=1is set, the browser context renders atdevice_scale_factor=3and the fixture writes a base<name>.pngplus retina<name>@2x.pngand<name>@3x.pngsiblings totest-artifacts/marketing/. The naming convention matches the website's existingoverview.png/overview@2x.png/overview@3x.pngset, so Hugo's image-set picker resolves the new files without extra config. A session-scoped autouse fixture clears that directory once per run (inside the container, so retries on the workflow side don't hit permission-denied on root-owned bind-mount files). Browser viewport and--no-sandboxoverrides are also consolidated here (previously duplicated intest_app.pyandtest_migrate_to_screenly.py).tests/_seed_data.py— single source of truth for the asset rows the integration tests seed. Office Space-themed parody content (Initech / TPS reports / Lumbergh memos / Chotchkie's / Milton / Peter Gibbons) — recognisable on purpose so the marketing captures read as real digital signage rather than placeholder text; standard parody/fair-use treatment for character and company names. Single swap point if a downstream use needs fully generic naming.tests/test_app.py— two new integration tests (test_home_renders_with_full_schedule,test_add_asset_modal_layers_over_full_schedule) exercise a populated asset table that the per-row smoke tests don't cover and double as thehomeandadd-assetmarketing captures. They assert concrete layout properties (row right edge inside the viewport, drag handle visible on each enabled row, Delete button within viewport, modal animation settled, modal.modal-cardis the topmost element at its centre viadocument.elementFromPoint) so layout regressions fail CI without requiringMARKETING_SCREENSHOTS=1.test_settings_page_rendersalso captures.tests/test_migrate_to_screenly.py— uses the shared wizard seeds and constants; previously-hardcoded basename / URL assertions reference the constants now..github/workflows/marketing-screenshots.yaml—workflow_dispatch-triggered, withpermissions: contents: readso a dispatched run on an arbitrary ref can't inherit broader write scopes. Builds the test stack, re-runs the integration suite withMARKETING_SCREENSHOTS=1, uploadstest-artifacts/marketing/as a 90-day artifact. The pytest step is wrapped innick-fields/retry@v4(same retry pattern astest-runner.yml) so a transient Playwright/SQLite flake doesn't abort the manual artifact pipeline.Captured artifact set (per screen
<name>):<name>.png(1400-wide base, the canonical URL),<name>@2x.png(2800-wide),<name>@3x.png(4200-wide). 1× has no@1xsuffix — matches the existingoverview*.pngset inwebsite/assets/images/.CI parity verified locally: all integration tests pass with the env var set, 9 PNGs produced (3 screens × 3 scales).
Checklist