Skip to content

fix: preserve adaptive tab baselines in screenshot updates #227

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rdlabo merged 3 commits into from
Sep 27, 2026
Merged

fix: preserve adaptive tab baselines in screenshot updates #227

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
77b73dc
fix: preserve adaptive tab screenshots in snapshot updates
rdlabo Sep 26, 2026
d001480
fix: scope required screenshots to the PR base branch
rdlabo Sep 26, 2026
db6497e
Merge remote-tracking branch 'origin/main' into HEAD
rdlabo Sep 27, 2026
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
25 changes: 19 additions & 6 deletions .github/workflows/update-screenshots.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,9 @@ jobs:
uses: actions/upload-artifact@v5
with:
name: updated-screenshots
path: demo/e2e/screenshot.spec.ts-snapshots/*.png
path: |
demo/e2e/screenshot.spec.ts-snapshots/*.png
demo/e2e/adaptive-tab-bar.spec.ts-snapshots/*.png
if-no-files-found: error

commit-screenshots:
Expand Down Expand Up @@ -134,7 +136,9 @@ jobs:
script: |
const fs = require('fs');
const path = require('path');
const snapshotPrefix = 'demo/e2e/screenshot.spec.ts-snapshots';
const snapshotPrefix = 'demo/e2e';
const snapshotDirectories = ['screenshot.spec.ts-snapshots'];
if (process.env.EXPECTED_BASE_REF === 'main') snapshotDirectories.push('adaptive-tab-bar.spec.ts-snapshots');
const artifactDirectory = 'updated-screenshots';
const expectedHeadSha = process.env.EXPECTED_HEAD_SHA;
const expectedHeadRef = process.env.EXPECTED_HEAD_REF;
Expand All @@ -158,11 +162,20 @@ jobs:
return;
}

const filenames = fs.readdirSync(artifactDirectory).sort();
if (filenames.length === 0 || filenames.some((filename) => !/^[A-Za-z0-9._-]+\.png$/.test(filename))) {
core.setFailed('The screenshot artifact contains an unexpected file.');
const directories = fs.readdirSync(artifactDirectory).sort();
if (directories.join('\n') !== [...snapshotDirectories].sort().join('\n')) {
core.setFailed('The screenshot artifact contains missing or unexpected directories.');
return;
Comment on lines +165 to 168

@devin-ai-integration devin-ai-integration Bot Sep 26, 2026

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.

🔴 iOS 26 向け PR のスクリーンショット更新が失敗する

ios26 向け PR では、対象ブランチの更新スクリプトが通常の画像だけを生成します。adaptive-tab のディレクトリも必須とするため、iOS 26 の画像をコミットできません。

Learn more

このワークフローは main と ios26 の PR を受け付け、PR の対象ブランチから更新手順を読み込みます（更新手順の選択、スクリプトの実行）。ios26 ブランチの更新手順は screenshot.spec.ts だけを実行し、同ブランチには adaptive-tab-bar.spec.ts がありません。そのため、通常の ios26 PR の成果物には通常の画像ディレクトリしか含まれません。新しい検証が成果物内に両方のディレクトリを要求するので、更新後のコミット処理が必ず失敗します。

Example: ios26 を対象とし、通常のスクリーンショットだけを持つ PR で /update-screenshots を実行すると、更新手順は成功しても成果物には screenshot.spec.ts-snapshots しかありません。コミット処理は「missing or unexpected directories」で終了します。

Recommended fix: PR の対象ブランチで実行するテストと一致するディレクトリだけを必須にしてください。削除対象も、その実行で生成したディレクトリに限定してください。

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}
const filenames = [];
for (const directory of directories) {
const files = fs.readdirSync(path.join(artifactDirectory, directory)).sort();
if (files.length === 0 || files.some((filename) => !/^[A-Za-z0-9._-]+\.png$/.test(filename))) {
core.setFailed('The screenshot artifact contains missing or unexpected files.');
return;
}
filenames.push(...files.map((filename) => `${directory}/${filename}`));
}

const commit = await github.rest.git.getCommit({
owner: context.repo.owner,
Expand All @@ -181,7 +194,7 @@ jobs:
const tree = currentTree.data.tree
.filter((entry) =>
entry.type === 'blob' &&
entry.path.startsWith(`${snapshotPrefix}/`) &&
snapshotDirectories.some((directory) => entry.path.startsWith(`${snapshotPrefix}/${directory}/`)) &&
!uploadedPaths.has(entry.path)
)
.map((entry) => ({ path: entry.path, mode: '100644', type: 'blob', sha: null }));
Expand Down
Loading