Add DefaultLookupTest for lookupOptional NPE fix

[ascheman]

What

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

Why

#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 — 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 master; 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 Maven Core regression test to ensure DefaultLookup.lookupOptional(...) continues to return Optional.empty() when the underlying PlexusContainer returns null (the behavior introduced by switching to Optional.ofNullable in #12336), preventing silent reintroduction of the prior NPE.

Changes:

• Introduces DefaultLookupTest covering both lookupOptional overloads for the null-return case.
• Adds assertions for the “value present” case and for translation of ComponentLookupException caused by NoSuchElementException to Optional.empty().

---

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

[gnodet]

Review: Add DefaultLookupTest for lookupOptional NPE fix (master)

Nice work adding this regression guard — the test is well-structured and covers the key scenarios. 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 (the original fix #12336 also had none).

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:

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 [#12336] prefix to the commit (this project tracks via GitHub issues rather than JIRA, so no MNG-XXXX).

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