Skip to content

Allow configuration of compare-images#6

Merged
moshensky merged 3 commits intomoshensky:masterfrom
mzl-md:master
Sep 11, 2020
Merged

Allow configuration of compare-images#6
moshensky merged 3 commits intomoshensky:masterfrom
mzl-md:master

Conversation

@mzl-md
Copy link
Copy Markdown
Contributor

@mzl-md mzl-md commented Aug 31, 2020

Changes:

  • typo yellow in compare-images, must be Yellow
  • enable configuration of compareImages in comparePdfToSnapshot
  • if tests fail in github actions: store snapshots as build artifacts so they can be reviewed later

@mzl-md
Copy link
Copy Markdown
Contributor Author

mzl-md commented Sep 7, 2020

Hi @moshensky , do you know when you get to review (and hopefully release) this? I would really appreciate to use the module but without the tolerance flag the tests always fail on some images inside the pdf.

@moshensky
Copy link
Copy Markdown
Owner

Hi @mzl-md sorry for the late reply. I will have a look asap. Latest tomorrow morning.

Copy link
Copy Markdown
Owner

@moshensky moshensky left a comment

Choose a reason for hiding this comment

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

👍
As a small improvement might be adding a test that makes sure that public interface is exercised, i.e. adding one test with options that would overwrite defaults.

@moshensky moshensky merged commit 60901b1 into moshensky:master Sep 11, 2020
@moshensky
Copy link
Copy Markdown
Owner

Hi @moshensky , do you know when you get to review (and hopefully release) this? I would really appreciate to use the module but without the tolerance flag the tests always fail on some images inside the pdf.

@mzl-md I released it: https://www.npmjs.com/package/pdf-visual-diff

@mzl-md
Copy link
Copy Markdown
Contributor Author

mzl-md commented Sep 11, 2020

Great, thank you!

As a small improvement might be adding a test that makes sure that public interface is exercised, i.e. adding one test with options that would overwrite defaults.

I thought about that but was not sure how to check that the overwritten defaults are being applied.
Also the README could be updated with the new params. Maybe I'll get to that later.

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