Skip to content

Commit

Permalink
Relaxed the multi-thread w/o newRequest test
Browse files Browse the repository at this point in the history
When the CI workers are busy and can't execute 20 requests within 10,000 milliseconds, the test would flake.
  • Loading branch information
jhy committed Jul 9, 2024
1 parent 6c55f01 commit 6423e65
Showing 1 changed file with 14 additions and 2 deletions.
16 changes: 14 additions & 2 deletions src/test/java/org/jsoup/integration/SessionIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.util.concurrent.atomic.AtomicInteger;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;

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

Thread[] threads = new Thread[numThreads];
AtomicInteger successful = new AtomicInteger();
for (int threadNum = 0; threadNum < numThreads; threadNum++) {
Thread thread = new Thread(() -> {
try {
Document doc = session.url(url).get();
assertEquals(title, doc.title());
successful.getAndIncrement();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand All @@ -103,8 +106,17 @@ public void multiThreadWithoutNewRequestBlowsUp() throws InterruptedException {
}

// only one should have passed, rest should have blown up (assuming the started whilst other was running)
assertEquals(numThreads - 1, catcher.multiThreadExceptions.get());
assertEquals(numThreads - 1, catcher.exceptionCount.get());
//assertEquals(numThreads - 1, catcher.multiThreadExceptions.get());
//assertEquals(numThreads - 1, catcher.exceptionCount.get());

/* The checks above work when all 20 threads are executed within 10 seconds. However, depending on how cloudy it
is when the CI jobs are run, they may not all complete in time. As of writing that appears most commonly on the
maxOS runners, which appear overtaxed. That makes this test flaky. So we relax the test conditions, and make
sure at least just one passed and one failed. That's OK in prod as well, because we are only concerned about
concurrent execution, which the impl does detect correctly. */
assertTrue(successful.get() > 0);
assertTrue(catcher.multiThreadExceptions.get() > 0);
assertEquals(catcher.multiThreadExceptions.get(), catcher.exceptionCount.get()); // all exceptions are multi-threaded
}

@Test
Expand Down

0 comments on commit 6423e65

Please sign in to comment.