Skip to content

Commit 6423e65

Browse files
committed
Relaxed the multi-thread w/o newRequest test
When the CI workers are busy and can't execute 20 requests within 10,000 milliseconds, the test would flake.
1 parent 6c55f01 commit 6423e65

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

src/test/java/org/jsoup/integration/SessionIT.java

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import java.util.concurrent.atomic.AtomicInteger;
1616

1717
import static org.junit.jupiter.api.Assertions.assertEquals;
18+
import static org.junit.jupiter.api.Assertions.assertTrue;
1819

1920
/** Integration tests to test longer running Connection */
2021
public class SessionIT {
@@ -82,11 +83,13 @@ public void multiThreadWithoutNewRequestBlowsUp() throws InterruptedException {
8283
Connection session = Jsoup.newSession();
8384

8485
Thread[] threads = new Thread[numThreads];
86+
AtomicInteger successful = new AtomicInteger();
8587
for (int threadNum = 0; threadNum < numThreads; threadNum++) {
8688
Thread thread = new Thread(() -> {
8789
try {
8890
Document doc = session.url(url).get();
8991
assertEquals(title, doc.title());
92+
successful.getAndIncrement();
9093
} catch (IOException e) {
9194
throw new UncheckedIOException(e);
9295
}
@@ -103,8 +106,17 @@ public void multiThreadWithoutNewRequestBlowsUp() throws InterruptedException {
103106
}
104107

105108
// only one should have passed, rest should have blown up (assuming the started whilst other was running)
106-
assertEquals(numThreads - 1, catcher.multiThreadExceptions.get());
107-
assertEquals(numThreads - 1, catcher.exceptionCount.get());
109+
//assertEquals(numThreads - 1, catcher.multiThreadExceptions.get());
110+
//assertEquals(numThreads - 1, catcher.exceptionCount.get());
111+
112+
/* The checks above work when all 20 threads are executed within 10 seconds. However, depending on how cloudy it
113+
is when the CI jobs are run, they may not all complete in time. As of writing that appears most commonly on the
114+
maxOS runners, which appear overtaxed. That makes this test flaky. So we relax the test conditions, and make
115+
sure at least just one passed and one failed. That's OK in prod as well, because we are only concerned about
116+
concurrent execution, which the impl does detect correctly. */
117+
assertTrue(successful.get() > 0);
118+
assertTrue(catcher.multiThreadExceptions.get() > 0);
119+
assertEquals(catcher.multiThreadExceptions.get(), catcher.exceptionCount.get()); // all exceptions are multi-threaded
108120
}
109121

110122
@Test

0 commit comments

Comments
 (0)