Skip to content

Conversation

@ssncferreira
Copy link
Contributor

@ssncferreira ssncferreira commented Nov 17, 2025

Problem

The OrgsSortedAlphabetically test from OrganizationSidebarView.stories.tsx was failing on Chromatic. Test logic was attempting to verify organization sorting order programmatically. This was identified in the Chromatic build of PR: https://www.chromatic.com/build?appId=624de63c6aacee003aa84340&number=26015

After fixing this test, two additional tests started failing:

  • VanillaJavascriptError from GlobalErrorBoundary.stories.tsx: Test was making incorrect assertions about stack trace content
  • MarkAllNotificationsAsReadError from NotificationsInbox.stories.tsx: Test was flaky due to competing WebSocket error messages

Solution

These are Chromatic snapshot tests, so implementation details (like sorting order or exact error message content) are already validated by the visual snapshots. Programmatic assertions were causing flakiness and are redundant.

@ssncferreira ssncferreira force-pushed the ssncferreira/fix-site-orgs-sorted-alphabetically branch from efeb716 to 83c9847 Compare November 17, 2025 18:06
@ssncferreira ssncferreira changed the title fix(site): OrgsSortedAlphabetically test fix(site): fix OrgsSortedAlphabetically test Nov 17, 2025
@ssncferreira ssncferreira force-pushed the ssncferreira/fix-site-orgs-sorted-alphabetically branch from 83c9847 to 0bb3de1 Compare November 17, 2025 18:17
@ssncferreira ssncferreira marked this pull request as ready for review November 17, 2025 18:33
@ssncferreira ssncferreira requested a review from aslilac November 17, 2025 18:33
@Emyrk
Copy link
Member

Emyrk commented Nov 17, 2025

I am also seeing Vanilla Javascript Error fail, and not sure exactly what is going on.

Copy link
Member

@aslilac aslilac left a comment

Choose a reason for hiding this comment

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

I think a better solution might be to remove this logic all together. stories are all snapshot tests, so we already know that the names are present and in the right order because they're in the screenshot. if this logic is flakey, then lets just rely on the snapshot instead.

@ssncferreira
Copy link
Contributor Author

I think a better solution might be to remove this logic all together. stories are all snapshot tests, so we already know that the names are present and in the right order because they're in the screenshot. if this logic is flakey, then lets just rely on the snapshot instead.

@aslilac I think the test might actually make sense. It validates that the organizations are listed in the correct order: first the active organization (Omega org), then the remaining organizations alphabetically.

AFAIU, if a bug is introduced where the order of the orgs changes, Chromatic will detect those visual changes in the screenshot, but it won't fail automatically, a reviewer would need to catch it manually. The programmatic assertion ensures the sorting logic works correctly and fails immediately if it breaks.

This new logic fixes the flakiness of the test by querying the .truncate span directly instead of relying on the full textContent (which included the avatar fallback text). The other failing tests in the CI are unrelated to this test and are failing on other builds as well.

@aslilac aslilac self-requested a review November 19, 2025 17:05
@ssncferreira
Copy link
Contributor Author

After discussing with @aslilac on the best practices around Chromatic tests, the focus should be on visual validation through snapshots rather than programmatic assertions. The snapshot will catch visual changes resulting from any programmatic changes during the Chromatic review process. The programmatic assertions on these tests were redundant and introduced flakiness, so removing them makes the tests more stable while still catching issues through visual diffs.

@ssncferreira ssncferreira changed the title fix(site): fix OrgsSortedAlphabetically test fix(site): fix flaky Chromatic tests Nov 19, 2025
Copy link
Member

@aslilac aslilac left a comment

Choose a reason for hiding this comment

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

thank you!

@ssncferreira ssncferreira merged commit 17edeea into main Nov 19, 2025
35 checks passed
@ssncferreira ssncferreira deleted the ssncferreira/fix-site-orgs-sorted-alphabetically branch November 19, 2025 18:17
@github-actions github-actions bot locked and limited conversation to collaborators Nov 19, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants