gnodet commented on code in PR #12117:
URL: https://github.com/apache/maven/pull/12117#discussion_r3279175715
##########
impl/maven-core/src/test/java/org/apache/maven/configuration/DefaultBeanConfiguratorTest.java:
##########
@@ -108,8 +110,72 @@ void testChildConfigurationElement() throws
BeanConfigurationException {
assertEquals(new File("test"), bean.file);
}
+ @Test
+ void testSealedTypeImplementationHintCanUseSimpleName() throws
BeanConfigurationException {
+ SealedBean bean = new SealedBean();
+
+ Xpp3Dom config = toConfig("<artifact
implementation=\"LocalArtifact\"><name>local</name></artifact>");
+
+ DefaultBeanConfigurationRequest request = new
DefaultBeanConfigurationRequest();
+ request.setBean(bean).setConfiguration(config);
+
+ configurator.configureBean(request);
+
+ LocalArtifact artifact = assertInstanceOf(LocalArtifact.class,
bean.artifact);
+ assertEquals("local", artifact.name);
+ }
+
+ @Test
+ void testSealedTypeImplementationHintCanStillUseClassName() throws
BeanConfigurationException {
+ SealedBean bean = new SealedBean();
+
+ Xpp3Dom config = toConfig("<artifact implementation=\"" +
RemoteArtifact.class.getName()
+ + "\"><url>https://example.invalid/artifact</url></artifact>");
+
+ DefaultBeanConfigurationRequest request = new
DefaultBeanConfigurationRequest();
+ request.setBean(bean).setConfiguration(config);
+
+ configurator.configureBean(request);
+
+ RemoteArtifact artifact = assertInstanceOf(RemoteArtifact.class,
bean.artifact);
+ assertEquals("https://example.invalid/artifact";, artifact.url);
+ }
+
+ @Test
+ void testSealedTypeImplementationHintMustMatchPermittedSubclass() {
+ SealedBean bean = new SealedBean();
+
+ Xpp3Dom config = toConfig("<artifact
implementation=\"MissingArtifact\"><name>missing</name></artifact>");
+
+ DefaultBeanConfigurationRequest request = new
DefaultBeanConfigurationRequest();
+ request.setBean(bean).setConfiguration(config);
+
+ BeanConfigurationException e =
+ assertThrows(BeanConfigurationException.class, () ->
configurator.configureBean(request));
+ assertEquals(
Review Comment:
Minor: there's no test for the **ambiguity** code path — where two permitted
subclasses share the same simple name. The code handles it with a sorted error
message, so it would be good to verify that too. This is admittedly a rare
scenario (e.g., inner classes from different enclosing types) but the code
branch exists.
Also, the `assertEquals(e.getMessage(), ...)` assertion here could be
brittle if `BeanConfigurationException` changes how it wraps the cause message.
Consider `assertTrue(e.getMessage().contains(...))` as a more resilient check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
