Skip to content

smokeTestRelease.py: Remove --test-alt-java support#4608

Open
dsmiley wants to merge 2 commits into
apache:mainfrom
dsmiley:smokeTestRelease_removeAltJava
Open

smokeTestRelease.py: Remove --test-alt-java support#4608
dsmiley wants to merge 2 commits into
apache:mainfrom
dsmiley:smokeTestRelease_removeAltJava

Conversation

@dsmiley

@dsmiley dsmiley commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

This option (PR-#2685, ported from a similar Lucene feature) added complexity: parallel lists of alt-JDK homes/runners/versions threaded through the Java config, a closure to dedupe per-JDK test runs, and per-JDK copies of the unpacked binary distribution. It was never well exercised -- it merged via lazy consensus with the sole tester noting they hadn't actually tried it with a different JDK version -- and it isn't invoked anywhere in the documented release process (releaseWizard.yaml never passes it). It has caused at least two real bugs, including a TypeError crash when no alt JDK is given at all.

Testing with another JDK is just as easy by setting JAVA_HOME and re-running the script; the modest cost of redoing some work is a fair trade for a much simpler script.


done with AI including research of the PR that got this done and also researched the Lucene PR.

This option (PR-apache#2685, ported from a similar Lucene feature) added
complexity: parallel lists of alt-JDK homes/runners/versions
threaded through the Java config, a closure to dedupe per-JDK test runs,
and per-JDK copies of the unpacked binary distribution. It was never
well exercised -- it merged via lazy consensus with the sole tester
noting they hadn't actually tried it with a different JDK version -- and
it isn't invoked anywhere in the documented release process
(releaseWizard.yaml never passes it). It has caused at least two real
bugs, including a TypeError crash when no alt JDK is given at all.

Testing with another JDK is just as easy by setting JAVA_HOME and
re-running the script; the modest cost of redoing some work is a fair
trade for a much simpler script.
@dsmiley dsmiley requested review from iamsanjay and janhoy July 4, 2026 17:01

@janhoy janhoy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's better to fix the feature than rip it out. I have sometimes smoke tested using two JDKs. Especially when I'm RM and want to be extra thorough. The Jenkins jobs can easily be configured to test JRE21 and JRE25, so that's not a real argument.

Yes, you can run the smoke tester twice with different JAVA_HOME, but will it be done? Two runs will mean double download, double maven test etc and take longer than with alt-java.

I think it would be better spent time to fix the feature and advocate for more testing rather than make it harder to have great test coverage prior to each release.

But if you feel strongly for it I won't block it.

@@ -0,0 +1,8 @@
title: >
smokeTestRelease.py: Remove --test-alt-java support

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need for a changelog for this internal change

@dsmiley

dsmiley commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Is it really that hard to simply run the smoke tester again? I doubt how many people are actually doing this. Yes, it's less efficient but whatever… it's a relatively rare event to run this

@dsmiley

dsmiley commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Also, doing test-alt-java increases the likelihood of one of them failing. Easy enough to run separately and repeat again the one thing that failed.

@janhoy

janhoy commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

We could of course make it easier to re-run smoke tester on the already downloaded files in /tmp. Perhaps a successful run could end with a text: "To re-run with another java version, use JAVA_HOME=/path/to/java ./smokeTestRelease.py --foo --bar file:///tmp/path/to/existing/download

@dsmiley

dsmiley commented Jul 4, 2026

Copy link
Copy Markdown
Contributor Author

Love that idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants