use the right AssertJ method assertThatThrownBy instead of error-prone construct "try/fail/catch(expected)".

Author: asolntsev

java/src/org/openqa/selenium/grid/sessionmap/jdbc/JdbcBackedSessionMap.java

@@ -221,7 +221,7 @@ public Session get(SessionId id) throws NoSuchSessionException {
         try (ResultSet sessions = statement.executeQuery()) {
           if (!sessions.next()) {
             NoSuchSessionException exception =
-                new NoSuchSessionException("Unable to find session.");
+                new NoSuchSessionException("Unable to find session with id: " + id);
             span.setAttribute("error", true);
             span.setStatus(Status.NOT_FOUND);
             EXCEPTION.accept(attributeMap, exception);

java/src/org/openqa/selenium/grid/sessionmap/redis/RedisBackedSessionMap.java

@@ -223,7 +223,8 @@ public URI getUri(SessionId id) throws NoSuchSessionException {
       attributeMap.put(REDIS_URI_KEY, uriKey);
 
       if (rawUri == null) {
-        NoSuchSessionException exception = new NoSuchSessionException("Unable to find session.");
+        NoSuchSessionException exception =
+            new NoSuchSessionException("Unable to find session with id: " + id);
         span.setAttribute("error", true);
         span.setStatus(Status.NOT_FOUND);
         EXCEPTION.accept(attributeMap, exception);

java/test/org/openqa/selenium/CorrectEventFiringTest.java

@@ -17,9 +17,7 @@
 
 package org.openqa.selenium;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.assertj.core.api.Assertions.*;
 import static org.junit.jupiter.api.Assumptions.assumeFalse;
 import static org.openqa.selenium.WaitingConditions.elementTextToContain;
 import static org.openqa.selenium.WaitingConditions.elementTextToEqual;
@@ -180,11 +178,9 @@ public void testShouldFireMouseMoveEventWhenClicking() {
   void testShouldNotThrowIfEventHandlerThrows() {
     driver.get(pages.javascriptPage);
 
-    try {
-      driver.findElement(By.id("throwing-mouseover")).click();
-    } catch (WebDriverException e) {
-      fail("Error in event handler should not have propagated: " + e);
-    }
+    assertThatCode(() -> driver.findElement(By.id("throwing-mouseover")).click())
+        .as("Error in event handler should not have propagated")
+        .doesNotThrowAnyException();
   }
 
   @Test
@@ -410,9 +406,9 @@ public void testSendingKeysToAFocusedElementShouldNotBlurThatElement() {
         throw new RuntimeException(e);
       }
     }
-    if (!focused) {
-      fail("Clicking on element didn't focus it in time - can't proceed so failing");
-    }
+    assertThat(focused)
+        .as("If clicking on element didn't focus it in time, we can't proceed with the test")
+        .isTrue();
 
     element.sendKeys("a");
     assertEventNotFired("blur", driver);

java/test/org/openqa/selenium/PageLoadTimeOutTest.java

@@ -17,14 +17,13 @@
 
 package org.openqa.selenium;
 
+import static java.lang.System.currentTimeMillis;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.fail;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.openqa.selenium.WaitingConditions.elementTextToEqual;
 import static org.openqa.selenium.support.ui.ExpectedConditions.titleIs;
 import static org.openqa.selenium.support.ui.ExpectedConditions.visibilityOfElementLocated;
-import static org.openqa.selenium.testing.drivers.Browser.CHROME;
-import static org.openqa.selenium.testing.drivers.Browser.EDGE;
-import static org.openqa.selenium.testing.drivers.Browser.SAFARI;
+import static org.openqa.selenium.testing.drivers.Browser.*;
 
 import java.time.Duration;
 import org.junit.jupiter.api.Test;
@@ -72,22 +71,16 @@ void testShouldTimeoutIfAPageTakesTooLongToLoad() {
   @Ignore(value = SAFARI, reason = "Flaky")
   public void testShouldTimeoutIfAPageTakesTooLongToLoadAfterClick() {
     driver.manage().timeouts().pageLoadTimeout(Duration.ofSeconds(2));
-
-    driver.get(appServer.whereIs("page_with_link_to_slow_loading_page.html"));
-    WebElement link = wait.until(visibilityOfElementLocated(By.id("link-to-slow-loading-page")));
-
-    long start = System.currentTimeMillis();
     try {
-      link.click();
-      fail("I should have timed out");
-    } catch (RuntimeException e) {
-      long end = System.currentTimeMillis();
+      driver.get(appServer.whereIs("page_with_link_to_slow_loading_page.html"));
+      WebElement link = wait.until(visibilityOfElementLocated(By.id("link-to-slow-loading-page")));
 
-      assertThat(e).isInstanceOf(TimeoutException.class);
+      long start = currentTimeMillis();
 
-      int duration = (int) (end - start);
-      assertThat(duration).isGreaterThan(2000);
-      assertThat(duration).isLessThan(5000);
+      assertThatThrownBy(() -> link.click()).isInstanceOf(TimeoutException.class);
+
+      long duration = currentTimeMillis() - start;
+      assertThat(duration).isBetween(2000L, 5000L);
     } finally {
       driver.manage().timeouts().pageLoadTimeout(Duration.ofSeconds(300));
     }
@@ -109,18 +102,13 @@ public void testShouldTimeoutIfAPageTakesTooLongToRefresh() {
 
     driver.manage().timeouts().pageLoadTimeout(Duration.ofSeconds(2));
 
-    long start = System.currentTimeMillis();
     try {
-      driver.navigate().refresh();
-      fail("I should have timed out");
-    } catch (RuntimeException e) {
-      long end = System.currentTimeMillis();
+      long start = currentTimeMillis();
 
-      assertThat(e).isInstanceOf(TimeoutException.class);
+      assertThatThrownBy(() -> driver.navigate().refresh()).isInstanceOf(TimeoutException.class);
 
-      int duration = (int) (end - start);
-      assertThat(duration).isGreaterThanOrEqualTo(2000);
-      assertThat(duration).isLessThan(4000);
+      long duration = currentTimeMillis() - start;
+      assertThat(duration).isBetween(2000L, 4000L);
     } finally {
       driver.manage().timeouts().pageLoadTimeout(Duration.ofSeconds(300));
     }
@@ -163,19 +151,18 @@ private void testPageLoadTimeoutIsEnforced(long webDriverPageLoadTimeout) {
 
   private void assertPageLoadTimeoutIsEnforced(
       long webDriverPageLoadTimeout, long pageLoadTimeBuffer) {
-    long start = System.currentTimeMillis();
-    try {
-      driver.get(
-          appServer.whereIs("sleep?time=" + (webDriverPageLoadTimeout + pageLoadTimeBuffer)));
-      fail("I should have timed out after " + webDriverPageLoadTimeout + " seconds");
-    } catch (RuntimeException e) {
-      long end = System.currentTimeMillis();
-
-      assertThat(e).isInstanceOf(TimeoutException.class);
-
-      long duration = end - start;
-      assertThat(duration).isGreaterThanOrEqualTo(webDriverPageLoadTimeout * 1000);
-      assertThat(duration).isLessThan((webDriverPageLoadTimeout + pageLoadTimeBuffer) * 1000);
-    }
+    long start = currentTimeMillis();
+    assertThatThrownBy(
+            () -> {
+              driver.get(
+                  appServer.whereIs(
+                      "sleep?time=" + (webDriverPageLoadTimeout + pageLoadTimeBuffer)));
+            })
+        .as("Should have timed out after " + webDriverPageLoadTimeout + " seconds")
+        .isInstanceOf(TimeoutException.class);
+
+    long duration = currentTimeMillis() - start;
+    assertThat(duration).isGreaterThanOrEqualTo(webDriverPageLoadTimeout * 1000);
+    assertThat(duration).isLessThan((webDriverPageLoadTimeout + pageLoadTimeBuffer) * 1000);
   }
 }

java/test/org/openqa/selenium/TakesScreenshotTest.java

@@ -18,7 +18,6 @@
 package org.openqa.selenium;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.fail;
 import static org.junit.jupiter.api.Assumptions.assumeTrue;
 import static org.openqa.selenium.support.ui.ExpectedConditions.frameToBeAvailableAndSwitchToIt;
 import static org.openqa.selenium.support.ui.ExpectedConditions.titleIs;
@@ -108,7 +107,7 @@ void testGetScreenshotAsBinary() {
 
   @Test
   @Ignore(value = CHROME, gitHubActions = true)
-  public void testShouldCaptureScreenshotOfCurrentViewport() {
+  public void testShouldCaptureScreenshotOfCurrentViewport() throws IOException {
     driver.get(appServer.whereIs("screen/screen.html"));
 
     BufferedImage screenshot = getImage();
@@ -147,7 +146,7 @@ void testShouldCaptureScreenshotOfAnElement() throws Exception {
 
   @Test
   @Ignore(value = CHROME, gitHubActions = true)
-  public void testShouldCaptureScreenshotAtFramePage() {
+  public void testShouldCaptureScreenshotAtFramePage() throws IOException {
     driver.get(appServer.whereIs("screen/screen_frames.html"));
     wait.until(frameToBeAvailableAndSwitchToIt(By.id("frame1")));
     wait.until(visibilityOfAllElementsLocatedBy(By.id("content")));
@@ -184,7 +183,7 @@ public void testShouldCaptureScreenshotAtFramePage() {
   @Test
   @Ignore(CHROME)
   @Ignore(EDGE)
-  public void testShouldCaptureScreenshotAtIFramePage() {
+  public void testShouldCaptureScreenshotAtIFramePage() throws IOException {
     driver.get(appServer.whereIs("screen/screen_iframes.html"));
 
     BufferedImage screenshot = getImage();
@@ -214,7 +213,7 @@ public void testShouldCaptureScreenshotAtIFramePage() {
   @Test
   @Ignore(FIREFOX)
   @Ignore(value = CHROME, gitHubActions = true)
-  public void testShouldCaptureScreenshotAtFramePageAfterSwitching() {
+  public void testShouldCaptureScreenshotAtFramePageAfterSwitching() throws IOException {
     driver.get(appServer.whereIs("screen/screen_frames.html"));
 
     driver.switchTo().frame(driver.findElement(By.id("frame2")));
@@ -248,7 +247,7 @@ public void testShouldCaptureScreenshotAtFramePageAfterSwitching() {
   @Ignore(CHROME)
   @Ignore(EDGE)
   @Ignore(FIREFOX)
-  public void testShouldCaptureScreenshotAtIFramePageAfterSwitching() {
+  public void testShouldCaptureScreenshotAtIFramePageAfterSwitching() throws IOException {
     driver.get(appServer.whereIs("screen/screen_iframes.html"));
 
     driver.switchTo().frame(driver.findElement(By.id("iframe2")));
@@ -272,7 +271,7 @@ public void testShouldCaptureScreenshotAtIFramePageAfterSwitching() {
             /* grid X size */ 6,
             /* grid Y size */ 6));
 
-    // expectation is that screenshot at page with Iframes after switching to a Iframe
+    // expectation is that screenshot at page with Iframes after switching to iframe
     // will be taken for full page
     compareColors(expectedColors, actualColors);
   }
@@ -282,17 +281,12 @@ public void testShouldCaptureScreenshotAtIFramePageAfterSwitching() {
    *
    * @return Image object
    */
-  private BufferedImage getImage() {
-    BufferedImage image = null;
-    try {
-      byte[] imageData = screenshooter.getScreenshotAs(OutputType.BYTES);
-      assertThat(imageData).isNotNull();
-      assertThat(imageData.length).isPositive();
-      image = ImageIO.read(new ByteArrayInputStream(imageData));
-      assertThat(image).isNotNull();
-    } catch (IOException e) {
-      fail("Image screenshot file is invalid: " + e.getMessage());
-    }
+  private BufferedImage getImage() throws IOException {
+    byte[] imageData = screenshooter.getScreenshotAs(OutputType.BYTES);
+    assertThat(imageData).isNotNull();
+    assertThat(imageData.length).isPositive();
+    BufferedImage image = ImageIO.read(new ByteArrayInputStream(imageData));
+    assertThat(image).isNotNull();
 
     // saveImageToTmpFile(image);
     return image;
@@ -337,28 +331,23 @@ private Set<String> generateExpectedColors(
   private Set<String> scanActualColors(BufferedImage image, final int stepX, final int stepY) {
     Set<String> colors = new TreeSet<>();
 
-    try {
-      int height = image.getHeight();
-      int width = image.getWidth();
-      assertThat(width > 0).isTrue();
-      assertThat(height > 0).isTrue();
-
-      Raster raster = image.getRaster();
-      for (int i = 0; i < width; i = i + stepX) {
-        for (int j = 0; j < height; j = j + stepY) {
-          String hex =
-              String.format(
-                  "#%02x%02x%02x",
-                  (raster.getSample(i, j, 0)),
-                  (raster.getSample(i, j, 1)),
-                  (raster.getSample(i, j, 2)));
-          colors.add(hex);
-        }
+    int height = image.getHeight();
+    int width = image.getWidth();
+    assertThat(width > 0).isTrue();
+    assertThat(height > 0).isTrue();
+
+    Raster raster = image.getRaster();
+    for (int i = 0; i < width; i = i + stepX) {
+      for (int j = 0; j < height; j = j + stepY) {
+        String hex =
+            String.format(
+                "#%02x%02x%02x",
+                (raster.getSample(i, j, 0)),
+                (raster.getSample(i, j, 1)),
+                (raster.getSample(i, j, 2)));
+        colors.add(hex);
       }
-    } catch (Exception e) {
-      fail("Unable to get actual colors from screenshot: " + e.getMessage());
     }
-
     assertThat(colors).isNotEmpty();
 
     return colors;
@@ -379,17 +368,7 @@ private void compareColors(Set<String> expectedColors, Set<String> actualColors)
     cleanActualColors.remove("#000000");
     cleanActualColors.remove("#ffffff");
 
-    if (!expectedColors.containsAll(cleanActualColors)) {
-      fail(
-          "There are unexpected colors on the screenshot: "
-              + Sets.difference(cleanActualColors, expectedColors));
-    }
-
-    if (!cleanActualColors.containsAll(expectedColors)) {
-      fail(
-          "There are expected colors not present on the screenshot: "
-              + Sets.difference(expectedColors, cleanActualColors));
-    }
+    assertThat(cleanActualColors).containsExactlyInAnyOrderElementsOf(expectedColors);
   }
 
   private boolean onlyBlack(Set<String> colors) {
@@ -406,13 +385,8 @@ private boolean onlyWhite(Set<String> colors) {
    * @param im image
    */
   @SuppressWarnings("unused")
-  private void saveImageToTmpFile(String testMethodName, BufferedImage im) {
-
+  private void saveImageToTmpFile(String testMethodName, BufferedImage im) throws IOException {
     File outputfile = new File(testMethodName + "_image.png");
-    try {
-      ImageIO.write(im, "png", outputfile);
-    } catch (IOException e) {
-      fail("Unable to write image to file: " + e.getMessage());
-    }
+    ImageIO.write(im, "png", outputfile);
   }
 }

java/test/org/openqa/selenium/WebScriptTest.java

@@ -18,8 +18,7 @@
 package org.openqa.selenium;
 
 import static java.util.concurrent.TimeUnit.SECONDS;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.fail;
+import static org.assertj.core.api.Assertions.*;
 import static org.openqa.selenium.support.ui.ExpectedConditions.visibilityOf;
 
 import java.time.Duration;
@@ -90,12 +89,10 @@ void canRemoveConsoleMessageHandler()
     ConsoleLogEntry logEntry = future1.get(5, TimeUnit.SECONDS);
     assertThat(logEntry.getText()).isEqualTo("Hello, world!");
 
-    try {
-      future2.get(5, TimeUnit.SECONDS);
-      fail("Should be able to read the console messages");
-    } catch (TimeoutException e) {
-      assertThat(e).isNotNull();
-    }
+    assertThatThrownBy(() -> future2.get(5, TimeUnit.SECONDS))
+        .as("Should be able to read the console messages")
+        .isInstanceOf(TimeoutException.class);
+
     ((RemoteWebDriver) driver).script().removeConsoleMessageHandler(id1);
   }
 
@@ -144,12 +141,9 @@ void canRemoveJsErrorHandler() throws ExecutionException, InterruptedException,
     assertThat(logEntry.getType()).isEqualTo("javascript");
     assertThat(logEntry.getLevel()).isEqualTo(LogLevel.ERROR);
 
-    try {
-      future2.get(5, TimeUnit.SECONDS);
-      fail("Should be able to read the JS errors");
-    } catch (TimeoutException e) {
-      assertThat(e).isNotNull();
-    }
+    assertThatThrownBy(() -> future2.get(5, TimeUnit.SECONDS))
+        .as("Should be able to read the JS errors")
+        .isInstanceOf(TimeoutException.class);
 
     ((RemoteWebDriver) driver).script().removeConsoleMessageHandler(id1);
   }
@@ -259,7 +253,7 @@ void canPinScript() throws ExecutionException, InterruptedException, TimeoutExce
 
   @Test
   @NeedsFreshDriver
-  void canUnpinScript() throws ExecutionException, InterruptedException, TimeoutException {
+  void canUnpinScript() {
     CountDownLatch latch = new CountDownLatch(2);
 
     String pinnedScript =