fix(ci): drop dead build artifact, guard an NPE in a catch block, and make integration tests report their failures

.github/actions/core-cicd/deployment/deploy-javadoc/action.yml

@@ -49,7 +49,6 @@ runs:
         maven-args: "javadoc:javadoc -pl :dotcms-core"
         generate-docker: false
         cleanup-runner: true
-        restore-classes: true
         artifacts-from: ${{ inputs.artifact-run-id }}
         github-token: ${{ inputs.github-token }}
 

.github/actions/core-cicd/maven-job/README.md

@@ -27,7 +27,6 @@ This GitHub Action sets up and runs a Maven job with extensive configuration opt
 | `dotcms-license` | The license key for dotCMS | No | `''` |
 | `artifacts-from` | Download artifacts from a previous job | No | `''` |
 | `github-token` | GitHub token for authentication | Yes | - |
-| `restore-classes` | Restore build classes | No | `false` |
 | `stage-name` | Stage name for the build | Yes | - |
 | `maven-args` | Arguments for Maven build | Yes | - |
 | `generates-test-results` | Generate test results artifacts | No | `false` |

.github/actions/core-cicd/maven-job/action.yml

@@ -61,10 +61,6 @@ inputs:
   github-token:
     description: 'GitHub token for authentication'
     required: true
-  restore-classes:
-    description: 'Restore build classes'
-    required: false
-    default: 'false'
   stage-name:
     description: 'Stage name for the build'
     required: true
@@ -262,15 +258,6 @@ runs:
       if: ${{ inputs.needs-docker-image == 'true' }}
       run: docker load < /tmp/docker-image/image.tar
 
-    - id: restore-artifact-classes
-      name: Restore Classes
-      if: ${{ inputs.restore-classes == 'true' }}
-      uses: actions/download-artifact@v4
-      with:
-        run-id: ${{ inputs.artifacts-from }}
-        github-token: ${{ inputs.github-token }}
-        name: build-classes${{ steps.artifact-suffix.outputs.suffix }}
-
     - name: Docker Hub Login
       if: ${{ inputs.docker-io-username != '' && inputs.docker-io-token != '' }}
       uses: docker/login-action@v3.0.0
@@ -383,18 +370,6 @@ runs:
         name: docker-image${{ steps.artifact-suffix.outputs.suffix }}
         path: /tmp/image.tar
 
-    - id: persist-build-classes
-      name: Persist Build Classes
-      if: ${{ inputs.generate-artifacts == 'true' }}
-      uses: actions/upload-artifact@v4
-      with:
-        name: build-classes${{ steps.artifact-suffix.outputs.suffix }}
-        path: |
-          **/target/classes/**/*.class
-          **/target/generated-sources/**/*.java
-          **/target/test-classes/**/*.class
-          LICENSE
-
     - id: delete-built-artifacts-from-cache
       name: Delete Built Artifacts From Cache
       if: ${{ inputs.generate-artifacts == 'true' && steps.restore-cache-maven.outputs.cache-hit != 'true' }}

.github/scripts/test-balance/find_swallowed.py

@@ -0,0 +1,80 @@
+#!/usr/bin/env python3
+"""Find integration tests that fail without reporting why.
+
+The pattern that just cost a CI run:
+
+    try  { doThing(); }
+    catch (Exception e) { Assert.assertTrue("No Exception should be thrown", false); }
+
+The test fails, but the caught exception is discarded, so the CI log says nothing
+about the actual cause. Retries just repeat the empty message.
+
+Classifies each catch block whose body FAILS the test:
+  SWALLOWED  - fails, never references the exception variable  (the bad case)
+  REPORTED   - fails and passes the exception / its message along
+Also flags empty catch blocks, which hide failures entirely.
+"""
+import os, re, os, glob, collections
+
+ROOT = os.path.join(os.environ.get("REPO_ROOT", os.getcwd()), "dotcms-integration/src/test/java")
+
+CATCH = re.compile(r'catch\s*\(\s*([\w.]+(?:\s*\|\s*[\w.]+)*)\s+(\w+)\s*\)\s*\{')
+# something that makes the test fail
+FAILS = re.compile(r'\b(fail|assertTrue|assertFalse|assertEquals|assertNotNull|Assert\.fail)\b')
+# an unconditional failure, i.e. the catch exists only to fail
+HARD_FAIL = re.compile(
+    r'\bfail\s*\(|\bAssert\.fail\s*\(|assertTrue\s*\([^;]*,\s*false\s*\)|assertFalse\s*\([^;]*,\s*true\s*\)')
+
+
+def body_of(src, open_idx):
+    """Return the text inside the braces starting at open_idx (index of '{')."""
+    depth, i = 0, open_idx
+    while i < len(src):
+        if src[i] == '{':
+            depth += 1
+        elif src[i] == '}':
+            depth -= 1
+            if depth == 0:
+                return src[open_idx + 1:i]
+        i += 1
+    return ''
+
+
+swallowed, empty, reported = [], [], []
+for path in glob.glob(ROOT + "/**/*.java", recursive=True):
+    src = open(path, errors='ignore').read()
+    rel = path[len(ROOT) + 1:]
+    for m in CATCH.finditer(src):
+        var = m.group(2)
+        body = body_of(src, m.end() - 1)
+        line = src[:m.start()].count('\n') + 1
+        stripped = re.sub(r'//[^\n]*|/\*.*?\*/', '', body, flags=re.S).strip()
+
+        if not stripped:
+            empty.append((rel, line, m.group(1)))
+            continue
+        if not HARD_FAIL.search(stripped):
+            continue                      # catch doesn't unconditionally fail
+        # does it carry the exception anywhere?
+        carries = re.search(rf'\b{re.escape(var)}\b', stripped)
+        (reported if carries else swallowed).append((rel, line, m.group(1), stripped[:90]))
+
+print(f"scanned {len(glob.glob(ROOT + '/**/*.java', recursive=True))} integration test files\n")
+print(f"SWALLOWED (fails without reporting the cause): {len(swallowed)}")
+print(f"REPORTED  (fails and includes the exception):  {len(reported)}")
+print(f"EMPTY catch blocks (hide failures entirely):   {len(empty)}\n")
+
+by_file = collections.Counter(f for f, *_ in swallowed)
+print("=== worst files (swallowed catches) ===")
+for f, n in by_file.most_common(20):
+    print(f"  {n:>3}  {f}")
+
+print("\n=== every swallowed catch ===")
+for f, line, exc, snippet in sorted(swallowed):
+    one = ' '.join(snippet.split())
+    print(f"  {f}:{line}  catch({exc})  ->  {one[:80]}")
+
+if empty:
+    print("\n=== empty catch blocks ===")
+    for f, line, exc in sorted(empty)[:25]:
+        print(f"  {f}:{line}  catch({exc}) {{}}")

dotCMS/src/main/java/com/dotcms/publisher/endpoint/bean/impl/AWSS3PublishingEndPoint.java

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.liferay.portal.language.LanguageUtil;
+import com.liferay.portal.model.User;
 import com.liferay.portal.util.PortalUtil;
 import java.io.IOException;
 import java.io.StringReader;
@@ -111,8 +112,18 @@ public void validatePublishingEndPoint() throws PublishingEndPointValidationExce
             final SystemMessageBuilder systemMessageBuilder = new SystemMessageBuilder();
             SystemMessage systemMessage = systemMessageBuilder.setMessage("Unable to verify S3 Endpoint. Please check your configuration:" + e.getMessage()).setType(MessageType.SIMPLE_MESSAGE)
                             .setSeverity(MessageSeverity.WARNING).setLife(100000).create();
-            SystemMessageEventUtil.getInstance().pushMessage(systemMessage, ImmutableList.of(PortalUtil.getUser().getUserId()));
 
+            // PortalUtil.getUser() returns null whenever no request is bound to the
+            // thread - scheduled jobs, background tasks, tests. Dereferencing it here
+            // threw a NullPointerException out of this catch block, which replaced the
+            // S3 error we are trying to report with a confusing NPE and defeated the
+            // point of catching at all. The message is a UI notification, so when
+            // there is no user to notify, the logged warning above is the whole story.
+            final User currentUser = PortalUtil.getUser();
+            if (currentUser != null) {
+                SystemMessageEventUtil.getInstance().pushMessage(systemMessage,
+                        ImmutableList.of(currentUser.getUserId()));
+            }
         }
 
     } //validatePublishingEndPoint.