Add DefaultLookupTest for lookupOptional NPE fix

[ascheman]

What

Adds DefaultLookupTest (maven-core) — the regression test that the backported NPE fix #12340 shipped without.

Why

#12340 (backport of #12336) changed DefaultLookup.lookupOptional(...) to use Optional.ofNullable instead of Optional.of, so a null component lookup returns an empty Optional instead of throwing NullPointerException. Nothing currently guards that behavior on this line — reverting the fix goes undetected by the suite.

Test

DefaultLookupTest covers both lookupOptional overloads:

• container returns null -> empty Optional (the guarded NPE case)
• container returns a value -> present Optional
• ComponentLookupException whose cause is NoSuchElementException -> empty

Verified locally: green on current maven-4.0.x; reverting ofNullable->of turns the two null-case tests red with the NPE from java.util.Optional.of. No production code change.

[Copilot]

Pull request overview

Adds a regression test in maven-core to lock in the behavior of DefaultLookup.lookupOptional(...) after the backported fix that switched to Optional.ofNullable(...), ensuring null container lookups yield Optional.empty() instead of an NPE.

Changes:

• Introduces DefaultLookupTest covering lookupOptional(Class) / lookupOptional(Class, String) returning empty when the container returns null.
• Adds coverage for translating ComponentLookupException caused by NoSuchElementException into Optional.empty() (currently only for the (Class) overload).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gnodet]

Review: Add DefaultLookupTest for lookupOptional NPE fix (maven-4.0.x)

Nice work adding this regression guard — the test is well-structured and covers the key scenarios. This is the backport of the same test from PR #12384 (master), with the only difference being the Javadoc reference to #12340 instead of #12336. A couple of minor suggestions below.

Findings

1. [Low / Suggestion] Missing test for the rethrow path

The production code has a branch where ComponentLookupException with a cause that is not NoSuchElementException rethrows as LookupException. The test covers the NoSuchElementException cause path but does not test that other causes are properly rethrown. This would round out coverage, but is not a blocker for this focused regression guard PR.

2. [Info] No [MNG-XXXX] prefix in commit message

Minor convention deviation — acceptable for a test-only addition.

Positive notes

• 6 well-structured JUnit 5 test methods covering both lookupOptional overloads × 3 scenarios (null, present, NoSuchElementException)
• Proper Mockito usage with existing project dependency
• Correct Apache License 2.0 header
• CI: all 22 checks pass

---

This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.

Reviewed with Claude Code on behalf of @gnodet. This review was generated by an AI agent and may contain inaccuracies; please verify all suggestions before applying.

[ascheman]

Thanks for the review, @gnodet! Both points addressed (backport of #12384):

1. Rethrow path — added coverage for a ComponentLookupException whose cause is not a NoSuchElementException: it is now asserted to rethrow as LookupException (rather than be swallowed into an empty Optional), for both lookupOptional overloads — lookupOptionalRethrowsOnNonNoSuchElementCause and the (Class, String) variant. The matrix is now both overloads × {null → empty, present → value, NoSuchElement cause → empty, other cause → rethrow} = 8 tests.

2. Commit reference — added a [#12340] prefix to the commit (this project tracks via GitHub issues rather than JIRA, so no MNG-XXXX).

Force-pushed (7277c0b); locally green with full gates (spotless/checkstyle/rat), 8/8. CI re-running.