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

Run some tests on JDK 17 #511

Merged
merged 3 commits into from
Dec 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .github/workflows/continuous-integration.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,19 @@ jobs:
- os: windows-latest
java: 8
epVersion: 2.4.0
- os: ubuntu-latest
java: 17
epVersion: 2.10.0
fail-fast: false
runs-on: ${{ matrix.os }}
steps:
- name: Check out NullAway sources
uses: actions/checkout@v2
- name: 'Set up JDK ${{ matrix.java }}'
uses: actions/setup-java@v1
uses: actions/setup-java@v2
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Took the opportunity to use the latest setup-java action and specify the zulu distribution (same as what Error Prone uses, though it shouldn't matter)

with:
java-version: ${{ matrix.java }}
distribution: 'zulu'
- name: Build and test using Gradle, Java 8, and Error Prone ${{ matrix.epVersion }}
env:
ORG_GRADLE_PROJECT_epApiVersion: ${{ matrix.epVersion }}
Expand All @@ -49,6 +53,11 @@ jobs:
with:
arguments: :nullaway:test :jmh:test
if: matrix.java == '11'
- name: Build and test using Gradle and Java 17
uses: eskatos/gradle-command-action@v1
with:
arguments: :nullaway:test :jmh:test
if: matrix.java == '17'
- name: Report jacoco coverage
uses: eskatos/gradle-command-action@v1
env:
Expand Down
18 changes: 17 additions & 1 deletion nullaway/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,29 @@ test {
maxHeapSize = "1024m"
if (!JavaVersion.current().java9Compatible) {
jvmArgs "-Xbootclasspath/p:${configurations.errorproneJavac.asPath}"
} else {
// to expose necessary JDK types on JDK 16+; see https://errorprone.info/docs/installation#java-9-and-newer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense. Wonder if we might need to change our own READMEs here once we officially support JDK 17.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, we need to make a pass on our READMEs, particularly for non-Android support; see #467 (which I really need to get back to). For Gradle users, I think these flags all get added automatically by gradle-errorprone-plugin. We need to add them manually here since our unit test runs don't get configured using that plugin

jvmArgs += [
"--add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.file=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.main=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.parser=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.processing=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
"--add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
"--add-opens=jdk.compiler/com.sun.tools.javac.comp=ALL-UNNAMED",
// Accessed by Lombok tests
"--add-opens=jdk.compiler/com.sun.tools.javac.jvm=ALL-UNNAMED",
]
}
}

apply plugin: 'com.vanniktech.maven.publish'

jacoco {
toolVersion = "0.8.2"
toolVersion = "0.8.7"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed for JDK 17 compatibility

}

jacocoTestReport {
Expand Down