-
Notifications
You must be signed in to change notification settings - Fork 2
✨ Adding support for ignoreCanvasSerializationErrors #648
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
✨ Adding support for ignoreCanvasSerializationErrors #648
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for making canvas serialization fail-safe by introducing an ignoreCanvasSerializationErrors
option that can be configured either at the snapshot level or globally in the Percy configuration.
- Introduces a new helper function
ignoreCanvasSerializationErrors
that resolves the option value from either snapshot options or global config - Updates the DOM serialization process to pass the
ignoreCanvasSerializationErrors
flag to the PercyDOM.serialize method - Adds comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
index.js | Adds the ignoreCanvasSerializationErrors helper function and updates DOM serialization to use it |
test/index.test.mjs | Adds comprehensive test coverage for the new canvas serialization error handling functionality |
Comments suppressed due to low confidence (3)
test/index.test.mjs:3
- The import statement adds
ignoreCanvasSerializationErrors
but there are no integration tests verifying thatpercySnapshot
properly uses this function internally. Consider adding a test that verifies the integration betweenpercySnapshot
andignoreCanvasSerializationErrors
.
import percySnapshot, { ignoreCanvasSerializationErrors } from '../index.js';
test/index.test.mjs:609
- This test doesn't verify the scenario where
options.ignoreCanvasSerializationErrors
is explicitlyundefined
vs when the property doesn't exist. The function uses optional chaining, so both cases should be tested to ensure proper fallback behavior.
it('should fall back to utils.percy.config.snapshot.ignoreCanvasSerializationErrors when options value is undefined', () => {
test/index.test.mjs:623
- This test creates a config object with an empty snapshot property but doesn't test the case where
utils.percy.config.snapshot
itself is undefined or null. Consider adding a test case where the entire config path is missing.
it('should return false when both options and config are undefined', () => {
…github.com:percy/percy-selenium-js into adding-support-for-ignoreCanvasSerializationErrors
56e4b02
to
e935175
Compare
Added support for making canvas serialization fail-safe.
Passing
ignoreCanvasSerializationErrors
in snapshot options or at global config. ignore any errors that occur during canvas serialization. And instead renders a broken Img.