Run tests on the current JVM / Gradle 8.8 #4730

dlvenable · 2024-07-11T15:50:11Z

Description

While debugging a test recently, I noticed that the Java 17 test reports a basic java.lang.NullPointerException. Java 17 provides better NPE information that Java 11. It turns out that the tests are actually running against Java 11 entirely. Even in our matrix tests, these run on Java 11.

This PR uses the current JVM versions (ie. what Gradle is running on) to execute the Java tests. This way, our tests will test actually test against other Java versions.

Also, I updated the Gradle 8.8 because JavaLanguageVersion.current() was added in 8.8.

Verification

I created a simple test to prove this:

public class QuickFail {
    @Test
    void fails() {
        A a = new A();
        B b = new B();
        
        a.b.c.doSomething();
    }

    static class A {
        B b;
    }

    static class B {
        C c;
    }

    static class C {
        void doSomething() {}
    }
}

Running on Java 11:

java11 ./gradlew -p data-prepper-api clean build

yields:

java.lang.NullPointerException
	at org.opensearch.dataprepper.QuickFail.fails(QuickFail.java:15)

Running on Java 17:

java17 ./gradlew -p data-prepper-api clean build

yields:

java.lang.NullPointerException: Cannot read field "c" because "a.b" is null
	at org.opensearch.dataprepper.QuickFail.fails(QuickFail.java:15)

Issues Resolved

N/A

Check List

New functionality includes testing.
New functionality has a documentation issue. Please link to it in this PR.
- New functionality has javadoc added
Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

… tests. This fixes a problem with our current GitHub tests where we are running against only Java 11 even though we want to run against different Java versions (11, 17, 21). Updates the Gradle version to 8.8. Signed-off-by: David Venable <dlv@amazon.com>

…stop which now always throws an UnsupportedOperationException. Signed-off-by: David Venable <dlv@amazon.com>

…'s stdout. This is too much noise. Signed-off-by: David Venable <dlv@amazon.com>

…o stdout. Signed-off-by: David Venable <dlv@amazon.com>

…vent_json codec. These tests are failing now on Java 17 and 21 with precision errors. Signed-off-by: David Venable <dlv@amazon.com>

…caused an IllegalArgumentException. Signed-off-by: David Venable <dlv@amazon.com>

dlvenable · 2024-07-12T16:05:38Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/sink/AbstractSink.java

@@ -76,7 +78,7 @@ public void output(Collection<T> records) {
    @Override
    public void shutdown() {
        if (retryThread != null) {
-            retryThread.stop();


In Java 21, Thread::stop always throws an UnsupportedOperationException. So I've changed this approach to tell the runnable below to stop itself.

Do we have other locations in plugins where threads are stopped like this?

I ran IntelliJ's "Find Usages" feature on Thread::stop. It found this usage, but found no others. So I don't believe so.

dlvenable · 2024-07-12T16:06:25Z

data-prepper-core/build.gradle

@@ -48,7 +48,6 @@ dependencies {
        exclude group: 'commons-logging', module: 'commons-logging'
    }
    implementation 'software.amazon.cloudwatchlogs:aws-embedded-metrics:2.0.0-beta-1'
-    testImplementation 'org.apache.logging.log4j:log4j-jpl:2.23.0'


For some reason, in Java 17 and 21, this results in all the logs going to stdout in the Gradle task. This makes understanding the build very difficult as there is far too much noise.

dlvenable · 2024-07-12T16:06:59Z

...s/avro-codecs/src/test/java/org/opensearch/dataprepper/avro/AvroAutoSchemaGeneratorTest.java

@@ -218,7 +218,7 @@ static class SomeUnknownTypesArgumentsProvider implements ArgumentsProvider {
        @Override
        public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
            return Stream.of(
-                    arguments(Random.class),


In Java 21 (and maybe 17?), Random is final and this causes Mockito to fail when it mocks it. So, we test on the Timer class instead.

dlvenable · 2024-07-12T16:07:21Z

.../test/java/org/opensearch/dataprepper/plugins/buffer/blockingbuffer/BlockingBufferTests.java

@@ -328,7 +328,7 @@ public Stream<? extends Arguments> provideArguments(final ExtensionContext conte
            return Stream.of(
                    Arguments.of(0, randomInt + 1, 0.0),
                    Arguments.of(1, 100, 1.0),
-                    Arguments.of(randomInt, randomInt, 100.0),


This can sometimes be zero, and this will cause the test to fail. So, ensure it is not 0.

dlvenable · 2024-07-12T16:07:47Z

...c/test/java/org/opensearch/dataprepper/plugins/codec/event_json/EventJsonInputCodecTest.java

@@ -11,9 +11,12 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.ValueSource;
+


This is mostly running the IntelliJ formatter. The only other changes are the truncatedTo. See comments below.

dlvenable · 2024-07-12T16:09:10Z

...c/test/java/org/opensearch/dataprepper/plugins/codec/event_json/EventJsonInputCodecTest.java

@@ -70,15 +74,15 @@ public void inCompatibleVersionTest() throws Exception {
        final String key = UUID.randomUUID().toString();
        final String value = UUID.randomUUID().toString();
        Map<String, Object> data = Map.of(key, value);
-        Instant startTime = Instant.now();


In later versions of Java on Linux, there is an issue where the nanosecond resolution results in test failures.

dlvenable · 2024-07-12T16:10:40Z

build.gradle

@@ -226,6 +226,9 @@ subprojects {

    test {
        useJUnitPlatform()
+        javaLauncher = javaToolchains.launcherFor {


This tells Gradle to use the current JVM version for running the tests. This is what now allows our tests to run on Java 17 or 21.

graytaylor0 · 2024-07-12T20:07:33Z

data-prepper-api/src/main/java/org/opensearch/dataprepper/model/sink/AbstractSink.java

@@ -76,7 +78,7 @@ public void output(Collection<T> records) {
    @Override
    public void shutdown() {
        if (retryThread != null) {
-            retryThread.stop();


Do we have other locations in plugins where threads are stopped like this?

…h-project#4730) Run tests on the current JVM rather than always using Java 11 for the tests. This fixes a problem with our current GitHub tests where we are running against only Java 11 even though we want to run against different Java versions (11, 17, 21). Updates the Gradle version to 8.8. Fix Java 21 support in the AbstractSink by removing usage of Thread::stop which now always throws an UnsupportedOperationException. Use only microsecond precision time when comparing the times in the event_json codec. These tests are failing now on Java 17 and 21 with precision errors. Fixed a randomly failing test in BlockingBufferTests where a value 0 caused an IllegalArgumentException. Logging changes to avoid noise in the Gradle builds in GitHub. Signed-off-by: David Venable <dlv@amazon.com> Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>

…pensearch-project#4730)" This reverts commit 67f3595.

…pensearch-project#4730)" This reverts commit 67f3595. Signed-off-by: Hai Yan <oeyh@amazon.com>

…4730)" (#4762) This reverts commit 67f3595. Signed-off-by: Hai Yan <oeyh@amazon.com>

…h-project#4730) Run tests on the current JVM rather than always using Java 11 for the tests. This fixes a problem with our current GitHub tests where we are running against only Java 11 even though we want to run against different Java versions (11, 17, 21). Updates the Gradle version to 8.8. Fix Java 21 support in the AbstractSink by removing usage of Thread::stop which now always throws an UnsupportedOperationException. Use only microsecond precision time when comparing the times in the event_json codec. These tests are failing now on Java 17 and 21 with precision errors. Fixed a randomly failing test in BlockingBufferTests where a value 0 caused an IllegalArgumentException. Logging changes to avoid noise in the Gradle builds in GitHub. Signed-off-by: David Venable <dlv@amazon.com> Signed-off-by: Krishna Kondaka <krishkdk@dev-dsk-krishkdk-2c-bd29c437.us-west-2.amazon.com>