ikxeno commented on code in PR #13276: URL: https://github.com/apache/maven/pull/13276#discussion_r4136384148
########## impl/maven-cli/src/test/java/org/apache/maven/cling/invoker/mvnval/ValidateInvokerTest.java: ########## @@ -0,0 +1,789 @@ +/* + * 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.cling.invoker.mvnval; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +import org.apache.maven.api.Session; +import org.apache.maven.api.cli.InvokerException; +import org.apache.maven.api.cli.mvnval.ValidateOptions; +import org.apache.maven.api.services.ModelBuilder; +import org.apache.maven.cling.invoker.ProtoLookup; +import org.apache.maven.impl.standalone.ApiRunner; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static java.nio.charset.StandardCharsets.UTF_8; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Unit tests for the {@link ValidateInvoker} class. + * Tests that the verdict depends on the file alone, that a file which cannot be read does not + * lose the verdict on the others, and that the exit code follows the reports. + */ +@DisplayName("ValidateInvoker") +class ValidateInvokerTest { + + private static final String DUPLICATE_DEPENDENCY = """ + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + %s + <groupId>org.test</groupId> + <artifactId>test</artifactId> + <version>1.0</version> + <dependencies> + <dependency> + <groupId>junit</groupId><artifactId>junit</artifactId><version>4.13.2</version> + </dependency> + <dependency> + <groupId>junit</groupId><artifactId>junit</artifactId><version>4.12</version> + </dependency> + </dependencies> + </project> + """; + + private static final String UNRESOLVABLE_PARENT = """ + <parent> + <groupId>org.nowhere</groupId> + <artifactId>does-not-exist</artifactId> + <version>999.0.0</version> + </parent>"""; + + @TempDir + Path tempDir; + + private List<String> output; + + /** Every path handed to the seam, so a test can assert on the one the run actually made. */ + private List<Path> aimed; + + private ValidateInvoker invoker; + + @BeforeEach + void setUp() { + output = new ArrayList<>(); + // One session for the whole test: building one is the expensive part of every test here. + Session shared = ApiRunner.createSession(); + aimed = new ArrayList<>(); + invoker = new ValidateInvoker(ProtoLookup.builder().build(), null) { + @Override + protected Session createSession(Path localRepository) { + // withLocalRepository is the line production uses, so an aimed repository is + // exercised rather than stubbed. The transports and the settings-derived + // repository list production adds are not: this session has neither. + aimed.add(localRepository); + return localRepository == null + ? shared + : shared.withLocalRepository(shared.createLocalRepository(localRepository)); + } + }; + } + + private Path writePom(String name, String parent) throws Exception { + Path pom = Files.createDirectories(tempDir.resolve(name)).resolve("pom.xml"); + Files.writeString(pom, String.format(DUPLICATE_DEPENDENCY, parent)); + return pom; + } + + private int run(List<String> poms) throws Exception { + return run(poms, Optional.empty()); + } + + private int run(List<String> poms, Optional<String> format) throws Exception { + return run(poms, format, Optional.of("raw")); + } + + /** Defaults to raw: the tests using this helper read files only. */ + private int run(List<String> poms, Optional<String> format, Optional<String> mode) throws Exception { + ValidateOptions options = mock(ValidateOptions.class); + when(options.format()).thenReturn(format); + when(options.mode()).thenReturn(mode); + when(options.poms()).thenReturn(poms.isEmpty() ? Optional.empty() : Optional.of(poms)); + + ValidateContext context = TestUtils.createMockContext(tempDir, options); + context.writer = output::add; + return invoker.execute(context); + } + + @Nested + @DisplayName("Wiring") + class WiringTests { + + @Test + @DisplayName("should register the transports resolution needs") + void shouldRegisterTransports() throws Exception { + // The block whose absence produces "No transporter factories registered". Proved over + // file://, which needs FileTransporterFactory and no network. Every other test here + // replaces createSession, so without this the block could be deleted and stay green. + Path repository = Files.createDirectories(tempDir.resolve("served/org/test/far/1.0")); + String parent = """ + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <groupId>org.test</groupId><artifactId>far</artifactId> + <version>1.0</version><packaging>pom</packaging> + </project>"""; + Files.writeString(repository.resolve("far-1.0.pom"), parent); + // A repository without checksums is refused before the content is looked at, so the + // transport would never be exercised. + Files.writeString(repository.resolve("far-1.0.pom.sha1"), sha1(parent)); + Path pom = Files.createDirectories(tempDir.resolve("fetcher")).resolve("pom.xml"); + Files.writeString(pom, """ + <project xmlns="http://maven.apache.org/POM/4.0.0";> + <modelVersion>4.0.0</modelVersion> + <parent> + <groupId>org.test</groupId><artifactId>far</artifactId> + <version>1.0</version><relativePath/> + </parent> + <artifactId>fetcher</artifactId> + <repositories> + <repository><id>served</id><url>%s</url></repository> + </repositories> + </project>""".formatted(tempDir.resolve("served").toUri())); + + ValidateOptions options = mock(ValidateOptions.class); + when(options.mode()).thenReturn(Optional.of("effective")); + when(options.localRepository()) + .thenReturn(Optional.of(tempDir.resolve("into").toString())); + when(options.poms()).thenReturn(Optional.of(List.of(pom.toString()))); + ValidateContext context = TestUtils.createMockContext(tempDir, options); + context.writer = output::add; + + int exitCode = new ValidateInvoker(ProtoLookup.builder().build(), null).execute(context); + + assertEquals(ValidateInvoker.OK, exitCode, output.toString()); + assertTrue( + Files.exists(tempDir.resolve("into/org/test/far/1.0/far-1.0.pom")), + "the parent has to arrive through a transport: " + output); + } + + @Test + @DisplayName("should exit on bad usage when the model builder cannot validate") + void shouldRefuseAModelBuilderThatCannotValidate() throws Exception { + // validate() is a default that throws, so an out-of-tree ModelBuilder may not have it. + // Unreachable from the shipped CLI, which is why it is worth pinning here: nothing was + // wrong with the POM, so this must not be the code that means a POM was rejected. + ValidateInvoker refusing = new ValidateInvoker(ProtoLookup.builder().build(), null) { + @Override + protected Session createSession(Path localRepository) { + Session session = mock(Session.class); + ModelBuilder builder = mock(ModelBuilder.class); + ModelBuilder.ModelBuilderSession builderSession = mock(ModelBuilder.ModelBuilderSession.class); + when(session.getService(ModelBuilder.class)).thenReturn(builder); + when(builder.newSession()).thenReturn(builderSession); + when(builderSession.validate(any())) + .thenThrow(new UnsupportedOperationException("does not support validating a model")); + return session; + } + }; + ValidateOptions options = mock(ValidateOptions.class); + when(options.mode()).thenReturn(Optional.of("raw")); + when(options.poms()) + .thenReturn(Optional.of(List.of(writePom("unsupported", "").toString()))); + ValidateContext context = TestUtils.createMockContext(tempDir, options); + context.writer = output::add; + + assertEquals(ValidateInvoker.BAD_OPERATION, refusing.execute(context)); + } + } + + private static String sha1(String content) throws Exception { + byte[] digest = java.security.MessageDigest.getInstance("SHA-1").digest(content.getBytes(UTF_8)); + StringBuilder hex = new StringBuilder(digest.length * 2); + for (byte b : digest) { + hex.append(String.format("%02x", b)); + } + return hex.toString(); + } + + @Nested + @DisplayName("Determinism") + class DeterminismTests { + + @Test + @DisplayName("should reach the same verdict whether or not the parent can be resolved") + void shouldReachSameVerdictRegardlessOfParent() throws Exception { + Path withParent = writePom("with-parent", UNRESOLVABLE_PARENT); + Path withoutParent = writePom("without-parent", ""); + + int exitWith = run(List.of(withParent.toString())); + List<String> linesWith = List.copyOf(output); + output.clear(); + int exitWithout = run(List.of(withoutParent.toString())); + + assertEquals(exitWith, exitWithout, "an unresolvable parent must not change the exit code"); + assertTrue( + linesWith.stream().anyMatch(l -> l.contains("must be unique")), + "the duplicate dependency should be reported even with an unresolvable parent: " + linesWith); + assertTrue( + output.stream().anyMatch(l -> l.contains("must be unique")), + "and also without a parent: " + output); + } + + @Test + @DisplayName("should print a path under the working directory as the caller typed it") + void shouldPrintPathsRelativeToTheWorkingDirectory() throws Exception { + Path pom = writePom("near", ""); + Path outside = Files.createDirectories( + Files.createTempDirectory("mvnval-outside-").resolve("far")) + .resolve("pom.xml"); + Files.writeString(outside, String.format(DUPLICATE_DEPENDENCY, "")); Review Comment: Done -- 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]
