Skip to content

Test fixtures/available java homes throw#36144

Merged
blindpirate merged 1 commit into
gradle:masterfrom
Julianw03:test-fixtures/available-java-homes-throw
Jan 20, 2026
Merged

Test fixtures/available java homes throw#36144
blindpirate merged 1 commit into
gradle:masterfrom
Julianw03:test-fixtures/available-java-homes-throw

Conversation

@Julianw03

@Julianw03 Julianw03 commented Dec 27, 2025

Copy link
Copy Markdown
Contributor

Fixes #35766

  • getDifferentJdk() and getDifferentVersion() now throw IllegalStateException with actionable error messages instead of returning null
  • Added lenient variants getDifferentJdkOrNull() and getDifferentVersionOrNull() for cases that need nullable returns
  • Added isDifferentJdkAvailable() and isDifferentVersionAvailable() for use in test preconditions
  • Updated IntegTestPreconditions to use the new boolean methods
  • Added @Requires annotations to tests that rely on these methods

Context

See root issue for context.

Additional Notes

  1. While working on this, I noticed several places where the precondition and actual usage seem mismatched (e.g., checking DifferentVersionAvailable but using getDifferentJdk()). I've flagged these with comments in the code. Could you clarify if these are bugs or intentional?

  2. Does this approach match what was envisioned in the issue discussion?

  3. Are the Messages of the thrown exceptions good enough?

  4. Should I apply changes like this to other methods aswell ( for example 'getDifferentVersion(final Spec<? super JvmInstallationMetadata> filter)')?

  5. All of the changes are to the tests themselves. I dont think I need to add unit tests is that correct ?

This is my first time contributing so any constructive feedback would be greatly appreciated.

Contributor Checklist

  • Review Contribution Guidelines.
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team.
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective.
  • Provide unit tests (under <subproject>/src/test) to verify logic.
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes.
  • Ensure that tests pass sanity check: ./gradlew sanityCheck.
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest.

Reviewing cheatsheet

Before merging the PR, comments starting with

  • ❌ ❓must be fixed
  • 🤔 💅 should be fixed
  • 💭 may be fixed
  • 🎉 celebrate happy things

@Julianw03 Julianw03 requested review from a team as code owners December 27, 2025 05:25
@Julianw03 Julianw03 requested a review from h0tk3y December 27, 2025 05:25
@bot-gradle bot-gradle added the from:contributor PR by an external contributor label Dec 27, 2025
@Julianw03 Julianw03 requested review from ghale and removed request for a team December 27, 2025 05:25
@Julianw03 Julianw03 requested review from asodja and bamboo December 27, 2025 05:25
@ov7a

ov7a commented Dec 29, 2025

Copy link
Copy Markdown
Member

Thank you for your proposed contribution!

This PR has a valid DCO and tests. The relevant team for this area will confirm the design of the implementation choices.

@ov7a ov7a added in:building-gradle gradle/gradle build 👋 team-triage Issues that need to be triaged by a specific team and removed to-triage labels Dec 29, 2025
@reinsch82 reinsch82 removed the request for review from a team December 29, 2025 14:16
@ov7a ov7a removed the to-triage label Jan 13, 2026
@cobexer cobexer requested review from blindpirate and removed request for a team January 16, 2026 07:45
@blindpirate

This comment has been minimized.

@bot-gradle

This comment has been minimized.

@bot-gradle

This comment has been minimized.

@blindpirate blindpirate left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for the delay. I think this PR makes totally sense and I appreciate your contribution. Please proceed with you proposed change in the comment and it's good to go!

@blindpirate

This comment has been minimized.

@bot-gradle

This comment has been minimized.

@bot-gradle

Copy link
Copy Markdown
Collaborator

The following builds have passed:

@blindpirate blindpirate left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution!

@blindpirate

Copy link
Copy Markdown
Member

@bot-gradle squash and merge

@bot-gradle

Copy link
Copy Markdown
Collaborator

We don't support squashing contributor's PR yet.

- getDifferentJdk() and getDifferentVersion() now throw IllegalStateException with actionable error messages instead of returning null
- Added lenient variants getDifferentJdkOrNull() and getDifferentVersionOrNull() for cases that need nullable returns
- Added isDifferentJdkAvailable() and isDifferentVersionAvailable() for use in test preconditions
- Updated IntegTestPreconditions to use the new boolean methods
- Added `@Requires` annotations to tests that rely on these methods

Co-authored-by: JulianW03 <woelkjulian@gmail.com>
@blindpirate blindpirate force-pushed the test-fixtures/available-java-homes-throw branch from 0221785 to db77830 Compare January 20, 2026 03:13
@blindpirate blindpirate enabled auto-merge January 20, 2026 03:13
@blindpirate blindpirate added this pull request to the merge queue Jan 20, 2026
@bot-gradle bot-gradle added this to the 9.4.0 RC1 milestone Jan 20, 2026
Merged via the queue into gradle:master with commit 9269228 Jan 20, 2026
3 checks passed
@cobexer cobexer added a:chore Minor issue without significant impact and removed 👋 team-triage Issues that need to be triaged by a specific team labels Jan 20, 2026
@Julianw03 Julianw03 deleted the test-fixtures/available-java-homes-throw branch January 20, 2026 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:chore Minor issue without significant impact from:contributor PR by an external contributor in:building-gradle gradle/gradle build

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of AvailableJavaHomes causes NPEs when environment is not ready

5 participants