Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build CI on more realistic combinations (and faster) #2898

Open
5 tasks done
TWiStErRob opened this issue Feb 3, 2023 · 3 comments · May be fixed by #3028
Open
5 tasks done

Build CI on more realistic combinations (and faster) #2898

TWiStErRob opened this issue Feb 3, 2023 · 3 comments · May be fixed by #3028

Comments

@TWiStErRob
Copy link
Contributor

Conversation split from #2894 (comment)

I strongly recommend adopting this approach in this repo: https://jakewharton.com/build-on-latest-java-test-through-lowest-java/
I'm opening this issue to discuss if there's an agreement.

Jake's point in that guide is that you build once, and test on different runtimes, same as your use will download your pre-built jar and run it on different JVMs.

Solved Issues

JDK incompatibilities in user's runtime

In the current CI setup:

  • the code is compiled on JDK 11 then tests executed
  • in a different matrix, code is compiled on JDK 17 then tests executed
  • then on tags, releases are built on JDK 11 and published (this is what users will download from maven)

This means that we never ever test the real use case, that the JDK 11-built JAR is executed on JDK 17, for example because the Mockito user's CI is running JDK 17.

@reta mentioned

From experience: the code compiled on older JDKs (fe JDK-8) may fail at runtime (due to slight differences in how JVM interprets bytecode), but it is very rare.

This could be very easily covered by the above approach.

One of the Gradle plugins don't support the old/new JDK version

With Android Gradle Plugin 8.0 (already in beta, released soon), it will be compiled and packaged with Java 17, forcing any of its users to upgrade their Gradle build setup to use JDK 17 or higher (that is the JVM running Gradle itself). This means that Mockito will either need to add conditionals in settings.gradle.kts similar to the one removed here: fe8881e to prevent including the :androidTest module on JDK 11 machines/CI matrix. This will practically mean that the real use case of Mockito built on Java 11, running in Java 17 test runtime in an Android project will not be covered.

Faster CI

Since the build will happen only on one JDK, producing one binary JAR, the build will be more reproducible and better covered. With Jake's approach the individual JDK coverage is done via Gradle tasks and toolchains, meaning contributors can easily run all test on all JDKs without much setup (as opposed to having to install and set JAVA_HOME to mimic CI). Additionally the CI time will be also reduced because the extra compilation on each matrix step will not be necessary any more; the freed up time can be used to cover more JDKs!

Easier adding of new JDKs to test on

Since each JDK coverage is just a task, adding it cannot cause incompatibilities with the hosting Gradle build, because the toolchain will only apply to the specific test task.


check that

  • The mockito message in the stacktrace have useful information, but it didn't help
  • The problematic code (if that's possible) is copied here;
    Note that some configuration are impossible to mock via Mockito
  • Provide versions (mockito / jdk / os / any other relevant information)
  • Provide a Short, Self Contained, Correct (Compilable), Example of the issue
    (same as any question on stackoverflow.com)
  • Read the contributing guide
@TimvdLippe
Copy link
Contributor

Yeah I think this would be good. Preferably, the required changes here are minimal, as I don't want to overhaul our complete CI setup for stability reasons. But this sounds like a small incremental step in the right direction.

That said, I would still like to see different GitHub action jobs in our matrix, to know which particular version we are breaking when CI fails. In other words, I think it is good if we build all jars on Java 11 (because that's the job that we release with) and then we run the tests on each different Java version.

@undermyumbrella1
Copy link
Contributor

Hi, I made a pr with my attempt at this issue, not sure if my approach is correct, do let me know, thank you!

@TWiStErRob
Copy link
Contributor Author

Moving to toolchains will become more prevalent as Gradle plugins are transitioning to require Java 17 running Gradle: AGP 8, and soon bnd 7.x. Gradle 9 will probably break some plugins (e.g. conventions removal will break bnd 6.x), so Mockito (and probably other projects) will get stuck until toolchains are used as far as I see.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants