This is an automated email from the ASF dual-hosted git repository.
snazy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris-tools.git
The following commit(s) were added to refs/heads/main by this push:
new 28f4fad Apprunner: Work around potential flakiness in GH (#52)
28f4fad is described below
commit 28f4fada8fbcf9fbc533bc3cd8a80a6918b62ea0
Author: Robert Stupp <[email protected]>
AuthorDate: Tue Nov 18 17:12:36 2025 +0100
Apprunner: Work around potential flakiness in GH (#52)
There's no real "wait" in the futures, but it's still possible that the
actual futures run late and cause a test failure.
---
apprunner/common/build.gradle.kts | 6 +-
.../apprunner/common/TestListenUrlWaiter.java | 26 ++++----
.../apprunner/common/TestProcessHandler.java | 73 +++++++++++-----------
apprunner/gradle/libs.versions.toml | 1 +
4 files changed, 57 insertions(+), 49 deletions(-)
diff --git a/apprunner/common/build.gradle.kts
b/apprunner/common/build.gradle.kts
index 89ca14e..e4bb722 100644
--- a/apprunner/common/build.gradle.kts
+++ b/apprunner/common/build.gradle.kts
@@ -19,4 +19,8 @@
plugins { id("polaris-apprunner-java") }
-dependencies { compileOnly(libs.jakarta.annotation.api) }
+dependencies {
+ compileOnly(libs.jakarta.annotation.api)
+
+ testImplementation(libs.junit.pioneer)
+}
diff --git
a/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestListenUrlWaiter.java
b/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestListenUrlWaiter.java
index 15ce06f..3db92c7 100644
---
a/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestListenUrlWaiter.java
+++
b/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestListenUrlWaiter.java
@@ -18,6 +18,8 @@
*/
package org.apache.polaris.apprunner.common;
+import static org.assertj.core.api.Assertions.assertThat;
+
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
@@ -30,9 +32,9 @@ import
org.assertj.core.api.junit.jupiter.InjectSoftAssertions;
import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junitpioneer.jupiter.RetryingTest;
@ExtendWith(SoftAssertionsExtension.class)
class TestListenUrlWaiter {
@@ -99,7 +101,8 @@ class TestListenUrlWaiter {
.containsExactly("https://localhost.in.some.space:4242";, null);
}
- @RepeatedTest(20) // repeat, risk of flakiness
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
+ // repeat, risk of flakiness
void timeout() {
var clock = new AtomicLong();
var timeout = 10_000L; // long timeout, for slow CI
@@ -107,20 +110,21 @@ class TestListenUrlWaiter {
var line = new AtomicReference<>();
var waiter = new ListenUrlWaiter(clock::get, timeout, line::set);
- soft.assertThat(waiter.isTimeout()).isFalse();
+ assertThat(waiter.isTimeout()).isFalse();
clock.set(TimeUnit.MILLISECONDS.toNanos(timeout + 1));
- soft.assertThat(waiter.isTimeout()).isTrue();
+ assertThat(waiter.isTimeout()).isTrue();
- soft.assertThat(executor.submit(waiter::getListenUrls))
+ assertThat(executor.submit(waiter::getListenUrls))
.failsWithin(5, TimeUnit.SECONDS)
.withThrowableOfType(ExecutionException.class)
.withRootCauseExactlyInstanceOf(TimeoutException.class)
.withMessageEndingWith(ListenUrlWaiter.TIMEOUT_MESSAGE +
ListenUrlWaiter.NOTHING_RECEIVED);
}
- @RepeatedTest(20) // repeat, risk of flakiness
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
+ // repeat, risk of flakiness
void noTimeout() throws Exception {
var clock = new AtomicLong();
var timeout = 10_000L; // long timeout, for slow CI
@@ -137,15 +141,15 @@ class TestListenUrlWaiter {
var listenLine =
"2021-05-28 12:12:25,753 INFO [io.quarkus] (main) nessie-quarkus
0.6.2-SNAPSHOT on JVM (powered by Quarkus 1.13.4.Final) started in 1.444s.
Listening on: http://4.2.4.2:4242. Management interface listening on
http://4.2.4.2:2424.";;
waiter.accept(listenLine);
- soft.assertThat(line.getAndSet(null)).isEqualTo(listenLine);
- soft.assertThat(waiter.getListenUrls())
+ assertThat(line.getAndSet(null)).isEqualTo(listenLine);
+ assertThat(waiter.getListenUrls())
.containsExactly("http://4.2.4.2:4242";, "http://4.2.4.2:2424";);
- soft.assertThat(waiter.isTimeout()).isFalse();
+ assertThat(waiter.isTimeout()).isFalse();
// Clock post the timeout-boundary (so a timeout-check would trigger)
clock.set(TimeUnit.MILLISECONDS.toNanos(timeout + 1));
- soft.assertThat(waiter.getListenUrls())
+ assertThat(waiter.getListenUrls())
.containsExactly("http://4.2.4.2:4242";, "http://4.2.4.2:2424";);
- soft.assertThat(waiter.isTimeout()).isFalse();
+ assertThat(waiter.isTimeout()).isFalse();
}
}
diff --git
a/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestProcessHandler.java
b/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestProcessHandler.java
index 79fa875..14a9716 100644
---
a/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestProcessHandler.java
+++
b/apprunner/common/src/test/java/org/apache/polaris/apprunner/common/TestProcessHandler.java
@@ -18,7 +18,10 @@
*/
package org.apache.polaris.apprunner.common;
+import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import java.io.InputStream;
import java.io.OutputStream;
@@ -39,9 +42,9 @@ import
org.assertj.core.api.junit.jupiter.InjectSoftAssertions;
import org.assertj.core.api.junit.jupiter.SoftAssertionsExtension;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.RepeatedTest;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
+import org.junitpioneer.jupiter.RetryingTest;
@ExtendWith(SoftAssertionsExtension.class)
class TestProcessHandler {
@@ -58,7 +61,7 @@ class TestProcessHandler {
@AfterAll
static void stopExecutor() throws Exception {
executor.shutdown();
- executor.awaitTermination(10, SECONDS);
+ executor.awaitTermination(5, MINUTES);
}
@Test
@@ -81,7 +84,7 @@ class TestProcessHandler {
.hasMessage("Process already started");
}
- @RepeatedTest(20)
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
// repeat, risk of flakiness
void processWithNoOutput() {
var phMock = new ProcessHandlerMock();
@@ -91,8 +94,7 @@ class TestProcessHandler {
var futureListenUrl = executor.submit(phMock.ph::getListenUrls);
while (phMock.clock.get() <
TimeUnit.MILLISECONDS.toNanos(phMock.timeToUrl)) {
- soft.assertThat(futureListenUrl).isNotDone();
- soft.assertAll();
+ assertThat(futureListenUrl).isNotDone();
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
}
// should be exactly at (but not "past") the time to wait for the
listen-url now
@@ -100,8 +102,8 @@ class TestProcessHandler {
// bump the clock "past" the listen-url-timeout
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
- soft.assertThat(futureListenUrl)
- .failsWithin(5, SECONDS)
+ assertThat(futureListenUrl)
+ .failsWithin(5, MINUTES)
.withThrowableOfType(ExecutionException.class) // EE from
ForkJoinPool/executor (test code)
.withRootCauseInstanceOf(
TimeoutException.class) // TE from
ProcessHandler/ListenUrlWaiter.getListenUrl
@@ -110,10 +112,10 @@ class TestProcessHandler {
// Need to wait for the watchdog to finish, before we can do any further
assertion
phMock.ph.watchdogExitGrace();
- soft.assertThat(phMock.ph.isAlive()).isFalse();
+ assertThat(phMock.ph.isAlive()).isFalse();
}
- @RepeatedTest(20)
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
// repeat, risk of flakiness
void processExitsEarly() {
var phMock = new ProcessHandlerMock();
@@ -122,14 +124,14 @@ class TestProcessHandler {
var futureListenUrl = executor.submit(phMock.ph::getListenUrls);
- soft.assertThat(phMock.ph.isAlive()).isTrue();
- soft.assertThatThrownBy(() -> phMock.ph.getExitCode())
+ assertThat(phMock.ph.isAlive()).isTrue();
+ assertThatThrownBy(() -> phMock.ph.getExitCode())
.isInstanceOf(IllegalThreadStateException.class);
phMock.exitCode.set(88);
- soft.assertThat(futureListenUrl)
- .failsWithin(5, SECONDS)
+ assertThat(futureListenUrl)
+ .failsWithin(5, MINUTES)
.withThrowableOfType(ExecutionException.class) // EE from
ForkJoinPool/executor (test code)
.withMessageEndingWith(
ListenUrlWaiter.TIMEOUT_MESSAGE
@@ -139,11 +141,11 @@ class TestProcessHandler {
// Need to wait for the watchdog to finish, before we can do any further
assertion
phMock.ph.watchdogExitGrace();
- soft.assertThat(phMock.ph.isAlive()).isFalse();
- soft.assertThat(phMock.ph.getExitCode()).isEqualTo(88);
+ assertThat(phMock.ph.isAlive()).isFalse();
+ assertThat(phMock.ph.getExitCode()).isEqualTo(88);
}
- @RepeatedTest(20)
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
// repeat, risk of flakiness
void processLotsOfIoNoListen() throws Exception {
var phMock = new ProcessHandlerMock();
@@ -158,27 +160,25 @@ class TestProcessHandler {
for (var c : message) {
phMock.stdout.add((byte) c);
}
- soft.assertThat(futureListenUrl).isNotDone();
- soft.assertAll();
+ assertThat(futureListenUrl).isNotDone();
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
}
// should be exactly at (but not "past") the time to wait for the
listen-url now
- soft.assertThat(phMock.ph.remainingWaitTimeNanos()).isEqualTo(0);
- soft.assertThat(phMock.ph.isAlive()).isTrue();
+ assertThat(phMock.ph.remainingWaitTimeNanos()).isEqualTo(0);
+ assertThat(phMock.ph.isAlive()).isTrue();
var timeoutFail = System.currentTimeMillis() + SECONDS.toMillis(10);
while (!phMock.stdout.isEmpty()) {
- soft.assertThat(System.currentTimeMillis() < timeoutFail).isTrue();
- soft.assertAll();
+ assertThat(System.currentTimeMillis() < timeoutFail).isTrue();
Thread.sleep(1L);
}
// bump the clock "past" the listen-url-timeout
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
- soft.assertThat(futureListenUrl)
- .failsWithin(5, SECONDS)
+ assertThat(futureListenUrl)
+ .failsWithin(5, MINUTES)
.withThrowableOfType(ExecutionException.class) // EE from
ForkJoinPool/executor (test code)
.withRootCauseInstanceOf(
TimeoutException.class) // TE from
ProcessHandler/ListenUrlWaiter.getListenUrl
@@ -189,13 +189,13 @@ class TestProcessHandler {
// Need to wait for the watchdog to finish, before we can do any further
assertion
phMock.ph.watchdogExitGrace();
- soft.assertThat(phMock.ph.isAlive()).isFalse();
- soft.assertThat(phMock.ph.getExitCode()).isGreaterThanOrEqualTo(0);
+ assertThat(phMock.ph.isAlive()).isFalse();
+ assertThat(phMock.ph.getExitCode()).isGreaterThanOrEqualTo(0);
- soft.assertThat(phMock.stdoutLines).hasSize((int) (phMock.timeToUrl / 10));
+ assertThat(phMock.stdoutLines).hasSize((int) (phMock.timeToUrl / 10));
}
- @RepeatedTest(20)
+ @RetryingTest(maxAttempts = 50, minSuccess = 20)
// repeat, risk of flakiness
void processLotsOfIoProperListenUrl() {
var phMock = new ProcessHandlerMock();
@@ -208,8 +208,7 @@ class TestProcessHandler {
for (var c : "Hello world\n".toCharArray()) {
phMock.stdout.add((byte) c);
}
- soft.assertThat(futureListenUrl).isNotDone();
- soft.assertAll();
+ assertThat(futureListenUrl).isNotDone();
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
}
// should be exactly at (but not "past") the time to wait for the
listen-url now
@@ -221,21 +220,21 @@ class TestProcessHandler {
// bump the clock "past" the listen-url-timeout
phMock.clock.addAndGet(TimeUnit.MILLISECONDS.toNanos(10));
- soft.assertThat(futureListenUrl)
- .succeedsWithin(5, SECONDS)
+ assertThat(futureListenUrl)
+ .succeedsWithin(5, MINUTES)
.isEqualTo(Arrays.asList("http://0.0.0.0:4242";, null));
- soft.assertThat(phMock.ph.isAlive()).isTrue();
- soft.assertThatThrownBy(() -> phMock.ph.getExitCode())
+ assertThat(phMock.ph.isAlive()).isTrue();
+ assertThatThrownBy(() -> phMock.ph.getExitCode())
.isInstanceOf(IllegalThreadStateException.class);
// The .stop() waits until the watchdog has finished its work
phMock.ph.stop();
- soft.assertThat(phMock.ph.isAlive()).isFalse();
- soft.assertThat(phMock.ph.getExitCode()).isGreaterThanOrEqualTo(0);
+ assertThat(phMock.ph.isAlive()).isFalse();
+ assertThat(phMock.ph.getExitCode()).isGreaterThanOrEqualTo(0);
- soft.assertThat(phMock.stdoutLines).hasSize((int) (phMock.timeToUrl / 10 /
2) + 1);
+ assertThat(phMock.stdoutLines).hasSize((int) (phMock.timeToUrl / 10 / 2) +
1);
}
static final class ProcessHandlerMock {
diff --git a/apprunner/gradle/libs.versions.toml
b/apprunner/gradle/libs.versions.toml
index 1c185b1..fc0fe7d 100644
--- a/apprunner/gradle/libs.versions.toml
+++ b/apprunner/gradle/libs.versions.toml
@@ -27,6 +27,7 @@ maven-core = { module = "org.apache.maven:maven-core",
version = "3.9.9" }
maven-plugin-annotations = { module =
"org.apache.maven.plugin-tools:maven-plugin-annotations", version = "3.15.1" }
jakarta-annotation-api = { module =
"jakarta.annotation:jakarta.annotation-api", version = "3.0.0" }
junit-bom = { module = "org.junit:junit-bom", version = "5.14.1" }
+junit-pioneer = { module = "org.junit-pioneer:junit-pioneer", version =
"2.3.0" }
soebes-itf-assertj = { module =
"com.soebes.itf.jupiter.extension:itf-assertj", version.ref = "soebes-itf" }
soebes-itf-jupiter-extension = { module =
"com.soebes.itf.jupiter.extension:itf-jupiter-extension", version.ref =
"soebes-itf" }
