VisualCeption icon indicating copy to clipboard operation
VisualCeption copied to clipboard

Fix issues with create screenshot

Open paulmskim opened this issue 3 years ago • 7 comments

This PR fixes a couple issues with the createScreenshot() method:

  • If the element position is outside the window dimensions, and we are not setting the fullScreenShot config to true, a blank image is generated. To fix this, I've added a new config forceFullScreenShot config to capture the full screen to allow cropping to occur when the element position is outside the initial window dimensions.
  • If a single test runs any of the test methods more than once, and the fullScreenShot config is true, all of the screenshots after the first image returns a repeated image of the last iterated image. To fix this, I've added a script for the webdriver to set the window position back to 0, 0.

Not sure if the forceFullScreenShot config is the best way to do this, but the other option is to capture the full screen in all cases.

paulmskim avatar Nov 07 '22 18:11 paulmskim

@buresmi7 any thoughts on when you can address this?

paulmskim avatar Nov 18 '22 17:11 paulmskim

@paulmskim thanks for your PR ;) new option looks good! it would be nice have some test for it.

buresmi7 avatar Nov 21 '22 21:11 buresmi7

@buresmi7 I tried adding tests but the webdriver I'm using on my project (https://packagist.org/packages/codeception/module-webdriver#1.4.0) runs differently from the webdriver for the tests (the facebook webdriver does not respect the window size option when taking a screenshot, which is the problem here, and I can't run a test to show the issue).

One option is to update webdriver to a non-abandoned package, and maybe codeception too.

Any other thoughts on how to test this?

paulmskim avatar Nov 22 '22 09:11 paulmskim

@buresmi7 I tried adding tests but the webdriver I'm using on my project (https://packagist.org/packages/codeception/module-webdriver#1.4.0) runs differently from the webdriver for the tests (the facebook webdriver does not respect the window size option when taking a screenshot, which is the problem here, and I can't run a test to show the issue).

One option is to update webdriver to a non-abandoned package, and maybe codeception too.

Any other thoughts on how to test this?

#78 updates codeception, webdriver & php version. Would you like check your PR against latest versions ?

ggiak avatar Jan 18 '23 15:01 ggiak

@paulmskim any updates on this ?

ggiak avatar Jan 29 '23 14:01 ggiak

@ggiak I've done some simple testing within the module, but not enough within other projects to be happy with this. I will say that jumping directly from php >=5.4 to >=8.0 is a bit extreme. There are also no incremental versions that follow codeception, webdriver, and other module releases. Projects that are still running on php 7.4 or older codeception versions, like the one I'm working on, cannot use this.

I'll keep this PR open, but will leave it to you and @buresmi7 to merge or close as you see fit.

paulmskim avatar Jan 30 '23 17:01 paulmskim

@ggiak I've done some simple testing within the module, but not enough within other projects to be happy with this. I will say that jumping directly from php >=5.4 to >=8.0 is a bit extreme. There are also no incremental versions that follow codeception, webdriver, and other module releases. Projects that are still running on php 7.4 or older codeception versions, like the one I'm working on, cannot use this.

I'll keep this PR open, but will leave it to you and @buresmi7 to merge or close as you see fit.

Hey @paulmskim, from my point of view we "jumped" so much in order support codeception version 5, not php8.

Unfortunately, codeception 5 requires php8 as seen here: https://github.com/Codeception/Codeception/blob/5.0/composer.json#L17

hence the huge refactoring. But you are right, Perhaps we should write something relevant to the readme

ggiak avatar Jan 30 '23 23:01 ggiak