pedrovgs / Shot

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

Update for JDK 17 #292

Closed Jawnnypoo closed 1 year ago

Jawnnypoo commented 2 years ago

Hey there, this project has conflicts and does not work on projects that are targeting JDK 17. Most of this is due to the outdated usages of scrimage, so the bulk of this is updating that usage, but with some bonuses:

This PR sort of supersedes https://github.com/pedrovgs/Shot/pull/288 FYI just since it adds the additional support for JDK 17. Fixes #287

pedrovgs commented 2 years ago

@Jawnnypoo looks like there is something wrong with the CI config after your update. Can you please take a look so we can ensure the CI is passing before releasing a new version? Thank you in advance 😃

pedrovgs commented 2 years ago

@Jawnnypoo I think after merging this PR all our users will be forced to use at least JDK >= 11. Am I wrong?

Jawnnypoo commented 2 years ago

Yeah, that sounds right. The latest Android Studio plugin requires Java 11 if I recall correctly.

Jawnnypoo commented 1 year ago

Sorry this took so long, it should be good now, can we rerun this?

Jawnnypoo commented 1 year ago

There might be some issues relating to Maven Central and the release signing, but thats not something I can look too much into myself. I confirmed that this branch can be deployed to Jitpack correctly, see https://jitpack.io/com/github/Jawnnypoo/Shot/1b555c2d20/build.log

Jawnnypoo commented 1 year ago

If you wanted to try it out:

classpath(com.github.Jawnnypoo.Shot:shot:1b555c2d20)

with:

maven("https://jitpack.io")

configured for buildscript

robertgorterns commented 1 year ago

I tried this PR but I get an error running it in my project:


> Task :app:debugExecuteScreenshotTests FAILED
🔎  Comparing screenshots with previous ones.
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':app:debugExecuteScreenshotTests'.
> Input image must store pixels as ints. Convert your input image using 0
* Try:
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.
* Exception is:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':app:debugExecuteScreenshotTests'.
    at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda$executeIfValid$1(ExecuteActionsTaskExecuter.java:142)
    at org.gradle.internal.Try$Failure.ifSuccessfulOrElse(Try.java:282)
    at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:140)
    at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:128)
    at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:77)
    at org.gradle.api.internal.tasks.execution.FinalizePropertiesTaskExecuter.execute(FinalizePropertiesTaskExecuter.java:46)
    at org.gradle.api.internal.tasks.execution.ResolveTaskExecutionModeExecuter.execute(ResolveTaskExecutionModeExecuter.java:51)
    at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:57)
    at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:57)
    at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:36)
    at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.executeTask(EventFiringTaskExecuter.java:77)
    at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:55)
    at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:52)
    at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
    at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
    at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
    at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
    at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
    at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
    at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
    at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
    at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:52)
    at org.gradle.execution.plan.LocalTaskNodeExecutor.execute(LocalTaskNodeExecutor.java:69)
    at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:322)
    at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:309)
    at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:302)
    at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:288)
    at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.execute(DefaultPlanExecutor.java:462)
    at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.run(DefaultPlanExecutor.java:379)
    at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
    at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:49)
Caused by: java.lang.IllegalStateException: Input image must store pixels as ints. Convert your input image using 0
    at thirdparty.romainguy.BlendComposite$BlendingContext.compose(BlendComposite.java:111)
    at com.sksamuel.scrimage.composite.BlenderComposite.apply(BlenderComposite.java:23)
    at com.sksamuel.scrimage.composite.RedComposite.apply(RedComposite.java:5)
    at com.sksamuel.scrimage.ImmutableImage.composite(ImmutableImage.java:509)
    at com.karumi.shot.screenshots.ScreenshotsDiffGenerator.generateDiff(ScreenshotsDiffGenerator.scala:37)
    at com.karumi.shot.screenshots.ScreenshotsDiffGenerator.$anonfun$generateDiffs$1(ScreenshotsDiffGenerator.scala:21)
    at scala.collection.parallel.AugmentedIterableIterator.map2combiner(RemainsIterator.scala:116)
    at scala.collection.parallel.AugmentedIterableIterator.map2combiner$(RemainsIterator.scala:113)
    at scala.collection.parallel.immutable.ParVector$ParVectorIterator.map2combiner(ParVector.scala:66)
    at scala.collection.parallel.ParIterableLike$Map.leaf(ParIterableLike.scala:1056)
    at scala.collection.parallel.Task.$anonfun$tryLeaf$1(Tasks.scala:53)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at scala.util.control.Breaks$$anon$1.catchBreak(Breaks.scala:67)
    at scala.collection.parallel.Task.tryLeaf(Tasks.scala:56)
    at scala.collection.parallel.Task.tryLeaf$(Tasks.scala:50)
    at scala.collection.parallel.ParIterableLike$Map.tryLeaf(ParIterableLike.scala:1053)
    at scala.collection.parallel.AdaptiveWorkStealingTasks$WrappedTask.compute(Tasks.scala:153)
    at scala.collection.parallel.AdaptiveWorkStealingTasks$WrappedTask.compute$(Tasks.scala:149)
    at scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$WrappedTask.compute(Tasks.scala:440)
    at scala.collection.parallel.ForkJoinTasks$WrappedTask.sync(Tasks.scala:379)
    at scala.collection.parallel.ForkJoinTasks$WrappedTask.sync$(Tasks.scala:379)
    at scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$WrappedTask.sync(Tasks.scala:440)
    at scala.collection.parallel.ForkJoinTasks.executeAndWaitResult(Tasks.scala:423)
    at scala.collection.parallel.ForkJoinTasks.executeAndWaitResult$(Tasks.scala:416)
    at scala.collection.parallel.ForkJoinTaskSupport.executeAndWaitResult(TaskSupport.scala:60)
    at scala.collection.parallel.ExecutionContextTasks.executeAndWaitResult(Tasks.scala:555)
    at scala.collection.parallel.ExecutionContextTasks.executeAndWaitResult$(Tasks.scala:555)
    at scala.collection.parallel.ExecutionContextTaskSupport.executeAndWaitResult(TaskSupport.scala:84)
    at scala.collection.parallel.ParIterableLike$ResultMapping.leaf(ParIterableLike.scala:960)
    at scala.collection.parallel.Task.$anonfun$tryLeaf$1(Tasks.scala:53)
    at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
    at scala.util.control.Breaks$$anon$1.catchBreak(Breaks.scala:67)
    at scala.collection.parallel.Task.tryLeaf(Tasks.scala:56)
    at scala.collection.parallel.Task.tryLeaf$(Tasks.scala:50)
    at scala.collection.parallel.ParIterableLike$ResultMapping.tryLeaf(ParIterableLike.scala:955)
    at scala.collection.parallel.AdaptiveWorkStealingTasks$WrappedTask.compute(Tasks.scala:153)
    at scala.collection.parallel.AdaptiveWorkStealingTasks$WrappedTask.compute$(Tasks.scala:149)
    at scala.collection.parallel.AdaptiveWorkStealingForkJoinTasks$WrappedTask.compute(Tasks.scala:440)```
Jawnnypoo commented 1 year ago

Was it working previously with the latest version of this library?

robertgorterns commented 1 year ago

Previously it was working with the last release of shot + java 11. But now my project uses java 17, the main release of shot didn't work, so I tried this branch but I got this error that I hadn't had before

Jawnnypoo commented 1 year ago

@pedrovgs I think you'll have to set up CI in a way that it doesn't need release signing unless it is actually pushing out the artifacts to Maven Central

Jawnnypoo commented 1 year ago

Removed release signing, should pass now

robertgorterns commented 1 year ago

After recording my screenshots again it's working fine!

vudzkostek commented 1 year ago

Verified that this also solves issue described under https://github.com/pedrovgs/Shot/issues/268. Hope it will be released soon, for now I will use local build in project because it prevents us from upgrading AGP, ComposeCompiler and couple of other things that require JDK 17.

But, as @robertgorterns wrote, it requires re-recording of all screenoshots so worth mentioning in changelog @pedrovgs

robertgorterns commented 1 year ago

also when the screenshot tests fail I don't get the proper error response, but the response I commented earlier

amatsegor commented 1 year ago

@pedrovgs as far as I can see, all the changes are approved and MR is kind of good to go. When could a release with this fix be expected?

robertgorterns commented 1 year ago

@Jawnnypoo There are still Build errors with that latest commit

thomasflad commented 1 year ago

@Jawnnypoo Thanks again for the work in this PR. Is there something missing to finish it? It seems close

pedrovgs commented 1 year ago

The update has been finally impemented here