You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "cmunier (via GitHub)" <gi...@apache.org> on 2026/10/11 09:38:54 UTC
[PR] Accept documented lowercase values for maven.resolver.validation [maven]
cmunier opened a new pull request, #13407:
URL: https://github.com/apache/maven/pull/13407
`maven.resolver.validation` is documented with the values `default`, `mild` and `off` (Javadoc of `Constants.MAVEN_RESOLVER_VALIDATION`, `@Config(defaultValue = "default")`, and the Maven 3.10.0 release notes). However, `MavenValidatorFactory` parses the value with `ConfigUtils.getEnum()`, which uses `Enum.valueOf()` and is case-sensitive. Following the documentation therefore fails the build:
```
$ mvn -Dmaven.resolver.validation=mild verify
[ERROR] Internal error: java.lang.IllegalArgumentException: No enum constant org.apache.maven.internal.aether.MavenValidatorFactory.ValidationLevel.mild -> [Help 1]
```
Only `MILD` and `OFF` work. This was noted in the review of #12549, and it affects the escape hatch that the 3.10.0 release notes recommend for builds with non-conformant coordinates.
This PR parses the value ignoring case and surrounding whitespace, so both the documented lowercase values and the uppercase values that work today are accepted. An unknown value is now reported with the property name and the supported values:
```
Invalid value 'foo' for maven.resolver.validation, supported values are: default, mild, off
```
Tested with the new `MavenValidatorFactoryTest` (4 of its 5 tests fail without the change), and end to end with a distribution built from this branch, using a project with a `system`-scoped dependency whose classifier contains a `/`: without the property the build fails as expected, and it passes with `mild`, `MILD` and `Off`.
**Backport:** the same code is in `maven-4.0.x` (`impl/maven-impl`) and `maven-3.10.x` (`maven-core`, `org.apache.maven.internal.aether.MavenValidatorFactory`). As 3.10.0 is released with this behavior, could this please be backported to both branches? I'm happy to open the backport PR for `maven-3.10.x` if you like.
AI disclosure: drafted with Claude Code (Anthropic Claude Opus 5.5, interactive mode). I have reviewed and tested the change myself.
---
- [x] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
- [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #13407:
URL: https://github.com/apache/maven/pull/13407
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "cmunier (via GitHub)" <gi...@apache.org>.
cmunier commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4248166903
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @Test
+ void mildLevelIgnoresCase() {
+ for (String level : new String[] {"mild", "MILD", "Mild", " mild "}) {
Review Comment:
If you prefer, I could implement this before merging. Just wanted to check, because you already approved the pull request.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4255307247
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
Review Comment:
🔧 **Nit:** License header closing `*/` is on the same line as the last comment text — every other Java file in the module uses `*/` on a separate line.
```suggestion
* under the License.
*/
```
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
Review Comment:
🔧 **Nit:** `defaultLevelIgnoresCase` uses a manual for-loop while the equivalent `mildLevelIgnoresCase` (added in the same commit) uses `@ParameterizedTest`. Inconsistent within the same file: a for-loop failure reports only via the assertion message, while a parameterized test gives separate entries per case in the test report.
```suggestion
@ParameterizedTest
@ValueSource(strings = {"default", "DEFAULT", "Default"})
void defaultLevelIgnoresCase(String level) {
Validator validator = newValidator(level);
assertInstanceOf(MavenValidator.class, validator, level);
assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
}
```
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"mild", "MILD", "Mild", " mild "})
+ void mildLevelIgnoresCase(String level) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertDoesNotThrow(() -> validator.validateArtifact(SLASH_CLASSIFIER));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> validator.validateArtifact(new DefaultArtifact("g:a:jar:${classifier}:1.0")));
+ }
+
+ @Test
+ void offLevelIgnoresCase() {
+ for (String level : new String[] {"off", "OFF", "Off"}) {
+ assertSame(ValidatorFactory.NOOP, newValidator(level), level);
+ }
+ }
+
Review Comment:
🔧 **Nit:** Same inconsistency: `offLevelIgnoresCase` uses a for-loop while `mildLevelIgnoresCase` is parameterized.
```suggestion
@ParameterizedTest
@ValueSource(strings = {"off", "OFF", "Off"})
void offLevelIgnoresCase(String level) {
assertSame(ValidatorFactory.NOOP, newValidator(level), level);
}
```
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "cmunier (via GitHub)" <gi...@apache.org>.
cmunier commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4248166903
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @Test
+ void mildLevelIgnoresCase() {
+ for (String level : new String[] {"mild", "MILD", "Mild", " mild "}) {
Review Comment:
> Parametrized test can be used for it.
@slawekjaranowski If you prefer, I could implement this before merging. Just wanted to check, because you already approved the pull request.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4250060160
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @Test
+ void mildLevelIgnoresCase() {
+ for (String level : new String[] {"mild", "MILD", "Mild", " mild "}) {
Review Comment:
if you wish
I will wait a day and can merge as is.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "cmunier (via GitHub)" <gi...@apache.org>.
cmunier commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4251421248
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @Test
+ void mildLevelIgnoresCase() {
+ for (String level : new String[] {"mild", "MILD", "Mild", " mild "}) {
Review Comment:
@slawekjaranowski done. I added `junit-jupiter-params` as a test dependency for using `@ParametrizedTest`. It was already used in several other Maven modules, so I assume it's ok.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "cmunier (via GitHub)" <gi...@apache.org>.
cmunier commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4255591824
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
Review Comment:
False positive?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Accept documented lowercase values for maven.resolver.validation [maven]
Posted by "slawekjaranowski (via GitHub)" <gi...@apache.org>.
slawekjaranowski commented on code in PR #13407:
URL: https://github.com/apache/maven/pull/13407#discussion_r4243037465
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/validator/MavenValidatorFactoryTest.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.impl.resolver.validator;
+
+import java.util.Locale;
+
+import org.apache.maven.api.Constants;
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.artifact.DefaultArtifact;
+import org.eclipse.aether.spi.validator.Validator;
+import org.eclipse.aether.spi.validator.ValidatorFactory;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * Tests {@link MavenValidatorFactory}.
+ */
+class MavenValidatorFactoryTest {
+
+ private static final DefaultArtifact SLASH_CLASSIFIER = new DefaultArtifact("g:a:jar:lib/foo.jar:1.0");
+
+ private final MavenValidatorFactory factory = new MavenValidatorFactory();
+
+ private Validator newValidator(String level) {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> false);
+ if (level != null) {
+ session.setConfigProperty(Constants.MAVEN_RESOLVER_VALIDATION, level);
+ }
+ return factory.newInstance(session);
+ }
+
+ @Test
+ void defaultWhenUnset() {
+ Validator validator = newValidator(null);
+ assertInstanceOf(MavenValidator.class, validator);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER));
+ }
+
+ @Test
+ void defaultLevelIgnoresCase() {
+ for (String level : new String[] {"default", "DEFAULT", "Default"}) {
+ Validator validator = newValidator(level);
+ assertInstanceOf(MavenValidator.class, validator, level);
+ assertThrows(IllegalArgumentException.class, () -> validator.validateArtifact(SLASH_CLASSIFIER), level);
+ }
+ }
+
+ @Test
+ void mildLevelIgnoresCase() {
+ for (String level : new String[] {"mild", "MILD", "Mild", " mild "}) {
Review Comment:
Parametrized test can be used for it.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org