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

Avoid using JRE.OTHER when disabling Artemis tests to work around ARTEMIS-4975 #41909

Closed
wilkinsona opened this issue Aug 16, 2024 · 7 comments
Closed
Assignees
Labels
type: task A general task
Milestone

Comments

@wilkinsona
Copy link
Member

Apache Artemis is incompatible with Java 23: https://issues.apache.org/jira/browse/ARTEMIS-4975. We're seeing failures on main due to this. I'm not sure why we're not also seeing them on our maintenance branches. We'll need to work around this where necessary. I think we have two options:

  1. Disable the Artemis tests on Java 23
  2. Configure the relevant test task(s) to run with -Djava.security.manager=allow

I think I prefer 1, as 2 would mask problems with other dependencies.

@wilkinsona wilkinsona added this to the 3.4.x milestone Aug 16, 2024
@wilkinsona wilkinsona added the type: task A general task label Aug 16, 2024
@snicoll
Copy link
Member

snicoll commented Aug 17, 2024

I'm not sure why we're not also seeing them on our maintenance branches.

I don't really understand what's going on. When I tried this, I ran the build with --rerun and it succeeded. Looking at CI, there are also a number of CI builds on main that succeeded on Java 23.

Java 23 is not enabled yet on 3.3.x. Looking at 3.2.x this seems to be working indeed.

@wilkinsona
Copy link
Member Author

It might be related to logging configuration. The failures that we've seen have stack traces similar to this:

java.lang.UnsupportedOperationException: getSubject is supported only if a security manager is allowed	
	at java.base/javax.security.auth.Subject.getSubject(Subject.java:347)	
	at org.apache.activemq.artemis.logs.AuditLogger.getCaller(AuditLogger.java:68)	
	at org.apache.activemq.artemis.logs.AuditLogger.getNotificationInfo(AuditLogger.java:1114)	
	at org.apache.activemq.artemis.core.management.impl.ActiveMQServerControlImpl.getNotificationInfo(ActiveMQServerControlImpl.java:4254)	
	at java.management/com.sun.jmx.mbeanserver.MBeanIntrospector.findNotifications(MBeanIntrospector.java:446)	
	at java.management/com.sun.jmx.mbeanserver.MBeanIntrospector.getMBeanInfo(MBeanIntrospector.java:392)	
	at java.management/com.sun.jmx.mbeanserver.MBeanSupport.<init>(MBeanSupport.java:139)	
	at java.management/com.sun.jmx.mbeanserver.StandardMBeanSupport.<init>(StandardMBeanSupport.java:60)	
	at java.management/javax.management.StandardMBean.construct(StandardMBean.java:181)	
	at java.management/javax.management.StandardMBean.<init>(StandardMBean.java:232)	
	at org.apache.activemq.artemis.core.management.impl.AbstractControl.<init>(AbstractControl.java:48)	
	at org.apache.activemq.artemis.core.management.impl.ActiveMQServerControlImpl.<init>(ActiveMQServerControlImpl.java:189)	
	at org.apache.activemq.artemis.core.server.management.impl.ManagementServiceImpl.registerServer(ManagementServiceImpl.java:229)	
	at org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.initialisePart1(ActiveMQServerImpl.java:3309)	
	at org.apache.activemq.artemis.core.server.impl.PrimaryOnlyActivation.run(PrimaryOnlyActivation.java:70)	
	at org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.internalStart(ActiveMQServerImpl.java:742)	
	at org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl.start(ActiveMQServerImpl.java:632)	
	at org.apache.activemq.artemis.core.server.embedded.EmbeddedActiveMQ.start(EmbeddedActiveMQ.java:134)

Looking at ActiveMQServerControlImpl.getNotificationInfo, it'll only go down that code path if base logging is enabled:

   @Override
   public MBeanNotificationInfo[] getNotificationInfo() {
      if (AuditLogger.isBaseLoggingEnabled()) {
         AuditLogger.getNotificationInfo(this.server);
      }
      CoreNotificationType[] values = CoreNotificationType.values();
      String[] names = new String[values.length];
      for (int i = 0; i < values.length; i++) {
         names[i] = values[i].toString();
      }
      return new MBeanNotificationInfo[]{new MBeanNotificationInfo(names, this.getClass().getName(), "Notifications emitted by a Core Server")};
   }

Perhaps the logging configuration isn't consistent across branches/builds. The latter could perhaps be related to test ordering where a previous test has changed the log levels? Regardless, it looks like some more investigation is needed and that a third option may be to tune the logging configuration used by the tests.

@wilkinsona
Copy link
Member Author

I don't really understand what's going on. When I tried this, I ran the build with --rerun and it succeeded.

Can you remember when you did that, @snicoll? Do you have a build scan link for it perhaps? I searched on https://ge.spring.io but didn't manage to find anything.

@snicoll
Copy link
Member

snicoll commented Aug 19, 2024

https://ge.spring.io/s/kfcuenyuu4z6y ? Sorry, I can't see in the scan if I used --rerun or not.

@wilkinsona
Copy link
Member Author

wilkinsona commented Aug 19, 2024

Thank you. The tests are skipped in the build scan and seeing that has given me the nudge that I needed to realise what's going on. I've only just noticed (sorry) that the tests are already disabled depending on the JRE:

@DisabledOnJre(value = JRE.OTHER, disabledReason = "https://issues.apache.org/jira/browse/ARTEMIS-4975")
class ArtemisAutoConfigurationTests {

Looking at the Develocity test dashboard, the tests were being skipped until 16 August when they started running and failing. That coincides with the upgrade to JUnit 5.11 which introduced a value for Java 23 (and Java 24) to its JRE enum so OTHER no longer matches Java 23.

We could switch to using @DisabledOnJre(JRE.Java_23) but I think it would be more future-proof to use @EnabledForJreRange(min = JRE.JAVA_17, max = JRE.JAVA_22) as this will avoid the problem reappearing with Java 24. I'll rework this issue to make that change.

@wilkinsona wilkinsona changed the title Work around ARTEMIS-4975 Avoid using JRE.OTHER when disabling Artemis tests to work around ARTEMIS-4975 Aug 19, 2024
@wilkinsona wilkinsona modified the milestones: 3.4.x, 3.2.x Aug 19, 2024
@wilkinsona wilkinsona self-assigned this Aug 19, 2024
@wilkinsona wilkinsona modified the milestones: 3.2.x, 3.2.9 Aug 19, 2024
snicoll added a commit that referenced this issue Aug 19, 2024
@snicoll
Copy link
Member

snicoll commented Aug 19, 2024

FTR They were a number of other tests that were using JRE.OTHER. I've polished infinspan in 6fd1f0f. It remains one test in JavaVersionTests that I couldn't fix (or I don't know how). I asked the JUnit team to reconsider the issue I created back then: junit-team/junit5#3918

@sbrannen
Copy link
Member

To avoid potential issues with OTHER and to support arbitrary Java feature numbers in JUnit Jupiter's JRE conditions, I spiked a proof of concept which you can view here: junit-team/junit5#3931 (comment).

@wilkinsona and @snicoll, the JUnit Team would appreciate any feedback you have on that proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: task A general task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants