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 4d8cdea4c7c5f0328f948257ab776d1d73072b8a Author: Chesnay Schepler <[email protected]> AuthorDate: Wed Oct 5 15:45:29 2022 +0200 [FLINK-25910][tests] Test checks for a given NOTICE file --- .../tools/ci/licensecheck/NoticeFileChecker.java | 132 ++++++++++++--------- .../ci/licensecheck/NoticeFileCheckerTest.java | 99 ++++++++++++++++ tools/maven/suppressions.xml | 1 + 3 files changed, 177 insertions(+), 55 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 e51da1d1d33..24011ff4cdb 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 @@ -18,6 +18,7 @@ package org.apache.flink.tools.ci.licensecheck; +import org.apache.flink.annotation.VisibleForTesting; import org.apache.flink.tools.ci.utils.dependency.DependencyParser; import org.apache.flink.tools.ci.utils.notice.NoticeContents; import org.apache.flink.tools.ci.utils.notice.NoticeParser; @@ -28,6 +29,8 @@ import com.google.common.collect.Multimap; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.annotation.Nullable; + import java.io.BufferedReader; import java.io.File; import java.io.IOException; @@ -43,6 +46,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -176,61 +180,11 @@ public class NoticeFileChecker { throws IOException { String moduleName = getModuleFromNoticeFile(noticeFile); - // 1st line contains module name - NoticeContents noticeContents = NoticeParser.parseNoticeFile(noticeFile).orElse(null); - - final Map<Severity, List<String>> problemsBySeverity = new HashMap<>(); - - if (noticeContents == null) { - addProblem(problemsBySeverity, Severity.CRITICAL, "The NOTICE file was empty."); - } - - // first line must be the module name. - if (!noticeContents.getNoticeModuleName().equals(moduleName)) { - addProblem( - problemsBySeverity, - Severity.TOLERATED, - String.format( - "First line does not start with module name. firstLine=%s", - noticeContents.getNoticeModuleName())); - } + Optional<NoticeContents> noticeContents = NoticeParser.parseNoticeFile(noticeFile); - // collect all declared dependencies from NOTICE file - Set<Dependency> declaredDependencies = new HashSet<>(); - for (Dependency toAdd : noticeContents.getDeclaredDependencies()) { - if (!declaredDependencies.add(toAdd)) { - addProblem( - problemsBySeverity, - Severity.CRITICAL, - String.format("Dependency %s is declared twice.", toAdd)); - } - } - // find all dependencies missing from NOTICE file - Collection<Dependency> expectedDependencies = modulesWithShadedDependencies.get(moduleName); - for (Dependency expectedDependency : expectedDependencies) { - if (!declaredDependencies.contains(expectedDependency)) { - addProblem( - problemsBySeverity, - Severity.CRITICAL, - String.format("Dependency %s is not listed.", expectedDependency)); - } - } - - boolean moduleDefinesExcessDependencies = - MODULES_DEFINING_EXCESS_DEPENDENCIES.contains(moduleName); - - // find all dependencies defined in NOTICE file, which were not expected - for (Dependency declaredDependency : declaredDependencies) { - if (!expectedDependencies.contains(declaredDependency)) { - final Severity severity = - moduleDefinesExcessDependencies ? Severity.SUPPRESSED : Severity.TOLERATED; - addProblem( - problemsBySeverity, - severity, - String.format( - "Dependency %s is not bundled, but listed.", declaredDependency)); - } - } + final Map<Severity, List<String>> problemsBySeverity = + checkNoticeFile( + modulesWithShadedDependencies, moduleName, noticeContents.orElse(null)); final List<String> severeProblems = problemsBySeverity.getOrDefault(Severity.CRITICAL, Collections.emptyList()); @@ -257,12 +211,80 @@ public class NoticeFileChecker { return severeProblems.size(); } + @VisibleForTesting + static Map<Severity, List<String>> checkNoticeFile( + Multimap<String, Dependency> modulesWithShadedDependencies, + String moduleName, + @Nullable NoticeContents noticeContents) { + + final Map<Severity, List<String>> problemsBySeverity = new HashMap<>(); + + if (noticeContents == null) { + addProblem(problemsBySeverity, Severity.CRITICAL, "The NOTICE file was empty."); + } else { + // first line must be the module name. + if (!noticeContents.getNoticeModuleName().equals(moduleName)) { + addProblem( + problemsBySeverity, + Severity.TOLERATED, + String.format( + "First line does not start with module name. firstLine=%s", + noticeContents.getNoticeModuleName())); + } + + // collect all declared dependencies from NOTICE file + Set<Dependency> declaredDependencies = new HashSet<>(); + for (Dependency declaredDependency : noticeContents.getDeclaredDependencies()) { + if (!declaredDependencies.add(declaredDependency)) { + addProblem( + problemsBySeverity, + Severity.CRITICAL, + String.format("Dependency %s is declared twice.", declaredDependency)); + } + } + + // find all dependencies missing from NOTICE file + Collection<Dependency> expectedDependencies = + modulesWithShadedDependencies.get(moduleName); + for (Dependency expectedDependency : expectedDependencies) { + if (!declaredDependencies.contains(expectedDependency)) { + addProblem( + problemsBySeverity, + Severity.CRITICAL, + String.format("Dependency %s is not listed.", expectedDependency)); + } + } + + boolean moduleDefinesExcessDependencies = + MODULES_DEFINING_EXCESS_DEPENDENCIES.contains(moduleName); + + // find all dependencies defined in NOTICE file, which were not expected + for (Dependency declaredDependency : declaredDependencies) { + if (!expectedDependencies.contains(declaredDependency)) { + final Severity severity = + moduleDefinesExcessDependencies + ? Severity.SUPPRESSED + : Severity.TOLERATED; + addProblem( + problemsBySeverity, + severity, + String.format( + "Dependency %s is not bundled, but listed.", + declaredDependency)); + } + } + } + + return problemsBySeverity; + } + private static void addProblem( Map<Severity, List<String>> problemsBySeverity, Severity severity, String problem) { problemsBySeverity.computeIfAbsent(severity, ignored -> new ArrayList<>()).add(problem); } - private enum Severity { + @VisibleForTesting + enum Severity { /** Issues that a legally problematic which must be fixed. */ CRITICAL, /** Issues that affect the correctness but aren't legally problematic. */ 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 new file mode 100644 index 00000000000..7b08642571d --- /dev/null +++ b/tools/ci/flink-ci-tools/src/test/java/org/apache/flink/tools/ci/licensecheck/NoticeFileCheckerTest.java @@ -0,0 +1,99 @@ +/* + * 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.flink.tools.ci.licensecheck; + +import org.apache.flink.tools.ci.utils.notice.NoticeContents; +import org.apache.flink.tools.ci.utils.shared.Dependency; + +import com.google.common.collect.ArrayListMultimap; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; + +import static org.assertj.core.api.Assertions.assertThat; + +class NoticeFileCheckerTest { + + @Test + void testCheckNoticeFileHappyPath() { + final String moduleName = "test"; + final Dependency bundledDependency = Dependency.create("a", "b", "c"); + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, bundledDependency); + + assertThat( + NoticeFileChecker.checkNoticeFile( + bundleDependencies, + moduleName, + new NoticeContents( + moduleName, Collections.singletonList(bundledDependency)))) + .isEmpty(); + } + + @Test + void testCheckNoticeFileRejectsEmptyFile() { + assertThat(NoticeFileChecker.checkNoticeFile(ArrayListMultimap.create(), "test", null)) + .containsOnlyKeys(NoticeFileChecker.Severity.CRITICAL); + } + + @Test + void testCheckNoticeFileToleratesModuleNameMismatch() { + final String moduleName = "test"; + + assertThat( + NoticeFileChecker.checkNoticeFile( + ArrayListMultimap.create(), + moduleName, + new NoticeContents(moduleName + "2", Collections.emptyList()))) + .containsOnlyKeys(NoticeFileChecker.Severity.TOLERATED); + } + + @Test + void testCheckNoticeFileRejectsDuplicateLine() { + final String moduleName = "test"; + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, Dependency.create("a", "b", "c")); + + assertThat( + NoticeFileChecker.checkNoticeFile( + bundleDependencies, + moduleName, + new NoticeContents( + moduleName, + Arrays.asList( + Dependency.create("a", "b", "c"), + Dependency.create("a", "b", "c"))))) + .containsOnlyKeys(NoticeFileChecker.Severity.CRITICAL); + } + + @Test + void testCheckNoticeFileRejectsMissingDependency() { + final String moduleName = "test"; + final ArrayListMultimap<String, Dependency> bundleDependencies = ArrayListMultimap.create(); + bundleDependencies.put(moduleName, Dependency.create("a", "b", "c")); + + assertThat( + NoticeFileChecker.checkNoticeFile( + bundleDependencies, + moduleName, + new NoticeContents(moduleName, Collections.emptyList()))) + .containsOnlyKeys(NoticeFileChecker.Severity.CRITICAL); + } +} diff --git a/tools/maven/suppressions.xml b/tools/maven/suppressions.xml index 7f92e6f6c88..a6d9158534b 100644 --- a/tools/maven/suppressions.xml +++ b/tools/maven/suppressions.xml @@ -31,6 +31,7 @@ under the License. <suppress files="NoticeFileChecker.java" checks="Regexp"/> <suppress files="NoticeFileChecker.java" checks="IllegalImport"/> + <suppress files="NoticeFileCheckerTest.java" checks="IllegalImport"/> <suppress files="JoinOperator.java" checks="FileLength"/> <suppress files="WindowOperatorTest.java" checks="FileLength"/>
