Skip to content
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

Jump to bottom

chore: Fix Storybook on CI #29162

Merged
merged 12 commits into from
Feb 25, 2025
Merged

chore: Fix Storybook on CI #29162

merged 12 commits into from
Feb 25, 2025

Conversation

rafaeelaudibert
Copy link
Member

Problem

Storybook is broken

Changes

Fix it

@rafaeelaudibert rafaeelaudibert changed the title remove commit later chore: Fix Storybook on CI Feb 25, 2025
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Feb 25, 2025 •
edited
Loading

Size Change: 0 B

Total Size: 9.72 MB

ℹ️ View Unchanged
Filename Size
frontend/dist/toolbar.js 9.72 MB

compressed-size-action

@rafaeelaudibert rafaeelaudibert force-pushed the fix-storybook-in-ci branch 3 times, most recently from 037d456 to 7e0449e Compare February 25, 2025 05:00
mariusandra
mariusandra approved these changes Feb 25, 2025
View reviewed changes
Copy link
Collaborator

@mariusandra mariusandra left a comment

Choose a reason for hiding this comment

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

Overall, yes, thank you! However any way to not have 1128 changed files :eyes_shaking: :D

@rafaeelaudibert
Copy link
Member Author

rafaeelaudibert commented Feb 25, 2025 •
edited
Loading

@mariusandra Oh yeah, it's better now, that was just me banging my head at the wall at 2 AM last night, it's better now
image

@rafaeelaudibert
Copy link
Member Author

@mariusandra Well, that didn't work, now no files have been committed... I will ping you once this is actually working

@rafaeelaudibert rafaeelaudibert force-pushed the fix-storybook-in-ci branch 2 times, most recently from e091d9c to 1cd800b Compare February 25, 2025 14:33
@PostHog PostHog deleted a comment from posthog-bot Feb 25, 2025
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

71 snapshot changes in total. 0 added, 71 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@rafaeelaudibert rafaeelaudibert marked this pull request as ready for review February 25, 2025 15:51
@rafaeelaudibert
Copy link
Member Author

@mariusandra Working now, will merge it and let teams know about the regressions that this PR found

@rafaeelaudibert rafaeelaudibert enabled auto-merge (squash) February 25, 2025 15:51
greptile-apps[bot]
greptile-apps bot reviewed Feb 25, 2025
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR fixes Storybook in CI by improving snapshot handling and server reliability.

  • Added absolute path resolution for snapshots in common/storybook/.storybook/test-runner.ts using path.resolve() to ensure consistent snapshot locations
  • Refactored snapshot functions into takeSnapshotWithTheme and doTakeSnapshotWithTheme for better code organization
  • Reduced wait timeout between snapshots from 2000ms to 100ms for faster test execution
  • Implemented robust server startup process in .github/workflows/storybook-chromatic.yml with retry logic and health checks
  • Added detailed server diagnostics for troubleshooting failed CI runs

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile

mariusandra
mariusandra reviewed Feb 25, 2025
View reviewed changes
frontend/__snapshots__/components-playerinspector--default--dark.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look good

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, some don't look good at all, so that's why I'll tag teams in #tell-posthog-anything once this is finally merged, just getting it to stop to flap

@PostHog PostHog deleted a comment from posthog-bot Feb 25, 2025
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

  • chromium: 0 added, 3 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@rafaeelaudibert rafaeelaudibert force-pushed the fix-storybook-in-ci branch 2 times, most recently from cf035d2 to 2029dcb Compare February 25, 2025 16:30
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

19 snapshot changes in total. 0 added, 19 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@rafaeelaudibert
fix: Reinstate snapshots on CI
b1864d7 
Our paths were all messed up, let's fix that. I've also added some better debugging tooling to the job which should hopefully make debugging this easier in the future
@rafaeelaudibert rafaeelaudibert merged commit 0b34335 into master Feb 25, 2025
99 checks passed
@rafaeelaudibert rafaeelaudibert deleted the fix-storybook-in-ci branch February 25, 2025 20:53
pauldambra pushed a commit that referenced this pull request Feb 25, 2025
@rafaeelaudibert @github-actions @pauldambra
chore: Fix Storybook on CI (#29162)
359370f 
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mariusandra mariusandra mariusandra approved these changes

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rafaeelaudibert @posthog-bot @mariusandra