Skip to content

Conversation

@bbakerman
Copy link
Member

@bbakerman bbakerman commented Jun 5, 2025

Nullaway

This PR adds errorprone / nullaway / jspecify support.

It changes the tests to run on 11/17/21 (and not just in 11 as before).
We compile and build with Java 21 now, but release for 11.
Gradle itself also runs now with 21 in the github actions.

A new custom annotation @Contract is introduced to enable custom contracts for example for the Assert class.
We fix up a few places that were found by nullaway.

One specific PropertyDataFetcher test is failing for 17/21, so it is now ignored when executed on a JVM > 11. As this is not a new problem we decided to ignore it in this case and deal with it later.

DataLoader bugfix

It also fixes a bug for the new chained dataloader logic that showed up only when running the tests with Java 21/

@github-actions
Copy link
Contributor

github-actions bot commented Jun 5, 2025

Test Results

  318 files    318 suites   2m 50s ⏱️
4 884 tests 4 874 ✅ 10 💤 0 ❌
4 973 runs  4 963 ✅ 10 💤 0 ❌

Results for commit a05a319.

♻️ This comment has been updated with latest results.

@bbakerman bbakerman requested review from andimarek and dondonz June 12, 2025 07:48
@andimarek andimarek force-pushed the errorprone-jspecify-nullaway-support branch from 8d3e1bd to 1d5d4f6 Compare June 14, 2025 11:38
}
test.dependsOn testWithJava17
test.dependsOn testWithJava11

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice - I am guessing this runs on both JDKs

Copy link
Member

Choose a reason for hiding this comment

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

we specify inside the task different toolchains ...so we run the tests with different Java versions

.build()

then: "execution shouldn't error"
for (int i = 0; i < NUM_OF_REPS; i++) {
Copy link
Member Author

Choose a reason for hiding this comment

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

why did this change ????

Copy link
Member

Choose a reason for hiding this comment

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

Because I found that the @RepeatUntilFailure is better way to achieve the same goal.

for (ResultPathWithDataLoader resultPathWithDataLoader : callStack.allResultPathWithDataLoader) {
// we need to copy the list because the callStack.allResultPathWithDataLoader can be modified concurrently
// while iterating over it
ArrayList<ResultPathWithDataLoader> resultPathWithDataLoaders = new ArrayList<>(callStack.allResultPathWithDataLoader);
Copy link
Member

Choose a reason for hiding this comment

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

@bbakerman fyi: this is a fix for a bug I found by running the tests on Java 21. This problem didn't show up earlier.


def 'Executable Definitions with type definition'() {
def query = """
def query = StringGroovyMethods.stripIndent("""
Copy link
Member

Choose a reason for hiding this comment

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

Java 15 introduced a stripIndent method which is used by default now. In order to keep the tests the same we now specify the groovy method explicitly

- uses: actions/checkout@v4
- uses: gradle/actions/wrapper-validation@v4
- name: Set up JDK 11
- name: Set up JDK 21
Copy link
Member

Choose a reason for hiding this comment

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

We should run gradle on the latest version by default.

@andimarek andimarek changed the title Adding errorprone support Adding errorprone support and fix chained dataloader bug Jun 17, 2025
@andimarek andimarek merged commit 5b7d12b into master Jun 17, 2025
2 checks passed
@dondonz dondonz added this to the 25.x breaking changes milestone Jul 20, 2025
@dondonz dondonz deleted the errorprone-jspecify-nullaway-support branch August 16, 2025 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants