Skip to content

Phase 12: cross-app visual/DOM/CSS/functional parity suite (feature)#323

Open
devin-ai-integration[bot] wants to merge 1 commit into
devin-06.09.2026-vibha/react-migration-phase8from
devin/1781112978-phase12-visual-parity
Open

Phase 12: cross-app visual/DOM/CSS/functional parity suite (feature)#323
devin-ai-integration[bot] wants to merge 1 commit into
devin-06.09.2026-vibha/react-migration-phase8from
devin/1781112978-phase12-visual-parity

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 10, 2026

Copy link
Copy Markdown

Summary

Adds Phase 12: Visual Parity Validation — a Playwright harness that measures whether the React migration renders identically to the original Angular app. The same specs run twice (a Playwright project per app, both pointed at deterministic mocked HN data) so any drift surfaces as a failing assertion in one app but not the other. A pixelmatch script then diffs the captured screenshots and emits an HTML report.

This PR delivers the measuring harness, not migration fixes. It currently reports that DOM / CSS / functional parity is fully achieved, while pixel parity is not yet met on some pages (see "Current parity status").

What's included

  • playwright.config.tsangular (:4200) and react (:5173) projects; desktop 1280×720 and mobile (iPhone X) 375×812.
  • tests/fixtures.ts + tests/helpers.ts — mock every node-hnapi request with identical bytes for both apps; helpers for theme switching, settings, screenshot capture.
  • tests/visual-parity.spec.ts (12b) — full-page screenshots across pages × 3 themes × 2 viewports.
  • tests/dom-parity.spec.ts (12d) — structural assertions (30 feed items, comment subtree counts, settings inputs, theme class, …).
  • tests/style-parity.spec.ts (12e) — computed-style assertions (header/wrapper bg per theme, .body-cover fixed overlay, title font-size follows settings, …).
  • tests/functional-parity.spec.ts (12f) — behaviour (settings persistence, pagination, nav, comment collapse, external-link target/rel, mobile back button, dark-mode auto-detect).
  • tests/compare-screenshots.ts (12c) — pixelmatch diff → screenshots/diffs/ + screenshots/report.html; classifies PASS <0.5%, WARN ≤2%, FAIL >2%; exits non-zero on any FAIL.
  • npm scripts (12g): test:visual:angular, test:visual:react, test:visual:compare, test:parity.
  • .github/workflows/visual-parity.yml (12h): checks out this branch + master (Angular), serves both, runs npm run test:parity, uploads screenshots/ + playwright-report/.

Notes for reviewers

  • The comparison runner uses tsx instead of ts-node (the plan's suggestion): pixelmatch@7 is ESM-only and tsx runs it without loader flags.
  • Test files are excluded from tsconfig.json, so they don't affect the existing lint/build.
  • Style/DOM parity specs assert canonical expected values captured from Angular (e.g. header rgb(185, 43, 39) / rgb(38, 50, 56) / rgb(0, 0, 0) per theme); both apps must resolve to those exact values to pass.

Current parity status

Running npm run test:parity against both running apps:

  • Functional / DOM / CSS specs: 7/7 pass on each app. Computed styles are byte-identical across all three themes.
  • Visual screenshots: 3 PASS / 21 WARN / 16 FAIL. The FAILs are dominated by a real mobile layout gap: the React mobile feed is shorter than Angular (e.g. /news/1 mobile: Angular 2718px vs React 2122px), so full-page diffs accumulate a large vertical offset. A few desktop pages (/jobs/1, settings popup) sit just above the 2% threshold. Desktop heights otherwise match.

Per spec 12h the workflow fails while any FAIL remains — this is the intended gate signalling the migration is not yet at full pixel parity. Closing those gaps is follow-up migration work, not part of this harness PR.

Link to Devin session: https://app.devin.ai/sessions/8415723ddaa94105b9b8f13eb80a2f84
Requested by: @lburgers


Devin Review

Status Commit
🟢 Reviewed bad954c
Open in Devin Review (Staging)

Add a Playwright-based parity harness comparing the Angular and React apps:
- playwright.config.ts with angular/react projects + desktop/mobile viewports
- tests/visual-parity.spec.ts: full-page screenshots across pages/themes/viewports
- tests/dom-parity.spec.ts: structural assertions
- tests/style-parity.spec.ts: computed-style assertions
- tests/functional-parity.spec.ts: behavioural assertions
- tests/compare-screenshots.ts: pixelmatch diff + HTML report
- npm scripts (test:visual:*, test:parity) and visual-parity CI workflow

Shared fixtures/helpers mock the HN API so both apps render identical data.

Co-Authored-By: Lukas Burger <lukaskburger@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration

Copy link
Copy Markdown
Author

Phase 12 status + note on the red visual-parity check

The harness runs end-to-end in CI (both apps install, serve, and get exercised):

  • DOM / CSS / functional parity: full pass — 59/59 Playwright tests pass on each app (angular and react). Computed styles are byte-identical across all 3 themes.
  • Visual pixel parity: not yet met — the compare-screenshots.ts gate reports diffs, so the visual-parity job exits non-zero by design (spec 12h: "fail the workflow if any FAIL"). lint + Snyk pass.

Main diff drivers the report surfaces:

  • Mobile layout (~30%): React's mobile feed is shorter than Angular's (e.g. /news/1 mobile is 2718px tall in Angular vs 2122px in React), so full-page diffs accumulate a vertical offset.
  • Desktop (~2–3.5%): /jobs/1 and the settings popup sit just over the 2% threshold; remaining desktop diffs are largely font anti-aliasing between CI and local.

So the red check is the harness correctly signalling the migration isn't at full pixel parity yet — not a bug in the suite.

How should the gate behave?

  1. Keep it strict as specified (stays red until the React app reaches pixel parity), or
  2. Make visual-parity non-blocking (still uploads the report artifact, but doesn't fail CI), or
  3. I start closing the actual React parity gaps (mobile spacing, jobs header, settings popup) as follow-up.

(Note: my send_to_user channel is erroring right now, so I'm posting this status here.)

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 5 additional findings.

Open in Devin Review

@staging-devin-ai-integration staging-devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review (Staging)
Debug

Playground

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant