pedrovgs / Shot

Screenshot testing library for Android
Apache License 2.0
1.18k stars 115 forks source link

Update various libraries and migrate to JDK 17 #343

Closed Kurt-Bonatz closed 1 year ago

Kurt-Bonatz commented 1 year ago

:pushpin: References

:tophat: What is the goal?

Android Gradle Plugin 8 requires that apps use JDK 17. Currently, Shot stands as a blocker to using JDK 17 on projects that rely on it.

How is it being implemented?

How can it be tested?

All steps completed below with local maven published version on a Mac M1 using JDK 17 (#287)

Kurt-Bonatz commented 1 year ago

I've had a number of issues getting the sample projects on master completing the executeScreenshotTests tasks. My changes produce the same results as 5.14.1 on the updated sample projects, but several of the screenshot sizes are different than what are on master. So it's likely that CI might fail and I might need to re-record the screenshots as some library updates like the bump to Material could have changed some of the Android views.

amatsegor commented 1 year ago

That looks like a lifesaver, thank you for your big work! I think this issue can be also mentioned as a reference

pedrovgs commented 1 year ago

Thank you so much @Kurt-Bonatz, let me take a look at the PR. It's quite big, but as the CI is passing I think we are good to go unless we find any major blocker πŸ˜ƒ

pedrovgs commented 1 year ago

Version 6.0.0 released, please @Kurt-Bonatz check if everything is working as expected from your end and let me know if there is something broken. Thanks πŸ˜ƒ

Kurt-Bonatz commented 1 year ago

@pedrovgs Apologies for the lengthy change πŸ˜…, and thanks for taking the time to review it! Thus far we haven't had any issues with the new version, though I did re-record our screenshots for good measure.

Also things worked out perfectly since you actually built and published the artifact with JDK 11 (don't worry things still work as expected on JDK 17), which means we could drop the new version in without also having to complete the JDK 17 migration on CI or locally. It actually simplified things quite a bit for us!

pedrovgs commented 1 year ago

That's why I decided to use 6.0.0 as the new version. The changes will force users to update some of their dependencies and maybe the JDK they use πŸ˜ƒ Thanks for your contribution @Kurt-Bonatz I really appreciate it!