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

Add compilation support for Java 8 & Java 11 #184

Closed
steve-donovan opened this issue Jul 22, 2022 · 13 comments
Closed

Add compilation support for Java 8 & Java 11 #184

steve-donovan opened this issue Jul 22, 2022 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@steve-donovan
Copy link
Contributor

steve-donovan commented Jul 22, 2022

Description

When having a later JDK installed >8, the amadeus-java tests fail.

Steps to Reproduce

  1. install a version of JDK > 9
  2. run the tests ./gradlew test

Expected Behavior: all tests from master branch to pass

Actual Behavior: almost half of the tests fail

Stable Behavior? 100%

Versions

JDK 15
Mac O/S 12.4

Cause

The issue is the removal of unsafe operations from JDK 9 onward.

Solution

update the version of mockito-core (simplest) or add the following to build.gradle file (obviously replacing my path with an env variable). User experience would suffer with the latter solution as they'd need to install another JRE and set up an env variable.

tasks.withType(Test) {
    executable = new File("/Library/Internet Plug-Ins/JavaAppletPlugin.plugin/Contents/Home/", 'bin/java')
}

should be more like:

tasks.withType(Test) {
    executable = new File("${PATH_TO_TEST_JRE}", 'bin/java')
}
@steve-donovan steve-donovan changed the title Test failures due to JDK > 9.0 and outdated mockito-core version Test failures due to JDK > 8.0 and outdated mockito-core version Jul 22, 2022
@steve-donovan
Copy link
Contributor Author

I'm happy to make the changes once someone decides approach

@jabrena
Copy link
Contributor

jabrena commented Jul 22, 2022

Hi Steven,

let’s review the dependency to upgrade it.
testImplementation "org.mockito:mockito-core:2.12.0"

https://mvnrepository.com/artifact/org.mockito/mockito-core

Juan Antonio

@steve-donovan
Copy link
Contributor Author

steve-donovan commented Jul 22, 2022

Hi Steven,

let’s review the dependency to upgrade it. testImplementation "org.mockito:mockito-core:2.12.0"

https://mvnrepository.com/artifact/org.mockito/mockito-core

Juan Antonio

all tests run and succeed with the latest version, and as it's mockito, its touch is confined to test suites

// https://mvnrepository.com/artifact/org.mockito/mockito-core
testImplementation 'org.mockito:mockito-core:4.6.1'

@jabrena
Copy link
Contributor

jabrena commented Jul 22, 2022

Hi @steve-donovan,

I was reviewed again the issue, and you are using a non LTS JVM version (Java 15).

The last LTS JVM versions are:

  • Java 8
  • Java 11
  • Java 17

Source: https://www.oracle.com/java/technologies/java-se-support-roadmap.html

Did you test with Java 17 the solution that you mention?

@steve-donovan
Copy link
Contributor Author

Hi @jabrena

I had not tried 17 - I'll try doing so shortly and will let you know the result.

@steve-donovan
Copy link
Contributor Author

Hi @jabrena

Beyond JDK 15 you get a Lombok error. I'm looking into it now

@jabrena
Copy link
Contributor

jabrena commented Jul 22, 2022

Maybe, it is related with this issue:
projectlombok/lombok#3219

and it is necessary to wait for a new version to improve the support for Java 17.

@steve-donovan
Copy link
Contributor Author

Maybe, it is related with this issue: projectlombok/lombok#3219

Maybe, it is necessary a new version for Java 17.

yeah, it was in a way, and that issue helped a lot. I actually solved it for lombok but now have a single test failing
com.amadeus.travel.TripParserIT#given_client_when_call_trip_parser_with_params_alternative_1_then_ok

class com.amadeus.travel.TripParser (in unnamed module @0x5a2e4553) cannot access class com.sun.org.apache.xerces.internal.impl.dv.util.Base64 (in module java.xml) because module java.xml does not export com.sun.org.apache.xerces.internal.impl.dv.util to unnamed module @0x5a2e4553

I'm investigating now.

@steve-donovan
Copy link
Contributor Author

steve-donovan commented Jul 22, 2022

Hi @jabrena

the breaking test is due to a call to internal api
String b64Encoded = Base64.encode(Files.readAllBytes(file.toPath()));

issue described here [adoptium/adoptium-support/issues/199]

@jabrena
Copy link
Contributor

jabrena commented Jul 22, 2022

It is great:

I actually solved it for lombok but now have a single test failing

How did you do it?

The Base64 issue was registered here: #139

In next months, we want to increase the minimum JVM version supported to Java 8, and later Java 11, so we could use the native Base64 support:
https://docs.oracle.com/javase/8/docs/api/java/util/Base64.html

@jabrena jabrena self-assigned this Jul 22, 2022
@jabrena jabrena added the enhancement New feature or request label Jul 22, 2022
@steve-donovan
Copy link
Contributor Author

steve-donovan commented Jul 22, 2022

bumper mockito version to solve original issue, then added/amended scoped lines for bumped version of lombok

    compileOnly "org.projectlombok:lombok:1.18.24"
    annotationProcessor 'org.projectlombok:lombok:1.18.24'
    testCompileOnly 'org.projectlombok:lombok:1.18.24'
    testAnnotationProcessor 'org.projectlombok:lombok:1.18.24'
    testImplementation 'org.mockito:mockito-core:4.6.1'

@steve-donovan
Copy link
Contributor Author

ok - got it
build.gradle change

dependencies {
    implementation 'com.google.code.gson:gson:2.9.0'
    implementation 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.0'
    compileOnly "org.projectlombok:lombok:1.18.24"
    annotationProcessor 'org.projectlombok:lombok:1.18.24'
    testCompileOnly 'org.projectlombok:lombok:1.18.24'
    testAnnotationProcessor 'org.projectlombok:lombok:1.18.24'
    testImplementation 'org.mockito:mockito-core:4.6.1'
    testRuntimeOnly "org.slf4j:slf4j-api:1.7.10"
    testImplementation 'org.junit.jupiter:junit-jupiter-api:5.8.1'
    testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine:5.8.1'
    testImplementation 'org.assertj:assertj-core:3.22.0'
    testImplementation 'com.github.tomakehurst:wiremock:2.27.2'
    testImplementation 'io.rest-assured:rest-assured:5.0.0'
    testImplementation 'org.slf4j:slf4j-api:1.7.36'
    testImplementation 'org.slf4j:slf4j-simple:1.7.36'
}

and TripParser change:

  public TripDetail post(File file) throws ResponseException, IOException {
    // Base64 encode file and create request body
    String b64Encoded = DatatypeConverter.printBase64Binary(Files.readAllBytes(file.toPath()));

    JsonObject body = new JsonObject();
    body.addProperty("payload", b64Encoded);

    Response response = client.post("/v3/travel/trip-parser", body);
    return (TripDetail) Resource.fromObject(response, TripDetail.class);
  }

I can raise a PR if you like 😊

@jabrena
Copy link
Contributor

jabrena commented Jul 22, 2022

Give me the opportunity to think about it.

I believe, that it is not possible to do it in a single round everything:

  • 1. build.gradle - Ok with the dependency upgrade.
  • 2. TripDetail - Waiting. Waiting to increase the minimum compilation version.
  • 3. Update pipeline build_gradle.yml to increase the number of JVM versions supported per commit (Java 8, Java 11) - Ok
  • 4. Update the pipeline to support Java 17 - Waiting for step 2.

We could update the pipeline build_gradle.yml in this way to compile for multiple JVM versions with no change in the build system:
https://github.com/actions/setup-java

jobs:
  build:
    runs-on: ubuntu-20.04
    strategy:
      matrix:
        java: [ '8', '11' ]
    name: Java ${{ matrix.Java }} sample
    steps:
      - uses: actions/checkout@v3
      - name: Setup java
        uses: actions/setup-java@v3
        with:
          distribution: 'adopt'
          java-version: ${{ matrix.java }}

If you observe, in one round, we could increase the compilation support for Java 8 & Java 11 but not for Java 17 immediately, because it is necessary to increase the minimum compilation support for Java 8 (We are discussing internally). In your side, using sdk cli tool, you could switch to different JVM versions in one second, for example from java 17 to java 19 (next september) or to java 11 or to java 8. :)

If you like, we could receive a PR for actions 1 & 3 in this round.

Juan Antonio

@jabrena jabrena changed the title Test failures due to JDK > 8.0 and outdated mockito-core version Add compilation support for Java 8 & Java 11 Jul 23, 2022
@jabrena jabrena closed this as completed Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants