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

Upgrade Gradle Wrapper and build against Java 18 #651

Merged
merged 31 commits into from Sep 19, 2022

Conversation

beatngu13
Copy link
Member

@beatngu13 beatngu13 commented Jul 14, 2022

EDIT: Updated based on Nicolai's comment.

Draft because this is still open for discussion.

Closes #613, #659.

Proposed commit message:

Upgrade Gradle, build against Java 18/19, improve modular tests (#613, #659 / #651)

(Yes, this change does too much, but it all hangs together.)

Updates the Gradle wrapper version from 7.4 to 7.5 because that comes
with support for Java 18, which is also added to the build pipeline.
The experimental Java version is now (for another few days), Java 19.

The modular build is properly configured to really test from the
module path.

Furthermore, replaces the archived/deprecated GitHub action
sormuras/download-jdk with oracle-actions/setup-java.

Closes: #613, #659
PR: #651

PR checklist

The following checklist shall help the PR's author, the reviewers and maintainers to ensure the quality of this project.
It is based on our contributors guidelines, especially the "writing code" section.
It shall help to check for completion of the listed points.
If a point does not apply to the given PR's changes, the corresponding entry can be simply marked as done.

Documentation (general)

  • There is documentation (Javadoc and site documentation; added or updated)
  • There is implementation information to describe why a non-obvious source code / solution got implemented
  • Site documentation has its own .adoc file in the docs folder, e.g. docs/report-entries.adoc
  • Site documentation in .adoc file references demo in src/demo/java instead of containing code blocks as text
  • Only one sentence per line (especially in .adoc files)
  • Javadoc uses formal style, while sites documentation may use informal style

Documentation (new extension)

  • The docs/docs-nav.yml navigation has an entry for the new extension
  • The package-info.java contains information about the new extension

Code

  • Code adheres to code style, naming conventions etc.
  • Successful tests cover all changes
  • There are checks which validate correct / false usage / configuration of a functionality and there are tests to verify those checks
  • Tests use AssertJ or our own PioneerAssert (which are based on AssertJ)

Contributing

  • A prepared commit message exists
  • The list of contributions inside README.md mentions the new contribution (real name optional)

@beatngu13 beatngu13 added the full-build Triggers full build suite on PR label Jul 14, 2022
@beatngu13
Copy link
Member Author

beatngu13 commented Jul 14, 2022

Warning
Added merge-ready to see how the build behaves.

@beatngu13
Copy link
Member Author

JUnit 5.9.0-RC1 is causing trouble:

/home/runner/work/junit-pioneer/junit-pioneer/src/test/java/org/junitpioneer/internal/TestExtensionContext.java:33: error: TestExtensionContext is not abstract and does not override abstract method getExecutableInvoker() in ExtensionContext
public class TestExtensionContext implements ExtensionContext {

Will exclude it from this PR, openend #652 for that purpose.

@beatngu13 beatngu13 changed the title Upgrade Gradle Wrapper and build (Java and JUnit) Upgrade Gradle Wrapper and build against Java 18 Jul 14, 2022
@Bukama
Copy link
Member

Bukama commented Jul 15, 2022

Task :demoTests
EnvironmentVariablesExtensionDemo > testSet() FAILED
    org.junit.jupiter.api.extension.ExtensionConfigurationException at EnvironmentVariableUtils.java:104
        Caused by: java.lang.reflect.InaccessibleObjectException at AccessibleObject.java:354
EnvironmentVariablesExtensionDemo > testClear() FAILED
    org.junit.jupiter.api.extension.ExtensionConfigurationException at EnvironmentVariableUtils.java:104
        Caused by: java.lang.reflect.InaccessibleObjectException at AccessibleObject.java:354
EnvironmentVariablesExtensionDemo > testClearAndSet() FAILED
    org.junit.jupiter.api.extension.ExtensionConfigurationException at EnvironmentVariableUtils.java:104
        Caused by: java.lang.reflect.InaccessibleObjectException at AccessibleObject.java:354
1[85](https://github.com/junit-pioneer/junit-pioneer/runs/7347294866?check_suite_focus=true#step:4:86) 
tests completed, 3 failed, 13 skipped

We got failing tests with Java 18, seems the EnvironmentExtension needs some love

@beatngu13
Copy link
Member Author

I believe it is because of this:

https://github.com/junit-pioneer/junit-pioneer/blob/main/docs/environment-variables.adoc#warnings-for-reflective-access

But why are tests passing on 17? Shouldn't the InaccessibleObjectException also occur there?

https://github.com/junit-pioneer/junit-pioneer/runs/6745721032?check_suite_focus=true#step:4:66

@beatngu13
Copy link
Member Author

Now we get an error in the default locale extension with Java 19:

> Task :compileJava
D:\a\junit-pioneer\junit-pioneer\src\main\java\org\junitpioneer\jupiter\DefaultLocaleExtension.java:69: warning: [deprecation] Locale(String,String,String) in Locale has been deprecated
			return new Locale(language, country, variant);
			       ^
D:\a\junit-pioneer\junit-pioneer\src\main\java\org\junitpioneer\jupiter\DefaultLocaleExtension.java:71: warning: [deprecation] Locale(String,String) in Locale has been deprecated
			return new Locale(language, country);
			       ^
D:\a\junit-pioneer\junit-pioneer\src\main\java\org\junitpioneer\jupiter\DefaultLocaleExtension.java:73: warning: [deprecation] Locale(String) in Locale has been deprecated
			return new Locale(language);

Apparently, the Locale ctors have been deprecated in Java 19. Several alternatives exist to obtain a Locale, so I looked into Locale.Builder.

However, the impl notes from the deprecated Locale ctor says:

  • Obsolete ISO 639 codes ("iw", "ji", and "in") are mapped to their current forms. See Legacy language codes for more information.
  • For backward compatibility reasons, this constructor does not make any syntactic checks on the input.

When we switch to Locale.Builder, which does syntax checks, we change the extension's behavior (breaking change) and also get test failures:

DefaultLocaleTests
  applied on the method level
    sets the default locale using a language, a country and a variant
      java.util.IllformedLocaleException: Ill-formed variant: gb [at index 0]
    sets the default locale using a language
      java.util.IllformedLocaleException: Ill-formed language: en_EN [at index 0]


@EnabledForJreRange(max = JRE.JAVA_16, disabledReason = "See: https://github.com/junit-pioneer/junit-pioneer/issues/509")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like seeing this annotation in the wild life :D

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

me too - specially as it is my baby :D

@beatngu13
Copy link
Member Author

To move on with this PR, how about excluding the update of the experimental Java version (i.e. Java 19)? I would create separate issue(s) for:

  • Let the default locale extension use Locale.Builder. We do this as part of Pioneer 2.0 since the syntax checks the builder does is a breaking change.
  • Afterwards, include Java 19 (or 20) in the build as experimental version.

@Bukama
Copy link
Member

Bukama commented Aug 16, 2022

I support your suggestion

@beatngu13
Copy link
Member Author

Created #658 and #659, and set experimental Java version to 18.

@beatngu13
Copy link
Member Author

(Almost) all builds are green!

grafik

(That episode deserves a better rating!)


Jokes aside:

  • The 2 failing experimental builds complain about:
    No compatible toolchains found for request filter: {languageVersion=17, vendor=any, implementation=vendor-specific} (auto-detect true, auto-download false)
    
    I think I saw that one before, will try a re-run tomorrow.
  • There are still some warnings such as:
    module-info.class ignored in patch: D:\a\junit-pioneer\junit-pioneer\build\classes\java\main
    
  • The modular setup needs a proper review (@nipafx 👀).

@aepfli
Copy link
Member

aepfli commented Sep 6, 2022

Let's activate auto downloading toolchains for Gradle - this should fix the issue

Gradle will use java 17 to build all kind of experimental builds. As this is the toolchain build default and configured within the gradle file
@aepfli
Copy link
Member

aepfli commented Sep 6, 2022

giphy

@beatngu13
Copy link
Member Author

@aepfli thx for the fix!

One more thing:

# but execute the build on the experimental version

# The Java Version Gradle will be using for execution

I find these comments a bit confusing, both mention "execution" but use different Java version. Maybe:

# Gradle doesn't work with JDK EA builds, so we launch it with a supported Java version,
# but set the Gradle toolchain to use the experimental version

WDYT?

Copy link
Member

@nipafx nipafx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes the failing build (shakes fist at @sormuras), so we should move forward with it. Here's a review, I will take a look at the linked issues soon, to see what else needs to be addressed.

build.gradle.kts Show resolved Hide resolved
build.gradle.kts Show resolved Hide resolved
build.gradle.kts Show resolved Hide resolved
build.gradle.kts Outdated Show resolved Hide resolved
build.gradle.kts Show resolved Hide resolved
docs/environment-variables.adoc Show resolved Hide resolved
@nipafx
Copy link
Member

nipafx commented Sep 14, 2022

Note that I also included the Java 19 upgrade (#659) into this PR by simply ignoring the deprecation warnings.

Updated commit message:

Upgrade Gradle, build against Java 18/19, improve modular tests (#613, #659 / #651)

(Yes, this change does too much, but it all hangs together.)

Updates the Gradle wrapper version from 7.4 to 7.5 because that comes
with support for Java 18, which is also added to the build pipeline.
The experimental Java version is now (for another few days), Java 19.

The modular build is properly configured to really test from the
module path.

Furthermore, replaces the archived/deprecated GitHub action
sormuras/download-jdk with oracle-actions/setup-java.

Closes: #613, #659
PR: #651

@beatngu13 beatngu13 marked this pull request as ready for review September 18, 2022 11:58
@Bukama Bukama merged commit 01713d7 into main Sep 19, 2022
@Bukama Bukama deleted the issue/613-upgrade-gradle-and-pipeline branch September 19, 2022 17:11
Bukama pushed a commit to Bukama/junit-pioneer that referenced this pull request Sep 20, 2022
…t-pioneer#613, junit-pioneer#659 / junit-pioneer#651)

(Yes, this change does too much, but it all hangs together.)

Updates the Gradle wrapper version from 7.4 to 7.5 because that comes
with support for Java 18, which is also added to the build pipeline.
The experimental Java version is now (for another few days), Java 19.

The modular build is properly configured to really test from the
module path.

Furthermore, replaces the archived/deprecated GitHub action
sormuras/download-jdk with oracle-actions/setup-java.

Closes: junit-pioneer#613, junit-pioneer#659
PR: junit-pioneer#651
@beatngu13 beatngu13 mentioned this pull request Nov 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
full-build Triggers full build suite on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade build to use Java 19 as experimental version Upgrade Gradle and pipeline when Java 18 is supported
5 participants