Added additional tests to ProjectModelResolverTest

[TheRealHaui]

Added additional tests to ProjectModelResolverTest

[cstamas]

LGTM. @gnodet ?

[gnodet]

Review: Added additional tests to ProjectModelResolverTest

Good addition — these tests exercise a meaningful error path: what happens when a user provides a syntactically invalid version range (: instead of , as separator). This covers the catch (VersionRangeResolutionException e) branch in ProjectModelResolver.resolveModel() that was previously untested. The throws Exception cleanup is also a nice touch.

The tests follow the existing conventions in the file (JUnit 5 Assertions, consistent naming, matching parent/dependency test pairs). @cstamas has already given an LGTM.

Observations

Low — Full message assertions depend on Aether library text

The assertions use assertEquals on the full exception message string:

"Failed to resolve version range for org.apache:apache:pom:[1:2): Invalid version range [1:2), single version must be surrounded by []"

This entire message originates from the Aether VersionRangeResolutionException — Maven just passes it through via e.getMessage(). If the resolver library changes this wording, the test breaks for reasons unrelated to Maven.

The existing tests in this file that use assertEquals assert on messages that Maven itself constructs (the String.format(...) calls at lines 197-198 and 209-210 of ProjectModelResolver.java). The one existing test that checks a message from an external library (testResolve*WhenNotFound) uses assertTrue(contains(...)) instead of assertEquals.

For consistency and resilience, consider assertTrue(e.getMessage().contains("Invalid version range")) or assertThat(e.getMessage()).contains("Invalid version range").

This is a minor observation — the tests are correct and useful as-is.

---

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of Guillaume Nodet

[gnodet]

Clean test-only PR that adds two new test cases covering the malformed version range error path in ProjectModelResolver and removes unnecessary throws Exception declarations from six existing test methods. All changes are correct and follow the file's existing conventions.

One minor observation: the assertEquals assertions check the full exception message string from VersionRangeResolutionException. If Aether changes this wording, these tests would break. Consider using assertTrue(e.getMessage().contains("Invalid version range")) for resilience — though the tests are correct as-is. (Already noted by a prior reviewer.)

CI is fully green (the single macOS+JDK17 IT failure is unrelated).

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of gnodet

[slachiewicz]

The last CI run on this branch was 2025-11-28, and GitHub has deleted the logs since, so there's no way to tell whether the change still builds against master.

A rebase triggers a fresh run and makes the state readable again. If you'd rather not carry it forward, closing is equally useful — the PR isn't being ignored, it's unreadable.

This comment was created with AI assistance.

[TheRealHaui]

@slachiewicz,
I think the only thing necessary to do here is to re-start the CI builds.

[slachiewicz]

I can't trigger it as it's old run and it's possible only for 30 days since last run