Skip to content

♻️ Post core refactor tweaks#373

Merged
wwilsman merged 2 commits intomasterfrom
ww/post-core-refactor-tweaks
Jun 18, 2021
Merged

♻️ Post core refactor tweaks#373
wwilsman merged 2 commits intomasterfrom
ww/post-core-refactor-tweaks

Conversation

@wwilsman
Copy link
Copy Markdown
Contributor

What is this?

Following #369 and subsequently #372, I've since found a couple other minor thing that need to be tweaked.

  • In agent, up to 10 browser pages were pooled to be running at once. It was lowered in the CLI since moving to a proper queue with a concurrency. Now that queue performance has gotten much better, let's up it to 10 again. Even in previous versions of the CLI up to 10 browser pages could be running when using the capture method, so being able to set this more accurately is a win from ♻️ Refactor core discovery queue #369.

  • Rename queue.pause() to queue.stop() (🚲 🏠). Pause implies that there is still more to run, while stop implies that it won't run regardless if there is more to run. Stop also feels more complimentary to run().

  • Add ability for close() to abort. This will stop and clear the queue while closing it, exactly what is really needed by the percy.close() method.

  • Abort queue when force stopping the Percy instance. Rather than clear the queues which will be closed, when force stopping we can utilize percy.close() to wholesale abort any currently queued tasks.

  • Catch sync errors when running queued tasks. While wrapping the dequeued callback in a resolve served to catch async errors, sync errors could still happen which would cause pending tasks to never be cleared.

  • While testing Storybook and sending a SIGTERM event, I noticed that there were occasional unhandled promise rejections caused by page/browser closed errors bubbling from random event callbacks. The closed errors always bubble and get raised from other methods, so in these few unhandled rejections, we can simply log a debug message to handle it. Coverage was skipped for these catches since they all only occur in specific race conditions.

@wwilsman wwilsman added 🐛 bug Something isn't working ✨ enhancement New feature or request labels Jun 17, 2021
@wwilsman wwilsman requested a review from Robdel12 June 17, 2021 21:57
@wwilsman wwilsman force-pushed the ww/post-core-refactor-tweaks branch 2 times, most recently from 4afcd6b to 686a4f0 Compare June 17, 2021 22:11
Copy link
Copy Markdown
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.

No need for 🚲 🏠 , I dig it. Nice 🏁

wwilsman added 2 commits June 18, 2021 13:28
- Default concurrency to 10
- Rename pause() to stop()
- Add ability for close() to abort
- Abort queue when closing the Percy instance
- Catch sync errors when running queued tasks
While testing Storybook and sending a SIGTERM event, I noticed that there were occasional unhandled
promise rejections caused by page/browser closed errors bubbling from random event callbacks. The
closed errors always bubble and get raised from other methods, so in these few unhandled rejections,
we can simply log a debug message to handle it.
@wwilsman wwilsman force-pushed the ww/post-core-refactor-tweaks branch from 686a4f0 to 1a4acea Compare June 18, 2021 18:29
@wwilsman wwilsman merged commit 6ae336c into master Jun 18, 2021
@wwilsman wwilsman deleted the ww/post-core-refactor-tweaks branch June 18, 2021 18:38
samarsault pushed a commit that referenced this pull request Mar 3, 2023
Bumps [@cypress/code-coverage](https://github.com/cypress-io/code-coverage) from 3.9.7 to 3.9.8.
- [Release notes](https://github.com/cypress-io/code-coverage/releases)
- [Commits](cypress-io/code-coverage@v3.9.7...v3.9.8)

---
updated-dependencies:
- dependency-name: "@cypress/code-coverage"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛 bug Something isn't working ✨ enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants