Skip to content

Use local avatar image for VRT instead of external URL - #7795

Merged
siddharthkp merged 1 commit into
mainfrom
use-local-avatar-image-for-vrt
May 11, 2026
Merged

Use local avatar image for VRT instead of external URL#7795
siddharthkp merged 1 commit into
mainfrom
use-local-avatar-image-for-vrt

Conversation

@siddharthkp

@siddharthkp siddharthkp commented Apr 28, 2026

Copy link
Copy Markdown
Member

Problem

Fetching from the external URL failed a few times (possibly due to downtime or service degradation), causing VRT tests because of a fallback avatar. These failures were confusing to debug since the root cause was not immediately obvious

Solution

We already change the image from user avatars to primer avatar. On top of that, using a local image makes the tests more reliable and independent of network requests

Added update snapshots and there are no diffs 🎉

@siddharthkp siddharthkp added the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 28, 2026
@changeset-bot

This comment was marked as resolved.

@github-actions github-actions Bot added staff Author is a staff member integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm labels Apr 28, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⚠️ Action required

👋 Hi, this pull request contains changes to the source code that github/github-ui depends on. If you are GitHub staff, test these changes with github/github-ui using the integration workflow. Check the integration testing docs for step-by-step instructions. Or, apply the integration-tests: skipped manually label to skip these checks.

To publish a canary release for integration testing, apply the Canary Release label to this PR.

@github-actions
github-actions Bot temporarily deployed to storybook-preview-7795 April 28, 2026 19:29 Inactive
@github-actions github-actions Bot removed the update snapshots 🤖 Command that updates VRT snapshots on the pull request label Apr 28, 2026
@siddharthkp siddharthkp added skip changeset This change does not need a changelog integration-tests: skipped manually Changes in this PR do not require an integration test labels Apr 29, 2026
@siddharthkp
siddharthkp marked this pull request as ready for review April 29, 2026 08:17
@siddharthkp
siddharthkp requested a review from a team as a code owner April 29, 2026 08:17

Copilot AI left a comment

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.

Pull request overview

This PR improves VRT reliability by avoiding flaky external avatar image fetches during Storybook-driven Playwright screenshots.

Changes:

  • Adds a local mock-avatar.png asset under packages/react/static/.
  • Configures the React Storybook to serve packages/react/static via staticDirs.
  • Updates the Playwright Storybook visit helper to fulfill https://github.com/*.png avatar requests using the local image instead of redirecting to an external URL.
Show a summary per file
File Description
packages/react/static/mock-avatar.png Adds a local PNG to use as a stable avatar image in tests.
packages/react/.storybook/main.ts Serves ../static assets from Storybook to enable local static asset usage.
e2e/test-helpers/storybook.ts Switches avatar request mocking from external redirect to local route.fulfill({path}).

Copilot's findings

  • Files reviewed: 2/3 changed files
  • Comments generated: 1

Comment on lines 65 to 68
/** Mock live avatar urls to make them stable for visual diffing (vrt) */
await page.route('https://github.com/*.png', async route => {
await route.continue({url: 'https://github.com/primer.png'})
await route.fulfill({path: AVATAR_IMAGE_PATH})
})

Copilot AI Apr 29, 2026

Copy link

Choose a reason for hiding this comment

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

The route mock only matches https://github.com/*.png, but several VRT-covered stories still request avatars from https://avatars.githubusercontent.com/... (e.g. packages/react/src/Avatar/Avatar.stories.tsx). Those requests will still hit the network and can still flake, which undermines the goal of making VRT independent of external avatar fetches. Consider adding an additional page.route for https://avatars.githubusercontent.com/** (and any other avatar hosts you use) to fulfill with the same local image, or broadening the existing matcher accordingly.

Copilot uses AI. Check for mistakes.
@siddharthkp
siddharthkp enabled auto-merge April 30, 2026 07:25
@siddharthkp
siddharthkp disabled auto-merge May 1, 2026 09:10
@siddharthkp
siddharthkp enabled auto-merge May 1, 2026 09:10
@primer
primer Bot disabled auto-merge May 4, 2026 16:32
@primer
primer Bot added this pull request to the merge queue May 4, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 4, 2026
@siddharthkp
siddharthkp enabled auto-merge May 4, 2026 18:25
@primer
primer Bot disabled auto-merge May 6, 2026 20:01
@primer
primer Bot added this pull request to the merge queue May 6, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 6, 2026
@siddharthkp
siddharthkp added this pull request to the merge queue May 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
@siddharthkp
siddharthkp added this pull request to the merge queue May 7, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 7, 2026
@siddharthkp
siddharthkp added this pull request to the merge queue May 11, 2026
Merged via the queue into main with commit 90cb7b6 May 11, 2026
82 of 85 checks passed
@siddharthkp
siddharthkp deleted the use-local-avatar-image-for-vrt branch May 11, 2026 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration-tests: recommended This change needs to be tested for breaking changes. See https://arc.net/l/quote/tdmpakpm integration-tests: skipped manually Changes in this PR do not require an integration test skip changeset This change does not need a changelog staff Author is a staff member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants