This is an automated email from the ASF dual-hosted git repository. chesnay pushed a commit to branch release-1.16 in repository https://gitbox.apache.org/repos/asf/flink.git
commit 375f6ae8da6069ef6d5b92cd4634203dde75580d Author: Chesnay Schepler <[email protected]> AuthorDate: Wed Oct 5 16:06:38 2022 +0200 [FLINK-29510][tests] Tests high-level logic --- .../tools/ci/licensecheck/NoticeFileChecker.java | 67 +++++++++++++------ .../ci/licensecheck/NoticeFileCheckerTest.java | 76 ++++++++++++++++++++++ 2 files changed, 122 insertions(+), 21 deletions(-) diff --git a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java index 24011ff4cdb..59741264dc3 100644 --- a/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java +++ b/tools/ci/flink-ci-tools/src/main/java/org/apache/flink/tools/ci/licensecheck/NoticeFileChecker.java @@ -90,7 +90,6 @@ public class NoticeFileChecker { "- ([^ :]+):([^:]+):([^ ]+)($| )|.*bundles \"([^:]+):([^:]+):([^\"]+)\".*"); static int run(File buildResult, Path root) throws IOException { - int severeIssueCount = 0; // parse included dependencies from build output final Multimap<String, Dependency> modulesWithBundledDependencies = combine( @@ -108,21 +107,51 @@ public class NoticeFileChecker { + modulesWithBundledDependencies.values().size() + " dependencies"); - final HashSet<String> moduleSkippingDeployment = - new HashSet<>(modulesWithBundledDependencies.keySet()); - moduleSkippingDeployment.removeAll(deployedModules); - moduleSkippingDeployment.forEach(modulesWithBundledDependencies::removeAll); - // find modules producing a shaded-jar List<Path> noticeFiles = findNoticeFiles(root); LOG.info("Found {} NOTICE files to check", noticeFiles.size()); + final Map<String, Optional<NoticeContents>> moduleToNotice = + noticeFiles.stream() + .collect( + Collectors.toMap( + NoticeFileChecker::getModuleFromNoticeFile, + noticeFile -> { + try { + return NoticeParser.parseNoticeFile(noticeFile); + } catch (IOException e) { + // some machine issue + throw new RuntimeException(e); + } + })); + + return run(modulesWithBundledDependencies, deployedModules, moduleToNotice); + } + + @VisibleForTesting + static int run( + Multimap<String, Dependency> modulesWithBundledDependencies, + Set<String> deployedModules, + Map<String, Optional<NoticeContents>> noticeFiles) + throws IOException { + int severeIssueCount = 0; + + final Set<String> modulesSkippingDeployment = + new HashSet<>(modulesWithBundledDependencies.keySet()); + modulesSkippingDeployment.removeAll(deployedModules); + modulesSkippingDeployment.forEach(modulesWithBundledDependencies::removeAll); + // check that all required NOTICE files exists - severeIssueCount += ensureRequiredNoticeFiles(modulesWithBundledDependencies, noticeFiles); + severeIssueCount += + ensureRequiredNoticeFiles(modulesWithBundledDependencies, noticeFiles.keySet()); // check each NOTICE file - for (Path noticeFile : noticeFiles) { - severeIssueCount += checkNoticeFile(modulesWithBundledDependencies, noticeFile); + for (Map.Entry<String, Optional<NoticeContents>> noticeFile : noticeFiles.entrySet()) { + severeIssueCount += + checkNoticeFileAndLogProblems( + modulesWithBundledDependencies, + noticeFile.getKey(), + noticeFile.getValue().orElse(null)); } return severeIssueCount; @@ -145,13 +174,11 @@ public class NoticeFileChecker { } private static int ensureRequiredNoticeFiles( - Multimap<String, Dependency> modulesWithShadedDependencies, List<Path> noticeFiles) { + Multimap<String, Dependency> modulesWithShadedDependencies, + Collection<String> modulesWithNoticeFile) { int severeIssueCount = 0; Set<String> shadingModules = new HashSet<>(modulesWithShadedDependencies.keys()); - shadingModules.removeAll( - noticeFiles.stream() - .map(NoticeFileChecker::getModuleFromNoticeFile) - .collect(Collectors.toList())); + shadingModules.removeAll(modulesWithNoticeFile); for (String moduleWithoutNotice : shadingModules) { LOG.error( "Module {} is missing a NOTICE file. It has shaded dependencies: {}", @@ -175,16 +202,14 @@ public class NoticeFileChecker { return moduleDirectory.getFileName().toString(); } - private static int checkNoticeFile( - Multimap<String, Dependency> modulesWithShadedDependencies, Path noticeFile) + private static int checkNoticeFileAndLogProblems( + Multimap<String, Dependency> modulesWithShadedDependencies, + String moduleName, + @Nullable NoticeContents noticeContents) throws IOException { - String moduleName = getModuleFromNoticeFile(noticeFile); - - Optional<NoticeContents> noticeContents = NoticeParser.parseNoticeFile(noticeFile); final Map<Severity, List<String>> problemsBySeverity = - checkNoticeFile( - modulesWithShadedDependencies, moduleName, noticeContents.orElse(null)); + checkNoticeFile(modulesWithShadedDependencies, moduleName, noticeContents); final List<String> severeProblems = problemsBySeverity.getOrDefault(Severity.CRITICAL, Collections.emptyList()); diff --git a/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/licensecheck/NoticeFileCheckerTest.java b/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/licensecheck/NoticeFileCheckerTest.java index 7b08642571d..cbf09b46f0d 100644 --- a/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/licensecheck/NoticeFileCheckerTest.java +++ b/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/licensecheck/NoticeFileCheckerTest.java @@ -24,12 +24,88 @@ import org.apache.flink.tools.ci.utils.shared.Dependency; import com.google.common.collect.ArrayListMultimap; import org.junit.jupiter.api.Test; +import java.io.IOException; import java.util.Arrays; import java.util.Collections; +import java.util.Optional; +import java.util.Set; import static org.assertj.core.api.Assertions.assertThat; class NoticeFileCheckerTest { + @Test + void testRunHappyPath() throws IOException { + final String moduleName = "test"; + final Dependency bundledDependency = Dependency.create("a", "b", "c"); + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, bundledDependency); + final Set<String> deployedModules = Collections.singleton(moduleName); + final Optional<NoticeContents> noticeContents = + Optional.of( + new NoticeContents( + moduleName, Collections.singletonList(bundledDependency))); + + assertThat( + NoticeFileChecker.run( + bundleDependencies, + deployedModules, + Collections.singletonMap(moduleName, noticeContents))) + .isEqualTo(0); + } + + @Test + void testRunRejectsMissingNotice() throws IOException { + final String moduleName = "test"; + final Dependency bundledDependency = Dependency.create("a", "b", "c"); + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, bundledDependency); + final Set<String> deployedModules = Collections.singleton(moduleName); + final Optional<NoticeContents> missingNotice = Optional.empty(); + + assertThat( + NoticeFileChecker.run( + bundleDependencies, + deployedModules, + Collections.singletonMap(moduleName, missingNotice))) + .isEqualTo(1); + } + + @Test + void testRunRejectsIncorrectNotice() throws IOException { + final String moduleName = "test"; + final Dependency bundledDependency = Dependency.create("a", "b", "c"); + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, bundledDependency); + final Set<String> deployedModules = Collections.singleton(moduleName); + final Optional<NoticeContents> emptyNotice = + Optional.of(new NoticeContents(moduleName, Collections.emptyList())); + + assertThat( + NoticeFileChecker.run( + bundleDependencies, + deployedModules, + Collections.singletonMap(moduleName, emptyNotice))) + .isEqualTo(1); + } + + @Test + void testRunSkipsNonDeployedModules() throws IOException { + final String moduleName = "test"; + final Dependency bundledDependency = Dependency.create("a", "b", "c"); + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, bundledDependency); + final Set<String> deployedModules = Collections.emptySet(); + // this would usually be a problem, but since the module is not deployed it's OK! + final Optional<NoticeContents> emptyNotice = + Optional.of(new NoticeContents(moduleName, Collections.emptyList())); + + assertThat( + NoticeFileChecker.run( + bundleDependencies, + deployedModules, + Collections.singletonMap(moduleName, emptyNotice))) + .isEqualTo(0); + } @Test void testCheckNoticeFileHappyPath() {
