Remove separate docs GitHub workflow

.github/bin/build-matrix-from-impacted.py

@@ -98,6 +98,9 @@ def build(matrix_file, impacted_file, output_file):
 def check_modules(modules, impacted):
     if isinstance(modules, str):
         modules = [modules]
+    # `impacted` can be empty when GIB detected no changes, but also when GIB was not run at all.
+    # The latter is the case on builds on master branch. For these builds we want to run all tests,
+    # so we don't filter out any modules.
     if impacted and not any(module in impacted for module in modules):
         return None
     # concatenate because matrix values should be primitives

.github/workflows/ci.yml

@@ -5,9 +5,6 @@ on:
     branches:
       - master
   pull_request:
-    paths-ignore:
-      - 'docs/**'
-      - '**.md'
   repository_dispatch:
     types: [test-with-secrets-command]
 
@@ -50,38 +47,18 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
-  build-success:
-    if: ${{ always() }} # if `failure()` would not work for cancellations, `!success()` would not work for skipped jobs
+  path-filters:
     runs-on: ubuntu-latest
-    needs:
-      - artifact-checks
-      - build-pt
-      - build-test-matrix
-      - check-commit
-      - check-commits-dispatcher
-      - error-prone-checks
-      - hive-tests
-      - maven-checks
-      - pt
-      - test
-      - test-jdbc-compatibility
-      - test-other-modules
+    outputs:
+      docs: ${{ steps.filter.outputs.docs }}
+      non_docs: ${{ steps.filter.outputs.non_docs }}
     steps:
-      - name: "Check results"
-        run: |
-          # generated by TestCiWorkflow
-          echo '${{ needs.artifact-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "artifact-checks" failed' >&2; exit 1; }
-          echo '${{ needs.build-pt.result }}' | grep -xE 'success|skipped' || { echo 'Job "build-pt" failed' >&2; exit 1; }
-          echo '${{ needs.build-test-matrix.result }}' | grep -xE 'success|skipped' || { echo 'Job "build-test-matrix" failed' >&2; exit 1; }
-          echo '${{ needs.check-commit.result }}' | grep -xE 'success|skipped' || { echo 'Job "check-commit" failed' >&2; exit 1; }
-          echo '${{ needs.check-commits-dispatcher.result }}' | grep -xE 'success|skipped' || { echo 'Job "check-commits-dispatcher" failed' >&2; exit 1; }
-          echo '${{ needs.error-prone-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "error-prone-checks" failed' >&2; exit 1; }
-          echo '${{ needs.hive-tests.result }}' | grep -xE 'success|skipped' || { echo 'Job "hive-tests" failed' >&2; exit 1; }
-          echo '${{ needs.maven-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "maven-checks" failed' >&2; exit 1; }
-          echo '${{ needs.pt.result }}' | grep -xE 'success|skipped' || { echo 'Job "pt" failed' >&2; exit 1; }
-          echo '${{ needs.test.result }}' | grep -xE 'success|skipped' || { echo 'Job "test" failed' >&2; exit 1; }
-          echo '${{ needs.test-jdbc-compatibility.result }}' | grep -xE 'success|skipped' || { echo 'Job "test-jdbc-compatibility" failed' >&2; exit 1; }
-          echo '${{ needs.test-other-modules.result }}' | grep -xE 'success|skipped' || { echo 'Job "test-other-modules" failed' >&2; exit 1; }
+      - uses: dorny/paths-filter@de90cc6fb38fc0963ad72b210f1f284cd68cea36 # v3
+        id: filter
+        with:
+          filters: |
+            docs: 'docs/**'
+            non_docs: '!docs/**'
 
   maven-checks:
     runs-on: ubuntu-latest
@@ -122,6 +99,8 @@ jobs:
         run: rm -rf ~/.m2/repository/io/trino/trino-*
 
   artifact-checks:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 45
     steps:
@@ -155,8 +134,9 @@ jobs:
         run: core/docker/build.sh
 
   check-commits-dispatcher:
+    needs: path-filters
+    if: github.event_name == 'pull_request' && needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
-    if: github.event_name == 'pull_request'
     outputs:
       matrix: ${{ steps.set-matrix.outputs.matrix }}
     steps:
@@ -208,6 +188,8 @@ jobs:
           base_ref: ${{ github.event.pull_request.base.ref }}
 
   error-prone-checks:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 45
     steps:
@@ -236,6 +218,8 @@ jobs:
             -pl '!:trino-docs,!:trino-server'
 
   test-jdbc-compatibility:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 30
     steps:
@@ -276,6 +260,8 @@ jobs:
           upload-heap-dump: ${{ env.SECRETS_PRESENT == '' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}
 
   hive-tests:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     strategy:
       fail-fast: false
@@ -332,6 +318,8 @@ jobs:
           github_token: ${{ secrets.GITHUB_TOKEN }}
 
   test-other-modules:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     timeout-minutes: 60
     steps:
@@ -419,6 +407,8 @@ jobs:
           upload-heap-dump: ${{ env.SECRETS_PRESENT == '' && github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository }}
 
   build-test-matrix:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.set-matrix.outputs.matrix }}
@@ -446,7 +436,7 @@ jobs:
       - name: Maven validate
         run: |
           export MAVEN_OPTS="${MAVEN_INSTALL_OPTS}"
-          $MAVEN validate ${MAVEN_FAST_INSTALL} ${MAVEN_GIB} -Dgib.logImpactedTo=gib-impacted.log -P disable-check-spi-dependencies -pl '!:trino-docs'
+          $MAVEN validate ${MAVEN_FAST_INSTALL} ${MAVEN_GIB} -Dgib.logImpactedTo=gib-impacted.log -P disable-check-spi-dependencies
       - name: Set matrix
         id: set-matrix
         run: |
@@ -842,6 +832,8 @@ jobs:
           github_token: ${{ secrets.GITHUB_TOKEN }}
 
   build-pt:
+    needs: path-filters
+    if: needs.path-filters.outputs.non_docs == 'true'
     runs-on: ubuntu-latest
     outputs:
       matrix: ${{ steps.set-matrix.outputs.matrix }}
@@ -1122,3 +1114,38 @@ jobs:
           check_name: ${{ github.job }} with secrets
           conclusion: ${{ job.status }}
           github_token: ${{ secrets.GITHUB_TOKEN }}
+
+  build-success:
+    if: ${{ always() }} # if `failure()` would not work for cancellations, `!success()` would not work for skipped jobs
+    runs-on: ubuntu-latest
+    needs:
+      - artifact-checks
+      - build-pt
+      - build-test-matrix
+      - check-commit
+      - check-commits-dispatcher
+      - error-prone-checks
+      - hive-tests
+      - maven-checks
+      - path-filters
+      - pt
+      - test
+      - test-jdbc-compatibility
+      - test-other-modules
+    steps:
+      - name: "Check results"
+        run: |
+          # generated by TestCiWorkflow
+          echo '${{ needs.artifact-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "artifact-checks" failed' >&2; exit 1; }
+          echo '${{ needs.build-pt.result }}' | grep -xE 'success|skipped' || { echo 'Job "build-pt" failed' >&2; exit 1; }
+          echo '${{ needs.build-test-matrix.result }}' | grep -xE 'success|skipped' || { echo 'Job "build-test-matrix" failed' >&2; exit 1; }
+          echo '${{ needs.check-commit.result }}' | grep -xE 'success|skipped' || { echo 'Job "check-commit" failed' >&2; exit 1; }
+          echo '${{ needs.check-commits-dispatcher.result }}' | grep -xE 'success|skipped' || { echo 'Job "check-commits-dispatcher" failed' >&2; exit 1; }
+          echo '${{ needs.error-prone-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "error-prone-checks" failed' >&2; exit 1; }
+          echo '${{ needs.hive-tests.result }}' | grep -xE 'success|skipped' || { echo 'Job "hive-tests" failed' >&2; exit 1; }
+          echo '${{ needs.maven-checks.result }}' | grep -xE 'success|skipped' || { echo 'Job "maven-checks" failed' >&2; exit 1; }
+          echo '${{ needs.path-filters.result }}' | grep -xE 'success|skipped' || { echo 'Job "path-filters" failed' >&2; exit 1; }
+          echo '${{ needs.pt.result }}' | grep -xE 'success|skipped' || { echo 'Job "pt" failed' >&2; exit 1; }
+          echo '${{ needs.test.result }}' | grep -xE 'success|skipped' || { echo 'Job "test" failed' >&2; exit 1; }
+          echo '${{ needs.test-jdbc-compatibility.result }}' | grep -xE 'success|skipped' || { echo 'Job "test-jdbc-compatibility" failed' >&2; exit 1; }
+          echo '${{ needs.test-other-modules.result }}' | grep -xE 'success|skipped' || { echo 'Job "test-other-modules" failed' >&2; exit 1; }

.github/workflows/cleanup.yml

@@ -18,5 +18,5 @@ jobs:
           # Cancel workflow when PR closed. https://github.com/styfle/cancel-workflow-action#advanced-ignore-sha
           ignore_sha: true
           # Note: workflow_id can be a Workflow ID (number) or Workflow File Name (string) or a comma-separated list of those.
-          workflow_id: "ci.yml,docs.yml"
+          workflow_id: "ci.yml"
           access_token: ${{ github.token }}

docs/pom.xml

@@ -51,7 +51,7 @@
                         <goals>
                             <goal>generate-thrift-idl</goal>
                         </goals>
-                        <phase>validate</phase>
+                        <phase>generate-sources</phase>
                         <configuration>
                             <outputFile>${project.build.directory}/TrinoThriftService.thrift</outputFile>
                             <classes>
@@ -83,7 +83,7 @@
                         <goals>
                             <goal>java</goal>
                         </goals>
-                        <phase>validate</phase>
+                        <phase>test</phase>
                         <configuration>
                             <mainClass>io.trino.sql.ReservedIdentifiers</mainClass>
                             <arguments>
@@ -97,7 +97,7 @@
                         <goals>
                             <goal>exec</goal>
                         </goals>
-                        <phase>validate</phase>
+                        <phase>test</phase>
                         <configuration>
                             <executable>diff</executable>
                             <arguments>

testing/trino-tests/src/test/java/io/trino/tests/ci/TestCiWorkflow.java

@@ -33,6 +33,7 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableMap.toImmutableMap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
+import static com.google.common.collect.Iterables.getLast;
 import static com.google.common.collect.MoreCollectors.onlyElement;
 import static java.util.Locale.ENGLISH;
 import static java.util.function.Predicate.not;
@@ -43,6 +44,7 @@ public class TestCiWorkflow
     private static final Logger log = Logger.get(TestCiWorkflow.class);
 
     private static final Path CI_YML_REPO_PATH = Paths.get(".github/workflows/ci.yml");
+    private static final String BUILD_SUCCESS = "build-success";
 
     @Test
     public void testUploadTestResultsCondition()
@@ -101,24 +103,22 @@ public void testUploadTestResultsCondition()
     public void testBuildSuccessDependencies()
             throws Exception
     {
-        String buildSuccessJobName = "build-success";
-
         Yaml yaml = new Yaml();
         Map<?, ?> workflow = yaml.load(new StringReader(Files.readString(findRepositoryRoot().resolve(CI_YML_REPO_PATH))));
         Map<String, ?> jobs = getMap(workflow, "jobs");
 
         Set<String> allJobNames = jobs.keySet();
-        assertThat(allJobNames).as("allJobNames").contains(buildSuccessJobName);
+        assertThat(allJobNames).as("allJobNames").contains(BUILD_SUCCESS);
         List<String> testJobNames = allJobNames.stream()
-                .filter(not(buildSuccessJobName::equals))
+                .filter(not(BUILD_SUCCESS::equals))
                 .sorted()
                 .toList();
-        Map<?, ?> buildSuccessJob = getMap(jobs, buildSuccessJobName);
+        Map<?, ?> buildSuccessJob = getMap(jobs, BUILD_SUCCESS);
 
         List<String> buildSuccessDependencies = getList(buildSuccessJob, "needs").stream()
                 .map(String.class::cast)
                 .toList();
-        assertThat(buildSuccessDependencies).as("dependencies for %s", buildSuccessJobName)
+        assertThat(buildSuccessDependencies).as("dependencies for %s", BUILD_SUCCESS)
                 .isSorted()
                 .doesNotHaveDuplicates()
                 .containsExactlyElementsOf(testJobNames);
@@ -137,6 +137,19 @@ public void testBuildSuccessDependencies()
                 .isEqualTo(expectedRunDefinition.toString());
     }
 
+    @Test
+    public void testBuildSuccessIsLast()
+            throws Exception
+    {
+        Yaml yaml = new Yaml();
+        Map<?, ?> workflow = yaml.load(new StringReader(Files.readString(findRepositoryRoot().resolve(CI_YML_REPO_PATH))));
+        Map<String, ?> jobs = getMap(workflow, "jobs");
+        // This assumes the `jobs` map preserves source order
+        assertThat(getLast(jobs.keySet()))
+                .describedAs("The %s job is logically last and depends on all others, we want it to be last in the source file", BUILD_SUCCESS)
+                .isEqualTo(BUILD_SUCCESS);
+    }
+
     private static Path findRepositoryRoot()
     {
         Path workingDirectory = Paths.get("").toAbsolutePath();