rticau / ScreenCapLibrary

Robot Framework test library for capturing screenshots and video recording.
Apache License 2.0
39 stars 15 forks source link

Added base64 encoded integration of images into HTML log (#71) #78

Closed simonmeggle closed 2 years ago

simonmeggle commented 2 years ago

Hello @mihaiparvu,

this pull request extends the ScreenCapLibrary by the base64 encoded integration of images into the RF HTML log file (as known from SeleniumLibrary of Browser and requested in #71 )

To be more concrete, it adds the argument embed64 to the following keywords:

Honestly, the already used term "embed" here suggests that the images are embedded indeed into the HTML log, but what this currently does is only to link them.

For Robotmk, I needed HTML logs without any external linked files to transport them over to Checkmk. For this, I created the argument embed64 to communicate that in this case a base64 encoded image will be the in the HTML log. This keeps compatibility with existing code. When embed64 is used, after the image got embedded into the HTML log, the original image file on the file system gets deleted to leave everything clean behind. 

In e2e monitoring it is crucial (especially for customers...) to see the cause of failed tests. By default the width in ScreenCapLibrary is always set to 800px which leads to blurry images (and makes it hard to guess the right width). To have images always in their original size, the library now checks the width parameter for a None value. In that case, no width is used at all and the images appear in their original size.

Thanks for the efforts you put so far into this versatile library. Can't wait to hear your thoughts! ;-)

Kind regards, Simon

*** Test Cases ***
Test default
    Take Screenshot  embed64=True  width=None
Test Partial
    Take Partial Screenshot  embed64=True  top=0  left=0  width=250  height=250  embed_width=None
Test Gif Recording
    Start Gif Recording  embed64=True  optimize=False  embed_width=None  size_percentage=1.0
    Sleep  3
    Stop Gif Recording

2021-11-26 22 53 13

simonmeggle commented 2 years ago

Hello @mihaiparvu,

may I ask, did you already have the chance to review this? Thanks, Simon

mihaiparvu commented 2 years ago

Hello @simonmeggle

Thank you for submitting this PR! It looks good to me, the only thing that's missing are the acceptance tests :)

I don't know if you noticed, but there is an already open pull request for this ticket: https://github.com/mihaiparvu/ScreenCapLibrary/pull/75/files which contains tests. Luckily this hasn't been merged yet, so take a look at it if you have time and let me know if there's anything you would like to have added to it.

Best regards, Mihai

simonmeggle commented 2 years ago

Hi @mihaiparvu ,

ah, I see. I'll have a look into that and try to implement the acceptance tests. Best regards, Simon

mihaiparvu commented 2 years ago

No need to implement the acceptance tests, they're already done. Just take a look at the PR #75 and let me know if it's ok for your needs.

simonmeggle commented 2 years ago

Just take a look at the PR #75

I don't think that these tests are appropriate for my PR, aren't they? The keywords used in there are not existing in my case.

mihaiparvu commented 2 years ago

PR #75 will be included in the next version of ScreenCapLibrary, because it's done and tested. Your PR is similar to that one and does roughly the same thing. What I'm asking is if your can review and test #75 and let me know if it works as you expect.

mihaiparvu commented 2 years ago

Closing as it was implemented in https://github.com/mihaiparvu/ScreenCapLibrary/commit/1fcd858e176a2cb5a381127dc8f0df5f87c2bcb6

simonmeggle commented 2 years ago

Sorry, I did not have any time to review the tests. Thanks for implementing the image embedding 👍