Proposal: Require Java 21

[Ryo_Sangnoir]

Hi all,

Much like we started requiring Java 17 two years ago, we're now considering requiring Java 21 (for example, so that we can use the simd-json library for faster JSON parsing, which may help people with extremely large inventories, like me).

If you're using it, you don't need to make any changes; if not you could download it in preparation. If you're already using Java 21, you may have already noticed a tiny benefit related to GOAWAY handling.

There was some previous discussion at https://kolmafia.us/threads/support-for-java-21-new-lts.29522/

 

[Veracity]

I get a warning on my terminal window with Java 21 on my Mac (macOS Sonoma 14.6.1), but everything seems to work the same as it ever was. (Once in a lifetime, water flowing underground.)

$ java -jar dist/KoLmafia-28139-M.jar 

KoLmafia r28139-M
Build main-981b593-M 21.0.5 (Eclipse Adoptium 21.0.5+11-LTS) Mac OS X aarch64 14.6.1

Currently Running on Mac OS X
Local Directory is /Users/pld/Library/Application Support/KoLmafia
Using Java 21.0.5

2024-11-28 09:30:20.762 java[8918:40868400] *** WARNING: References to Carbon menus are disallowed with AppKit menu system on macOS 14 and newer. Use instances of NSMenu and NSMenuItem directly instead. See https://developer.apple.com/documentation/macos-release-notes/appkit-release-notes-for-macos-14#Menus

 

[heeheehee]

a tiny benefit related to GOAWAY handling.

For what it's worth, I don't think the GOAWAY fix has been backported to Java 21 yet. I think Java 22 might have it, 23 definitely does.

 

[fronobulax]

For what it's worth, I don't think the GOAWAY fix has been backported to Java 21 yet. I think Java 22 might have it, 23 definitely does.

Darn. The "withResponse" test helpers don't work (for me) when the base code uses a resetting connection. It would be easier for me to just deprecate the resetting connection.

 

[heeheehee]

Darn. The "withResponse" test helpers don't work (for me) when the base code uses a resetting connection. It would be easier for me to just deprecate the resetting connection.

https://github.com/openjdk/jdk21u-dev/pull/1020 is the backport to Java 21, FYI.

I'd like to hear more about how it's not working for you, since AFAIK it's not causing problems for our Windows test executions on Github.

 

[fronobulax]

https://github.com/openjdk/jdk21u-dev/pull/1020 is the backport to Java 21, FYI.

I'd like to hear more about how it's not working for you, since AFAIK it's not causing problems for our Windows test executions on Github.

See https://github.com/kolmafia/kolmafia/pull/2523, primarily the last two tests in the file which are driven by function and not a coverage report.

I tried to write a test for setting _faxDataChanged using another test (wiki plurals?) as an example. The example injected an http response. My test did not work, I spent time running the test in the debugger and realized the code I was testing used a resetting request whereas the example used a plain request. I changed the code under test to use a plain request instead of a resetting request and the test worked. I then got somewhat distracted. My working hypothesis is that the existing test framework does not support response injection with a resetting request. A less likely alternative but worth considering is that I wrote the "wrong" test.

I am wondering whether the right solution is to make the resetting request extend the plain request and I may play with that later today.

 

[Ryo_Sangnoir]

withResponse only resets the client on the GenericRequest. The client on FileUtilities is separate and cached, so if any other code uses it first you'd not expect it to be overridden by the fake client created by the tests.