This is an automated email from the ASF dual-hosted git repository.
hansva pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/hop.git
The following commit(s) were added to refs/heads/main by this push:
new f8dca0e829 avoid UI test hang on GHA (#8152)
f8dca0e829 is described below
commit f8dca0e8297a22b5299be8d6c41423ce29253039
Author: Hans Van Akelyen <[email protected]>
AuthorDate: Fri Aug 28 16:49:29 2026 +0200
avoid UI test hang on GHA (#8152)
* avoid UI test hang on GHA
* pin the selenium version
---
.github/workflows/pr_build_code.yml | 5 +
pom.xml | 3 +
.../hop/ui/testing/SwtBotHarnessTimeoutTest.java | 110 ++++++++++
.../org/apache/hop/ui/testing/SwtBotTestBase.java | 233 ++++++++++++++++++---
web-tests/pom.xml | 2 +
.../org/apache/hop/web/it/HopWebEnvironment.java | 14 +-
6 files changed, 338 insertions(+), 29 deletions(-)
diff --git a/.github/workflows/pr_build_code.yml
b/.github/workflows/pr_build_code.yml
index 84babadd89..e2cb9cdbf8 100644
--- a/.github/workflows/pr_build_code.yml
+++ b/.github/workflows/pr_build_code.yml
@@ -31,6 +31,7 @@ on:
jobs:
build:
runs-on: ubuntu-latest
+ timeout-minutes: 90
steps:
- uses: actions/checkout@v2
@@ -59,6 +60,10 @@ jobs:
# Add a UI and run the UI tests only
ui-tests:
runs-on: ubuntu-latest
+ # A hung SWT event loop used to sit here until GitHub's 6 hour default cut
it off, with no
+ # log to show for it. The harness watchdog and the surefire fork timeout
should fire long
+ # before this does; if this one trips, the hang is outside the test JVM.
+ timeout-minutes: 45
steps:
- uses: actions/checkout@v2
diff --git a/pom.xml b/pom.xml
index 590958e799..3881aaed48 100644
--- a/pom.xml
+++ b/pom.xml
@@ -148,6 +148,7 @@
--add-opens java.security.jgss/sun.security.krb5=ALL-UNNAMED
--add-exports
java.base/sun.nio.ch=ALL-UNNAMED</maven-surefire-plugin.argLine>
<maven-surefire-plugin.forkCount>1</maven-surefire-plugin.forkCount>
+
<maven-surefire-plugin.forkedProcessTimeoutInSeconds>0</maven-surefire-plugin.forkedProcessTimeoutInSeconds>
<maven-surefire-plugin.reuseForks>true</maven-surefire-plugin.reuseForks>
<maven-surefire-plugin.testFailureIgnore>false</maven-surefire-plugin.testFailureIgnore>
<maven.build.timestamp.format>yyyy-MM-dd
hh.mm.ss</maven.build.timestamp.format>
@@ -401,6 +402,7 @@
-DHOP_AUDIT_FOLDER=${project.build.directory}/audit
${maven-surefire-plugin.argLine}
${ui.test.argLine}</argLine>
<forkCount>${maven-surefire-plugin.forkCount}</forkCount>
+
<forkedProcessTimeoutInSeconds>${maven-surefire-plugin.forkedProcessTimeoutInSeconds}</forkedProcessTimeoutInSeconds>
<reuseForks>${maven-surefire-plugin.reuseForks}</reuseForks>
<testFailureIgnore>${maven-surefire-plugin.testFailureIgnore}</testFailureIgnore>
<groups>${ui.test.includedGroups}</groups>
@@ -792,6 +794,7 @@
<profile>
<id>uitest</id>
<properties>
+
<maven-surefire-plugin.forkedProcessTimeoutInSeconds>1200</maven-surefire-plugin.forkedProcessTimeoutInSeconds>
<ui.test.excludedGroups></ui.test.excludedGroups>
<ui.test.includedGroups>uitest</ui.test.includedGroups>
</properties>
diff --git
a/rcp/src/test/java/org/apache/hop/ui/testing/SwtBotHarnessTimeoutTest.java
b/rcp/src/test/java/org/apache/hop/ui/testing/SwtBotHarnessTimeoutTest.java
new file mode 100644
index 0000000000..9fe4469b63
--- /dev/null
+++ b/rcp/src/test/java/org/apache/hop/ui/testing/SwtBotHarnessTimeoutTest.java
@@ -0,0 +1,110 @@
+/*
+ * 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.hop.ui.testing;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.widgets.Display;
+import org.eclipse.swt.widgets.Shell;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The harness's own escape hatch: a UI test that never finishes has to fail
on its own, not sit
+ * there until CI kills the job hours later with nothing to show for it. Both
shapes the harness
+ * offers are covered - a scene whose worker walks away, and a dialog running
its own event loop
+ * that nobody closes - because they hang in different places and only one of
them can be unblocked
+ * from the pump loop.
+ */
+@Tag("uitest")
+class SwtBotHarnessTimeoutTest extends SwtBotTestBase {
+
+ private static final long TEST_TIMEOUT_MILLIS = 1500L;
+
+ private String previousTimeout;
+
+ @BeforeEach
+ void shortenTheDeadline() {
+ previousTimeout =
+ System.setProperty("swtbot.test.timeoutMillis",
Long.toString(TEST_TIMEOUT_MILLIS));
+ }
+
+ @AfterEach
+ void restoreTheDeadline() {
+ if (previousTimeout == null) {
+ System.clearProperty("swtbot.test.timeoutMillis");
+ } else {
+ System.setProperty("swtbot.test.timeoutMillis", previousTimeout);
+ }
+ }
+
+ @Test
+ void sceneWhoseWorkerNeverReturnsFailsInsteadOfHanging() {
+ AssertionError failure =
+ assertThrows(
+ AssertionError.class,
+ () -> withScene(shell -> shell.setText("never finishes"), bot ->
sleepForever()));
+
+ assertTrue(
+ failure.getMessage().contains("did not finish within"),
+ "the harness should report the timeout: " + failure.getMessage());
+ assertTrue(
+ failure.getMessage().contains("worker"),
+ "the failure should say what the threads were doing: " +
failure.getMessage());
+ }
+
+ @Test
+ void dialogNobodyClosesFailsInsteadOfHanging() {
+ AssertionError failure =
+ assertThrows(
+ AssertionError.class,
+ () -> withDialog(this::openDialogThatOnlyClosingCanEnd, bot ->
sleepForever()));
+
+ assertTrue(
+ failure.getMessage().contains("did not finish within"),
+ "the harness should report the timeout: " + failure.getMessage());
+ }
+
+ /** A dialog of the shape the harness drives: its own event loop, running
until it is disposed. */
+ private void openDialogThatOnlyClosingCanEnd(Shell parent) {
+ Shell dialog = new Shell(parent, SWT.DIALOG_TRIM | SWT.APPLICATION_MODAL);
+ dialog.setText("never closed");
+ dialog.setSize(240, 120);
+ dialog.open();
+ Display dialogDisplay = dialog.getDisplay();
+ while (!dialog.isDisposed()) {
+ if (!dialogDisplay.readAndDispatch()) {
+ dialogDisplay.sleep();
+ }
+ }
+ }
+
+ /** Interactions that never hand the UI thread anything back - what a wedged
test looks like. */
+ private static void sleepForever() {
+ try {
+ Thread.sleep(TEST_TIMEOUT_MILLIS * 100);
+ } catch (InterruptedException e) {
+ // the harness interrupts the worker when it gives up
+ Thread.currentThread().interrupt();
+ }
+ }
+}
diff --git a/ui/src/test/java/org/apache/hop/ui/testing/SwtBotTestBase.java
b/ui/src/test/java/org/apache/hop/ui/testing/SwtBotTestBase.java
index a4c6f108fe..16c3fc11b1 100644
--- a/ui/src/test/java/org/apache/hop/ui/testing/SwtBotTestBase.java
+++ b/ui/src/test/java/org/apache/hop/ui/testing/SwtBotTestBase.java
@@ -24,6 +24,7 @@ import java.nio.file.Path;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.BooleanSupplier;
import java.util.function.Consumer;
import org.apache.hop.core.HopEnvironment;
import org.apache.hop.history.AuditManager;
@@ -33,6 +34,7 @@ import org.apache.hop.pipeline.transform.ITransform;
import org.apache.hop.ui.core.PropsUi;
import org.apache.hop.ui.core.gui.GuiResource;
import org.eclipse.swt.SWT;
+import org.eclipse.swt.SWTException;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swtbot.swt.finder.SWTBot;
@@ -58,6 +60,19 @@ public abstract class SwtBotTestBase {
*/
private static final String HOLD_MILLIS_PROPERTY = "swtbot.test.holdMillis";
+ /**
+ * Hard ceiling (milliseconds) on a single scene/dialog. A UI test that
never finishes - a modal
+ * box nobody dismissed, a worker that walked away from the event loop -
would otherwise hold the
+ * build until CI kills the job hours later. When it expires the harness
tears the windows down
+ * and fails the test with the thread stacks, e.g. {@code
-Dswtbot.test.timeoutMillis=300000}.
+ */
+ private static final String TIMEOUT_MILLIS_PROPERTY =
"swtbot.test.timeoutMillis";
+
+ private static final long DEFAULT_TIMEOUT_MILLIS = 120_000L;
+
+ /** How often the {@link Pump} wakes the display on its own. */
+ private static final int PUMP_INTERVAL_MILLIS = 50;
+
protected static Display display;
@BeforeAll
@@ -162,11 +177,20 @@ public abstract class SwtBotTestBase {
}
},
"swtbot-worker");
+ // Daemon: a worker wedged in a syncExec must never keep the surefire
JVM alive.
+ worker.setDaemon(true);
worker.start();
- pumpUntil(done);
- join(worker);
- rethrow(error.get());
+ Pump pump = new Pump(worker);
+ try {
+ pump.until(done::get);
+ // The worker sets `done` from its finally, so it is a hair away from
exiting. Keep pumping
+ // rather than joining outright: its last act may still need the UI
thread.
+ pump.until(() -> !worker.isAlive());
+ } finally {
+ pump.close();
+ }
+ rethrow(pump.timedOut() ? pump.timeoutFailure() : error.get());
} finally {
if (!shell.isDisposed()) {
shell.dispose();
@@ -219,18 +243,31 @@ public abstract class SwtBotTestBase {
}
},
"swtbot-worker");
+ worker.setDaemon(true);
worker.start();
+ // The pump's deadline fires from whichever event loop is dispatching at
the time - the
+ // dialog's own blocking loop below, or a modal box nested inside it -
so a window nobody
+ // closed ends the test instead of parking the build.
+ Pump pump = new Pump(worker);
Throwable openError = null;
try {
- // Runs the dialog's own event loop on the UI thread until the dialog
closes.
- blockingOpener.accept(parent);
- } catch (Throwable t) {
- openError = t;
+ try {
+ // Runs the dialog's own event loop on the UI thread until the
dialog closes.
+ blockingOpener.accept(parent);
+ } catch (Throwable t) {
+ openError = t;
+ }
+ // Keep pumping so the worker's SWTBot calls resolve (or time out) and
its cleanup runs.
+ pump.until(() -> !worker.isAlive());
+ drain();
+ } finally {
+ pump.close();
}
- // Keep pumping so the worker's SWTBot calls resolve (or time out) and
its cleanup runs.
- pumpUntilThreadDone(worker);
+ if (pump.timedOut()) {
+ rethrow(pump.timeoutFailure());
+ }
rethrow(error.get() != null ? error.get() : openError);
} finally {
if (!parent.isDisposed()) {
@@ -254,21 +291,158 @@ public abstract class SwtBotTestBase {
}
}
- private void pumpUntil(AtomicBoolean done) {
- while (!done.get()) {
- if (!display.readAndDispatch()) {
- display.sleep();
+ /**
+ * Keeps the event loop honest for the length of one scene or dialog, and
ends the test if that
+ * takes longer than {@link #TIMEOUT_MILLIS_PROPERTY}.
+ *
+ * <p>Two things run for as long as a pump is open. A heartbeat thread wakes
the display every
+ * {@value #PUMP_INTERVAL_MILLIS} ms, and a timer on the UI thread carries
the deadline.
+ *
+ * <p>The heartbeat is what makes the rest of this reliable. Waking the loop
from the worker's
+ * {@link Display#wake()} alone is a lost-wakeup race: the wake can be
consumed by a
+ * readAndDispatch that runs between the loop's condition check and its
{@link Display#sleep()},
+ * and that sleep then parks with nobody left to wake it - the worker has
already finished. It is
+ * a microsecond-wide window locally and a real hang on a loaded CI runner,
where the worker can
+ * be descheduled between its last statement and the thread actually dying.
The heartbeat also
+ * gets the deadline timer dispatched on macOS, where a Cocoa run loop does
not return for timers
+ * (they are not input sources) and a sleeping display would otherwise never
run it.
+ */
+ private final class Pump implements AutoCloseable {
+
+ private final AtomicReference<String> stuckReport = new
AtomicReference<>();
+ private final Thread worker;
+ private final Thread heartbeat;
+ private final Runnable deadline;
+
+ private Pump(Thread worker) {
+ this.worker = worker;
+ this.deadline = this::giveUp;
+ this.heartbeat = new Thread(this::beat, "swtbot-heartbeat");
+ heartbeat.setDaemon(true);
+ display.timerExec((int) timeoutMillis(), deadline);
+ heartbeat.start();
+ }
+
+ /**
+ * Pumps the event loop on this (the UI) thread until {@code done} turns
true, or we give up.
+ */
+ private void until(BooleanSupplier done) {
+ while (!done.getAsBoolean() && stuckReport.get() == null) {
+ if (!display.readAndDispatch()) {
+ display.sleep();
+ }
+ }
+ }
+
+ private boolean timedOut() {
+ return stuckReport.get() != null;
+ }
+
+ private AssertionError timeoutFailure() {
+ return new AssertionError(
+ "SWTBot UI test did not finish within "
+ + timeoutMillis()
+ + " ms; the harness closed the windows so the build could carry
on. Where it was "
+ + "stuck:"
+ + System.lineSeparator()
+ + stuckReport.get());
+ }
+
+ private void beat() {
+ while (!Thread.currentThread().isInterrupted()) {
+ try {
+ Thread.sleep(PUMP_INTERVAL_MILLIS);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ return;
+ }
+ try {
+ if (display.isDisposed()) {
+ return;
+ }
+ display.wake();
+ } catch (SWTException disposedMeanwhile) {
+ return;
+ }
+ }
+ }
+
+ /**
+ * Runs on the UI thread once the deadline passes: records what the
threads were doing,
+ * screenshots the display, then closes - and, where a shell ignores that,
disposes - every
+ * window so any event loop parked in one returns.
+ */
+ private void giveUp() {
+ if (!stuckReport.compareAndSet(null, describeStuckThreads(worker))) {
+ return;
+ }
+ captureScreenshot("timeout");
+ for (Shell openShell : display.getShells()) {
+ if (!openShell.isDisposed()) {
+ openShell.close();
+ }
+ if (!openShell.isDisposed()) {
+ // A close() the shell vetoed - or never saw, because its own loop
is wedged - still has
+ // to go, or the loop we are trying to unblock keeps running.
+ openShell.dispose();
+ }
+ }
+ if (worker.isAlive()) {
+ worker.interrupt();
+ }
+ }
+
+ @Override
+ public void close() {
+ heartbeat.interrupt();
+ display.timerExec(-1, deadline);
+ }
+ }
+
+ private static long timeoutMillis() {
+ return Long.getLong(TIMEOUT_MILLIS_PROPERTY, DEFAULT_TIMEOUT_MILLIS);
+ }
+
+ /**
+ * What the UI thread and the worker were doing when the deadline passed,
plus the names of the
+ * other live threads. A hung job leaves nothing else behind, so this
travels in the failure
+ * message rather than in output nobody keeps.
+ */
+ private static String describeStuckThreads(Thread worker) {
+ StringBuilder report = new StringBuilder();
+ appendStack(report, "UI thread", Thread.currentThread());
+ appendStack(report, "worker", worker);
+ report.append("other live threads:");
+ for (Thread other : Thread.getAllStackTraces().keySet()) {
+ if (other != Thread.currentThread() && other != worker) {
+ report.append(' ').append(other.getName());
}
}
+ return report.toString();
}
- private void pumpUntilThreadDone(Thread worker) {
- while (worker.isAlive()) {
- if (!display.readAndDispatch()) {
- display.sleep();
+ private static void appendStack(StringBuilder report, String label, Thread
thread) {
+ report
+ .append(label)
+ .append(" [")
+ .append(thread.getName())
+ .append("] ")
+ .append(thread.getState())
+ .append(':')
+ .append(System.lineSeparator());
+ for (StackTraceElement frame : thread.getStackTrace()) {
+ // Drop the frames of the dump itself; the caller wants the frames
underneath it.
+ if (frame.getClassName().equals(SwtBotTestBase.class.getName())
+ && (frame.getMethodName().equals("appendStack")
+ || frame.getMethodName().equals("describeStuckThreads"))) {
+ continue;
+ }
+ if (frame.getClassName().equals("java.lang.Thread")
+ && frame.getMethodName().equals("getStackTrace")) {
+ continue;
}
+ report.append("\tat ").append(frame).append(System.lineSeparator());
}
- drain();
}
private void drain() {
@@ -277,18 +451,10 @@ public abstract class SwtBotTestBase {
}
}
- private static void join(Thread worker) {
- try {
- worker.join();
- } catch (InterruptedException e) {
- Thread.currentThread().interrupt();
- }
- }
-
private static final AtomicInteger SCREENSHOT_COUNTER = new AtomicInteger();
/**
- * Captures the SWT display to {@code
screenshots/<TestClass>.<method>-N.png} the moment a UI
+ * Captures the SWT display to {@code
target/screenshots/<TestClass>.<method>-N.png} the moment a
* test's worker thread sees an assertion failure or unexpected exception.
We do this here, before
* the harness's finally tears the dialog/shell down - by the time the
SWTBot extension's
* testFailed runs the UI is gone and its auto-screenshot would just be the
empty Xvfb desktop.
@@ -296,6 +462,11 @@ public abstract class SwtBotTestBase {
* propagates with its full stack trace.
*/
private static void captureLiveScreenshot(Throwable failure) {
+ captureScreenshot(screenshotName(failure));
+ }
+
+ /** The test frame a failure came from, for use as the screenshot's file
name. */
+ private static String screenshotName(Throwable failure) {
String name = "harness-failure";
for (StackTraceElement frame : failure.getStackTrace()) {
String cn = frame.getClassName();
@@ -310,8 +481,14 @@ public abstract class SwtBotTestBase {
break;
}
}
+ return name;
+ }
+
+ private static void captureScreenshot(String name) {
+ // Under target/ so a screenshot never lands in the checked-out tree; CI
collects the folder
+ // from wherever it is (the workflow globs **/screenshots/**).
String path =
- String.format("screenshots/%s-%d.png", name,
SCREENSHOT_COUNTER.incrementAndGet());
+ String.format("target/screenshots/%s-%d.png", name,
SCREENSHOT_COUNTER.incrementAndGet());
try {
SWTUtils.captureScreenshot(path);
} catch (Throwable ignored) {
diff --git a/web-tests/pom.xml b/web-tests/pom.xml
index 6311210a9d..a5ae28c9dc 100644
--- a/web-tests/pom.xml
+++ b/web-tests/pom.xml
@@ -31,6 +31,7 @@
<properties>
<hopweb.browser>auto</hopweb.browser>
+
<hopweb.browserImage>selenium/standalone-chrome:4.47.0</hopweb.browserImage>
<hopweb.build>false</hopweb.build>
<hopweb.excludedGroups>full</hopweb.excludedGroups>
<hopweb.groups></hopweb.groups>
@@ -93,6 +94,7 @@
<hopweb.repository>${project.basedir}/..</hopweb.repository>
<hopweb.url>${hopweb.url}</hopweb.url>
<hopweb.browser>${hopweb.browser}</hopweb.browser>
+
<hopweb.browserImage>${hopweb.browserImage}</hopweb.browserImage>
<hopweb.headless>${hopweb.headless}</hopweb.headless>
<hopweb.startupTimeout>${hopweb.startupTimeout}</hopweb.startupTimeout>
<hopweb.artifacts>${project.build.directory}/hopweb-artifacts</hopweb.artifacts>
diff --git
a/web-tests/src/test/java/org/apache/hop/web/it/HopWebEnvironment.java
b/web-tests/src/test/java/org/apache/hop/web/it/HopWebEnvironment.java
index 7473d0abbb..edb22a4320 100644
--- a/web-tests/src/test/java/org/apache/hop/web/it/HopWebEnvironment.java
+++ b/web-tests/src/test/java/org/apache/hop/web/it/HopWebEnvironment.java
@@ -60,6 +60,7 @@ import org.testcontainers.utility.MountableFile;
* <li>{@code hopweb.url} - URL of an already running Hop Web. Set it while
writing tests to skip
* container startup entirely, e.g. {@code
-Dhopweb.url=http://localhost:8080/ui}
* <li>{@code hopweb.browser} - {@code auto} (default), {@code container} or
{@code local}
+ * <li>{@code hopweb.browserImage} - the browser image, for a containerised
browser
* <li>{@code hopweb.headless} - only meaningful for a local browser
* </ul>
*/
@@ -68,6 +69,16 @@ public final class HopWebEnvironment {
private static final String LOCAL_IMAGE_VERSION = "local";
private static final String LOCAL_IMAGE = "hop-web:" + LOCAL_IMAGE_VERSION;
+ /**
+ * The browser image. Pinned here rather than left to Testcontainers, which
derives the tag from
+ * the Selenium client on the classpath: the selenium/standalone-* images
are published a release
+ * behind that client, so every client bump would fail the build on a
manifest that does not exist
+ * yet ({@code 404 manifest for selenium/standalone-chrome:4.48.0 not
found}). Bump this when the
+ * matching image is out; the client and the browser only have to speak the
same WebDriver
+ * protocol, not carry the same version.
+ */
+ private static final String DEFAULT_BROWSER_IMAGE =
"selenium/standalone-chrome:4.47.0";
+
private static final String HOP_WEB_ALIAS = "hop-web";
private static final int HOP_WEB_PORT = 8080;
@@ -276,7 +287,8 @@ public final class HopWebEnvironment {
private WebDriver containerBrowser(Network network) {
BrowserWebDriverContainer<?> browser =
- new BrowserWebDriverContainer<>()
+ new BrowserWebDriverContainer<>(
+ DockerImageName.parse(property("hopweb.browserImage",
DEFAULT_BROWSER_IMAGE)))
.withNetwork(network)
.withCapabilities(chromeOptions())
.withStartupTimeout(Duration.ofSeconds(startupTimeoutSeconds()));
