This is an automated email from the ASF dual-hosted git repository. echauchot pushed a commit to branch ci_utils in repository https://gitbox.apache.org/repos/asf/flink-connector-shared-utils.git
commit 30d0b106eb1df8db2875f24d2353c28920af468e Author: Etienne Chauchot <[email protected]> AuthorDate: Mon Oct 9 16:28:45 2023 +0200 [FLINK-34137] Setup CI and archunit for archunit tests skipping --- .github/workflows/_testing.yml | 21 ++++++++-- .github/workflows/ci.yml | 10 +++++ .../06adde03-b226-46a1-880e-e3f9d0fbcfcb | 0 .../2b408df1-117c-43ee-8829-11a33f2f4c3d | 0 .../47150079-df0b-4320-b1a9-117804d1d2b9 | 0 .../61e2247d-4ef0-4327-ad7f-2dc5cd881a4d | 0 .../737d7692-7d8e-4a4e-94d4-61e573619ec4 | 0 .../9193bee2-f400-432f-b664-7ed218451dc9 | 0 .../a6ff808a-1aeb-4ff3-8fe6-3a5cb49fba71 | 0 .../acfbc678-b208-4e82-b2c9-09b53dc0eec5 | 0 archunit-violations/stored.rules | 10 +++++ pom.xml | 36 ++++++++++++++++- .../apache/flink/connector/testing/SomeClass.java | 11 +++++- .../ProductionCodeArchitectureTest.java | 42 ++++++++++++++++++++ .../architecture/TestCodeArchitectureTest.java | 46 ++++++++++++++++++++++ src/test/resources/archunit.properties | 38 ++++++++++++++++++ 16 files changed, 209 insertions(+), 5 deletions(-) diff --git a/.github/workflows/_testing.yml b/.github/workflows/_testing.yml index aca4d74..84eb71d 100644 --- a/.github/workflows/_testing.yml +++ b/.github/workflows/_testing.yml @@ -25,13 +25,28 @@ jobs: specific-version: uses: ./.github/workflows/ci.yml with: - flink_version: 1.16.1 + flink_version: 1.17.1 connector_branch: ci_utils - snapshot-version: + # we can test connectors against flink snapshots but enable archunit tests only when tested against + # the flink version used for the connector build. + snapshot-without-archunit-tests: uses: ./.github/workflows/ci.yml with: flink_version: 1.16-SNAPSHOT - connector_branch: ci_utils + skip_archunit_tests: true + # we need to test the connectors against last 2 flink major versions. + # let's say the connector under test was built against flink 1.17.1, disable the archunit tests when tested against 1.16.2 + non-main-version-without-archunit-tests: + uses: ./.github/workflows/ci.yml + with: + flink_version: 1.16.2 + skip_archunit_tests: true + # we need to test the connectors against last 2 flink major versions. + # let's say the connector under test was built against flink 1.17.1, leave the archunit tests enabled when tested against 1.17.1 + main-version-with-archunit-tests: + uses: ./.github/workflows/ci.yml + with: + flink_version: 1.17.1 flink118-java17-version: uses: ./.github/workflows/ci.yml with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 418998d..bc12766 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -51,6 +51,11 @@ on: required: false type: boolean default: true + skip_archunit_tests: + description: "Whether to skip the archunit tests" + required: false + type: boolean + default: false connector_branch: description: "Branch that need to be checked out" required: false @@ -101,6 +106,10 @@ jobs: if: ${{ inputs.optional_maven_profiles }} run: echo "MVN_COMMON_OPTIONS=${MVN_COMMON_OPTIONS} -P ${{ inputs.optional_maven_profiles }}" >> $GITHUB_ENV + - name: "Disable archunit tests" + if: ${{ inputs.skip_archunit_tests }} + run: echo "MVN_ARCHUNIT_TESTS=-Dtest='!*ArchitectureTest'" >> $GITHUB_ENV + - name: "Determine Flink binary url" run: | binary_url=${{ inputs.flink_url }} @@ -159,6 +168,7 @@ jobs: -Dscala-2.12 \ -Prun-end-to-end-tests -DdistDir=${{ env.FLINK_CACHE_DIR }}/flink-${{ inputs.flink_version }} \ ${{ env.MVN_DEPENDENCY_CONVERGENCE }} \ + ${{ env.MVN_ARCHUNIT_TESTS }} \ ${{ env.MVN_CONNECTION_OPTIONS }} \ -Dlog4j.configurationFile=file://$(pwd)/tools/ci/log4j.properties \ | tee ${{ env.MVN_BUILD_OUTPUT_FILE }} diff --git a/archunit-violations/06adde03-b226-46a1-880e-e3f9d0fbcfcb b/archunit-violations/06adde03-b226-46a1-880e-e3f9d0fbcfcb new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/2b408df1-117c-43ee-8829-11a33f2f4c3d b/archunit-violations/2b408df1-117c-43ee-8829-11a33f2f4c3d new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/47150079-df0b-4320-b1a9-117804d1d2b9 b/archunit-violations/47150079-df0b-4320-b1a9-117804d1d2b9 new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/61e2247d-4ef0-4327-ad7f-2dc5cd881a4d b/archunit-violations/61e2247d-4ef0-4327-ad7f-2dc5cd881a4d new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/737d7692-7d8e-4a4e-94d4-61e573619ec4 b/archunit-violations/737d7692-7d8e-4a4e-94d4-61e573619ec4 new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/9193bee2-f400-432f-b664-7ed218451dc9 b/archunit-violations/9193bee2-f400-432f-b664-7ed218451dc9 new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/a6ff808a-1aeb-4ff3-8fe6-3a5cb49fba71 b/archunit-violations/a6ff808a-1aeb-4ff3-8fe6-3a5cb49fba71 new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/acfbc678-b208-4e82-b2c9-09b53dc0eec5 b/archunit-violations/acfbc678-b208-4e82-b2c9-09b53dc0eec5 new file mode 100644 index 0000000..e69de29 diff --git a/archunit-violations/stored.rules b/archunit-violations/stored.rules new file mode 100644 index 0000000..4c2eeb9 --- /dev/null +++ b/archunit-violations/stored.rules @@ -0,0 +1,10 @@ +# +#Wed Oct 11 15:35:58 CEST 2023 +Return\ and\ argument\ types\ of\ methods\ annotated\ with\ @Public\ must\ be\ annotated\ with\ @Public.=2b408df1-117c-43ee-8829-11a33f2f4c3d +Connector\ production\ code\ must\ not\ depend\ on\ non-public\ API\ outside\ of\ connector\ packages=47150079-df0b-4320-b1a9-117804d1d2b9 +ITCASE\ tests\ should\ use\ a\ MiniCluster\ resource\ or\ extension=acfbc678-b208-4e82-b2c9-09b53dc0eec5 +Production\ code\ must\ not\ call\ methods\ annotated\ with\ @VisibleForTesting=a6ff808a-1aeb-4ff3-8fe6-3a5cb49fba71 +Tests\ inheriting\ from\ AbstractTestBase\ should\ have\ name\ ending\ with\ ITCase=06adde03-b226-46a1-880e-e3f9d0fbcfcb +Options\ for\ connectors\ and\ formats\ should\ reside\ in\ a\ consistent\ package\ and\ be\ public\ API.=737d7692-7d8e-4a4e-94d4-61e573619ec4 +Return\ and\ argument\ types\ of\ methods\ annotated\ with\ @PublicEvolving\ must\ be\ annotated\ with\ @Public(Evolving).=61e2247d-4ef0-4327-ad7f-2dc5cd881a4d +Classes\ in\ API\ packages\ should\ have\ at\ least\ one\ API\ visibility\ annotation.=9193bee2-f400-432f-b664-7ed218451dc9 diff --git a/pom.xml b/pom.xml index fa1b1d6..7544839 100644 --- a/pom.xml +++ b/pom.xml @@ -33,7 +33,7 @@ under the License. <packaging>jar</packaging> <properties> - <flink.version>1.16.1</flink.version> + <flink.version>1.17.1</flink.version> <japicmp.referenceVersion>1.16.0</japicmp.referenceVersion> </properties> @@ -48,7 +48,41 @@ under the License. <artifactId>flink-test-utils-junit</artifactId> <version>${flink.version}</version> <scope>test</scope> + <!-- for dependency convergence --> + <exclusions> + <exclusion> + <groupId>org.apache.commons</groupId> + <artifactId>commons-compress</artifactId> + </exclusion> + </exclusions> </dependency> + + <!-- ArchUit test dependencies --> + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-architecture-tests-test</artifactId> + <version>${flink.version}</version> + <scope>test</scope> + <!-- for dependency convergence --> + <exclusions> + <exclusion> + <groupId>com.esotericsoftware.kryo</groupId> + <artifactId>kryo</artifactId> + </exclusion> + <exclusion> + <groupId>org.junit.platform</groupId> + <artifactId>junit-platform-engine</artifactId> + </exclusion> + </exclusions> + </dependency> + + <dependency> + <groupId>org.apache.flink</groupId> + <artifactId>flink-architecture-tests-production</artifactId> + <version>${flink.version}</version> + <scope>test</scope> + </dependency> + </dependencies> <dependencyManagement> diff --git a/src/main/java/org/apache/flink/connector/testing/SomeClass.java b/src/main/java/org/apache/flink/connector/testing/SomeClass.java index 36da836..8285bb4 100644 --- a/src/main/java/org/apache/flink/connector/testing/SomeClass.java +++ b/src/main/java/org/apache/flink/connector/testing/SomeClass.java @@ -18,5 +18,14 @@ package org.apache.flink.connector.testing; +import static org.apache.flink.util.Preconditions.checkNotNull; + /** A dummy class; only exists so that the compile/packaging plugins have something to do. */ -public class SomeClass {} +public class SomeClass { + public static void main(String[] args) { + // this call is an intended (for testing) violation of the + // ConnectorRules#CONNECTOR_CLASSES_ONLY_DEPEND_ON_PUBLIC_API archunit rule that was fixed + // in Flink 1.18 + checkNotNull(new Object()); + } +} diff --git a/src/test/java/org/apache/flink/connector/testing/architecture/ProductionCodeArchitectureTest.java b/src/test/java/org/apache/flink/connector/testing/architecture/ProductionCodeArchitectureTest.java new file mode 100644 index 0000000..6f80e05 --- /dev/null +++ b/src/test/java/org/apache/flink/connector/testing/architecture/ProductionCodeArchitectureTest.java @@ -0,0 +1,42 @@ +/* + * 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.connector.testing.architecture; + +import org.apache.flink.architecture.ProductionCodeArchitectureBase; +import org.apache.flink.architecture.common.ImportOptions; + +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.junit.ArchTests; + +/** product code Architecture tests. */ +@AnalyzeClasses( + packages = "org.apache.flink.connector", + importOptions = { + ImportOption.DoNotIncludeTests.class, + ImportOption.DoNotIncludeArchives.class, + ImportOptions.ExcludeScalaImportOption.class, + ImportOptions.ExcludeShadedImportOption.class + }) +public class ProductionCodeArchitectureTest { + + @ArchTest + public static final ArchTests COMMON_TESTS = ArchTests.in(ProductionCodeArchitectureBase.class); +} diff --git a/src/test/java/org/apache/flink/connector/testing/architecture/TestCodeArchitectureTest.java b/src/test/java/org/apache/flink/connector/testing/architecture/TestCodeArchitectureTest.java new file mode 100644 index 0000000..7357fcc --- /dev/null +++ b/src/test/java/org/apache/flink/connector/testing/architecture/TestCodeArchitectureTest.java @@ -0,0 +1,46 @@ +/* + * 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.connector.testing.architecture; + +import org.apache.flink.architecture.TestCodeArchitectureTestBase; +import org.apache.flink.architecture.common.ImportOptions; + +import com.tngtech.archunit.core.importer.ImportOption; +import com.tngtech.archunit.junit.AnalyzeClasses; +import com.tngtech.archunit.junit.ArchTest; +import com.tngtech.archunit.junit.ArchTests; + +/** Architecture tests for test code. */ +@AnalyzeClasses( + packages = { + "org.apache.flink.batch.connectors.cassandra", + "org.apache.flink.streaming.connectors.cassandra", + "org.apache.flink.connector.cassandra", + "org.apache.flink.connectors.cassandra" + }, + importOptions = { + ImportOption.OnlyIncludeTests.class, + ImportOptions.ExcludeScalaImportOption.class, + ImportOptions.ExcludeShadedImportOption.class + }) +public class TestCodeArchitectureTest { + + @ArchTest + public static final ArchTests COMMON_TESTS = ArchTests.in(TestCodeArchitectureTestBase.class); +} diff --git a/src/test/resources/archunit.properties b/src/test/resources/archunit.properties new file mode 100644 index 0000000..07b2ea7 --- /dev/null +++ b/src/test/resources/archunit.properties @@ -0,0 +1,38 @@ +# +# 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. +# + +# This controls if the violation store is writable +freeze.store.default.allowStoreUpdate=true + +# Enable this if a new rule was added or the description of an existing rule has changed. +# It is needed to record future violations of this rule. +# If omitted, future violations of the rule will not be considered as violations as the initial store was not created for this rule. +#freeze.store.default.allowStoreCreation=true + +# Enable this to record the current state of violations. +# This can make sense, because current violations are consciously accepted and should be added to the store +# By default we allow removing existing violations, but fail when new violations are added +# NOTE: Adding new violations should be avoided when possible. If the rule was correct to flag a new +# violation, please try to avoid creating the violation. If the violation was created due to a +# shortcoming of the rule, file a JIRA issue so the rule can be improved. +#freeze.refreeze=true + +freeze.store.default.path=archunit-violations + +# To allow all rules to be evaluated without checking any classes you can set the following property +archRule.failOnEmptyShould = false
