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

🐛 Add missing tests and miscellaneous fixes #611

Merged
merged 9 commits into from
Jul 21, 2022

Conversation

wwilsman
Copy link
Contributor

@wwilsman wwilsman commented Jul 20, 2022

What is this?

With #592, it was discovered that this SDK was actually missing quite a bit of tests. Since it was out of scope for that PR, we opted to address this in another PR (this PR).

With the addition of new tests to achieve 100% coverage, a few small bugs inevitably came up and were fixed.

  • When splitting snapshots into shards with --shard-count, providing the --partial flag will now always set the parallel total environment variable to -1 unless already set. Previously, the --shard-count flag did not consider the --partial flag when setting the environment variable.

  • When creating the story URL and settings params with the native URLSearchParams API, it will encode any provided values. However, args and globals params are already encoded, therefore resulting in double encoding of those params. This was fixed with string concatenation for already encoded values, while still using the URLSearchParams API for provided query params.

  • Both the storybook and storybook:start commands have similar try-catch blocks to start Percy and call the same utility function with a different baseUrl. The try-catch repetition was moved into the utility function which now accepts a callback that is called during the finally block.

  • The deferUploads option was changed to the new delayUploads option so snapshots get sent to the API as soon as possible rather than all at once at the end of the process. Uploads are still delayed rather than be left eager so empty builds are not created when failing to find any snapshots or Storybook build.

  • A new event listener was added to the end of story navigation, which will wait until Storybook emits a storyRendered event before taking a snapshot. While network idle is always awaited on, this might help in situations when no network requests occur, which may cause the snapshot to be taken too early when no other wait options are provided.

@wwilsman wwilsman added the 🐛 bug Something isn't working label Jul 20, 2022
@wwilsman wwilsman requested a review from Robdel12 July 20, 2022 22:00
@wwilsman wwilsman marked this pull request as ready for review July 20, 2022 22:11
@wwilsman wwilsman enabled auto-merge (squash) July 20, 2022 22:11
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

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

🏁 Pretty uncharacteristic for us to miss coverage like this but very happy to have this SDK back up to 100%.

channel.emit('updateGlobals', { globals: {} });
channel.emit('setCurrentStory', { storyId: story.id });
channel.emit('updateQueryParams', { ...story.queryParams });
channel.emit('updateGlobals', { globals: story.globals });
channel.emit('updateStoryArgs', { storyId: story.id, updatedArgs: story.args });

// resolve when rendered, reject on any other renderer event
return new Promise((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Slicccckk

@wwilsman wwilsman merged commit 84828c2 into master Jul 21, 2022
@wwilsman wwilsman deleted the ww/missing-tests-and-misc-fixes branch July 21, 2022 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants