search

zinserjan / wdio-screenshot

A WebdriverIO plugin. Additional commands for taking screenshots with WebdriverIO.
MIT License
108 stars 43 forks source link

Removed shadow from iOS screencaps #72

Closed jankcat closed 7 years ago

jankcat commented 7 years ago

I have removed the shadow from iOS screencaps.

The change essentially just takes slightly overlapping screenshots (scrolling to just above the previous screenshot's bottom edge, as opposed of directly at the edge of the previous screenshot), and then crops half the difference from the bottom of the first screenshot and from the top of the second screenshot.

I took the liberty of running it against my own suite as well, with the following results:

Without Fix With Fix iPhone 6 iOS 9 iPhone 7 iOS 10.2
zinserjan commented 7 years ago

Awesome! Thanks for this.

Just two things before this is ready to merge:

jankcat commented 7 years ago

For sure, let me take a look at that.

jankcat commented 7 years ago

Aaand saucelabs is down, so the int tests are failing: https://status.saucelabs.com/incidents/sczxwt2zwz30

jankcat commented 7 years ago

@zinserjan let me know if you want any more changes.

jankcat commented 7 years ago

@zinserjan Any more comments?

zinserjan commented 7 years ago

@jankcat sorry, hadn't any time to review this, yet.

Looks good for me. Great job! I think this is ready to merge. But before I'll give it a try on a few projects, probably on wednesday evening.

jankcat commented 7 years ago

Oh sure, fair enough. Let me know if it needs more work, I have time the end of this week to do so.

jankcat commented 7 years ago

@zinserjan I made the requested changes, I see what you mean about making it easier to understand.

zinserjan commented 7 years ago

@jankcat looks good. I'll merge this after the CI build is fixed (#74).

Question is how this should be released, as a patch (0.5.3) or as a minor/major (0.6.0)? This PR is kind of breaking change in case of the capturing results :grin:

jankcat commented 7 years ago

Haha thats a tough one... From a regression standpoint I would say 0.6.0. No apis changed but the output would break visual diffing for sure.

zinserjan commented 7 years ago

I would also say 0.6.0 to avoid visual regressions.

jankcat commented 7 years ago

Hurray! Thanks.

jankcat commented 7 years ago

When will you do an npm publish?

zinserjan commented 7 years ago

wdio-screenshot 0.6.0 published :tada: Great job @jankcat :)

I'll publish also a new version of wdio-visual-regression-service :wink: