Skip to content

Conversation

@trask
Copy link
Member

@trask trask commented Sep 24, 2024

Currently blocked on test failures in VariousCollectorsTest

Disabled the G1 test for now on Java 21+

@jeanbisutti
Copy link
Member

VariousCollectorsTest fails with a JDK 23 because a new GarbageCollectorMXBean, the G1 Concurrent GC, was introduced in the JDK 20 and is not managed by the code.

cc @johnoliver

@trask trask force-pushed the update-test-versions branch from 64d5d6b to 1367556 Compare October 23, 2024 19:37
@trask trask requested a review from johnoliver as a code owner October 23, 2024 19:37
@trask
Copy link
Member Author

trask commented Oct 23, 2024

@johnoliver @dsgrieve I disabled the G1 test on Java 21+ so that we can merge this and make sure at least our other tests are passing on 21 and 23

@trask trask force-pushed the update-test-versions branch from 1367556 to 6bad36c Compare October 23, 2024 19:48
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.condition.JRE.JAVA_11;
import static org.junit.jupiter.api.condition.JRE.JAVA_13;
import static org.junit.jupiter.api.condition.JRE.JAVA_17;
Copy link
Contributor

Choose a reason for hiding this comment

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

does it need java 21 and java 23 test here since you added java 17?

Copy link
Member Author

Choose a reason for hiding this comment

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

this import is for an exclusion, see below:

@EnabledForJreRange(max = JAVA_17)

}

@Test
@EnabledForJreRange(max = JAVA_17)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@EnabledForJreRange(max = JAVA_17)
@EnabledForJreRange(max = JAVA_17) // see https://github.com/microsoft/ApplicationInsights-Java/pull/3878#issuecomment-2431980712

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to merge so I don't have to wait for another CI build, but will send a quick follow-up to add this comment

@trask trask merged commit 3c69f1b into main Oct 24, 2024
90 checks passed
@trask trask deleted the update-test-versions branch October 24, 2024 14:30
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