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.

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

Query testsForStory and testsForStatus separately to avoid pagination #37

Merged

Conversation

ghengeveld
Copy link
Member

@ghengeveld ghengeveld commented Aug 17, 2023

Rather than fetching all data for all tests on the build, we only query for all data for the tests for the currently selected story, and additionally fetch limited data for up to 1000 (max page size) tests that are in progress, pending, denied, failed or broken. This way we avoid having to do any kind of pagination logic.

In the unusual case that >1000 tests have such a status this will break down, but this is acceptable because it's used only to display status dots in the Storybook sidebar.

Depends on https://github.com/chromaui/chromatic/pull/7628

📦 Published PR as canary version: 0.0.29--canary.37.4a8d155.0

✨ Test out this PR locally via:

npm install @chromaui/[email protected]
# or 
yarn add @chromaui/[email protected]

@linear
Copy link

linear bot commented Aug 17, 2023

@ghengeveld ghengeveld requested review from tmeasday, weeksling and thafryer and removed request for tmeasday August 17, 2023 17:11
Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

Looks like this PR should be telescoping on #36 (review) -- a bit hard to review as a result.

But the code changes to VisualTests.tsx look good. I'm a bit surprised there aren't more changes to the story data in VisualTests.stories.ts though?

@ghengeveld ghengeveld changed the base branch from main to tom/ap-3450-buildinfo-component-errors-if-build-has-errors August 18, 2023 11:06
@ghengeveld
Copy link
Member Author

@tmeasday You're right I forgot to commit+push some changes. I updated the base to telescope off your PR.

Copy link
Member

@tmeasday tmeasday left a comment

Choose a reason for hiding this comment

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

LGTM. One question about testing strategy we should address.

src/screens/VisualTests/VisualTests.tsx Show resolved Hide resolved
Base automatically changed from tom/ap-3450-buildinfo-component-errors-if-build-has-errors to main August 21, 2023 01:41
@ghengeveld ghengeveld merged commit c33ab36 into main Aug 22, 2023
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.

2 participants