CI: Test on all current Java LTS versions (8, 11 ,17, and 21)

[allentiak]

Tests on JDK 17 and 21 are optional (since they currently fail).

The very existence of these testes will increase the awareness on the importance of code compatibility with newer Java versions, thus enabling future transitions to be implemented gradually.

Making the code run on JDK 11 took a lot of hacking. This will reduce the effort for eventual future transitions.

[tool4ever]

Looks like it's TestNG causing problems?
Would they even offer a version that can support 4 JDK simultaneously... 🤔
Otherwise I think seeing all PR fail (partly) is too noisy and confusing for contributors.

[allentiak]

Looks like it's TestNG causing problems? Would they even offer a version that can support 4 JDK simultaneously... 🤔 Otherwise I think seeing all PR fail (partly) is too noisy and confusing for contributors.

Well, it seems this PR is already achieving its aim of raising awareness on things to improve...
What about accepting this pain as an incentive to

1. deprecate TestNG?, and
2. move existing TestNG tests to JUnit 5?

Like this PR, for example: #4942

[tehdiplomat]

Don't you need to run your test with add-opens?

forge/forge-gui-desktop/src/main/config/forge.sh

Lines 44 to 52 in 152ad0b

	if [[ $v -ge 17 ]]
	then
	java --add-opens java.desktop/java.beans=ALL-UNNAMED --add-opens java.desktop/java.awt.color=ALL-UNNAMED --add-opens java.desktop/javax.swing.border=ALL-UNNAMED --add-opens java.desktop/javax.swing.event=ALL-UNNAMED --add-opens java.desktop/sun.awt.image=ALL-UNNAMED --add-opens java.desktop/sun.swing=ALL-UNNAMED --add-opens java.desktop/javax.swing=ALL-UNNAMED --add-opens java.desktop/java.awt=ALL-UNNAMED --add-opens java.base/java.util=ALL-UNNAMED --add-opens java.base/java.lang=ALL-UNNAMED --add-opens java.base/java.lang.reflect=ALL-UNNAMED --add-opens java.base/java.text=ALL-UNNAMED --add-opens java.desktop/java.awt.font=ALL-UNNAMED --add-opens java.desktop/java.awt.image=ALL-UNNAMED --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --add-opens java.base/sun.nio.ch=ALL-UNNAMED --add-opens java.base/java.nio=ALL-UNNAMED --add-opens java.base/java.math=ALL-UNNAMED --add-opens java.base/java.util.concurrent=ALL-UNNAMED --add-opens java.base/java.net=ALL-UNNAMED -Dio.netty.tryReflectionSetAccessible=true $SHAREDPARAMS
	elif [[ $v -ge 11 ]]
	then
	java --illegal-access=permit $SHAREDPARAMS
	else
	java $SHAREDPARAMS
	fi

[allentiak]

Don't you need to run your test with add-opens?

Wouldn't this hide the fact that the current code does not compile with JDK 17+?

I think that it is important to be explicitly aware of this...
(aka "Bring the pain forward")
https://medium.com/continuousdelivery/if-it-hurts-do-it-more-often-f5a00cc12ffa

If we are not even aware of this problem, then no one will see an incentive to fix it...

As I said above, I think that just this awareness in itself makes merging this worth it...

[Hanmac]

Wouldn't this hide the fact that the current code does not compile with JDK 17+?

it would. if using the right ADD-OPENS parameters

[github-actions]

This PR has not been updated in a while nad has been marked on stale. Stale PRs will be auto closed