-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
TST: Add test for correct rendering of watermarks #2130
Conversation
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #2130 +/- ##
==========================================
+ Coverage 94.13% 94.16% +0.02%
==========================================
Files 41 41
Lines 7467 7467
Branches 1474 1474
==========================================
+ Hits 7029 7031 +2
+ Misses 273 271 -2
Partials 165 165 ☔ View full report in Codecov by Sentry. |
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.
This test should not prevent passing tests if gs is not installed
Just for the record: I currently get a similarity value of 0.765599726490022 when running the check against the original page. I am not sure whether we should verify that this value always stays below the limit imposed for the watermark variant. |
What is the results when you compare without the fix ? I think comparing with .5 would be required. However I'm surprised you are getting such a low comparing result... |
The current comparison of the expected and actual result yields 100 % similarity, which I do not really consider surprising for PNGs where the general rendering artifacts are mostly non-existent (compared to DCTs for JPEGs). The remaining 5 % in the check allow for some general flexibility. I do not really think that 76.5 % similarity between the original and watermarked version is low due to the fact that the test basically adds a background image/pattern to the page. The value should be the same as in the errorneous state due to the watermark not being visible in this case as well, but I have not tested this again. |
What I meant is that without the fix the page is displayed blank. the comparison should return about 0 but the test will still pass. Am I wrong ? |
Why would the test still pass when there would be 0 % similarity, but we set a similarity gate of >= 95 %? Nevertheless, the blank case only occurs for already blank input files, which is what I provided as initial reproducing example on my bug report as white pages are uncritical regarding privacy. Citing from my initial report at #2112:
This means that the document would (mostly) be rendered the same as the input document, sometimes (like in Adobe Acrobat) with some warnings regarding the issues. |
I have to appologize again: I did not see the assertion test. This is all good!👏 |
This was originally added by stefan on #2130 This allows anyone to manually run the workflow on their fork if desired with any branch. See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_dispatch Co-authored-by: stefan6419846 <96178532+stefan6419846@users.noreply.github.com>
This was originally added by stefan on #2130 This allows anyone to manually run the workflow on their fork if desired with any branch. See https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#workflow_dispatch Co-authored-by: stefan6419846 <96178532+stefan6419846@users.noreply.github.com>
I've added the workflow_dispatch via #2145. Although I don't quite see the need for it, I also see no harm. If it's useful for you, I'm happy to add it. I wanted to have this in a separate PR as it's completely unrelated to the core changes in here. |
@stefan6419846 If you're ok with it, I would add those three files:
to https://github.com/py-pdf/sample-files . That is better than having it just on Github as its directly cloned. Do you own the copyright on those files (aka: did you create them)? Is it OK for you if I add them to the sample-files repository? |
Thank you for providing this test 🙏 I'll check what I have in my private repository; hopefully tomorrow. My test is for sure different ... maybe there is value in having both ways 🤔 |
## What's new ### Bug Fixes (BUG) - Cope with missing /I in articles (#2134) - Fix image look-up table in EncodedStreamObject (#2128) - remove_images not operating in sub level forms (#2133) ### Robustness (ROB) - Cope with damaged PDF (#2129) ### Documentation (DOC) - How to take ownership (#2123) ### Developer Experience (DEV) - Download PDFs before executing the tests to not run into timeouts (#2143) - Add workflow_dispatch to CI (#2145) - Automatically create release message / tag message (#2127) - Ensure the REL commit message is consistently created (#2126) ### Testing (TST) - Add test for correct rendering of watermarks (#2130) [Full Changelog](3.15.4...3.15.5)
No, these are the ones added by #1952 and have been uploaded in #1951 (comment) by @ucomru. |
This adds a test for the correct rendering of watermarked files.
Closes #2112