[extension-selenium] Improve logging of search results (#3202)

vividus-extension-selenium/src/main/java/org/vividus/ui/action/search/AbstractElementAction.java

@@ -23,6 +23,7 @@
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang3.StringUtils;
 import org.openqa.selenium.By;
 import org.openqa.selenium.SearchContext;
 import org.openqa.selenium.StaleElementReferenceException;
@@ -37,6 +38,10 @@ public abstract class AbstractElementAction implements IElementAction
 {
  private static final Logger LOGGER = LoggerFactory.getLogger(AbstractElementAction.class);
 
+ private static final String NUMBER_OF_ELEMENTS_MESSAGE = "The total number of elements found by \"{}\" is {}";
+ private static final String NUMBER_OF_FILTERED_ELEMENTS_MESSAGE =
+ NUMBER_OF_ELEMENTS_MESSAGE + ", the number of {} elements is {}";
+
  private IWaitActions waitActions;
  @Inject private ElementActions elementActions;
  private Duration waitForElementTimeout;
@@ -75,22 +80,37 @@ private List<WebElement> findElements(SearchContext searchContext, By locator, V
  return Objects.requireNonNullElseGet(value, List::of);
  }
  List<WebElement> elements = searchContext.findElements(locator);
- LOGGER.atInfo()
- .addArgument(locator)
- .addArgument(elements::size)
- .log("Total number of elements found {} is {}");
+
+ if (Visibility.ALL == visibility || elements.isEmpty())
+ {
+ LOGGER.atInfo()
+ .addArgument(() -> convertLocatorToReadableForm(locator))
+ .addArgument(elements::size)
+ .log(NUMBER_OF_ELEMENTS_MESSAGE);
+ return elements;
+ }
  try
  {
- return Visibility.ALL == visibility || elements.isEmpty()
- ? elements
- : filterElementsByVisibility(elements, visibility, retry);
+ List<WebElement> filteredElements = filterElementsByVisibility(elements, visibility, retry);
+ LOGGER.atInfo()
+ .addArgument(() -> convertLocatorToReadableForm(locator))
+ .addArgument(elements::size)
+ .addArgument(visibility::getDescription)
+ .addArgument(filteredElements::size)
+ .log(NUMBER_OF_FILTERED_ELEMENTS_MESSAGE);
+ return filteredElements;
  }
  catch (StaleElementReferenceException e)
  {
  return findElements(searchContext, locator, visibility, false, true);
  }
  }
 
+ private static String convertLocatorToReadableForm(By locator)
+ {
+ return StringUtils.removeStart(locator.toString(), "By.");
+ }
+
  protected List<WebElement> filterElementsByVisibility(List<WebElement> elements, Visibility visibility,
  boolean retry)
  {
@@ -109,13 +129,7 @@ protected List<WebElement> filterElementsByVisibility(List<WebElement> elements,
  LOGGER.warn(e.getMessage(), e);
  return false;
  }
- }).collect(Collectors.collectingAndThen(Collectors.toList(), list ->
- {
- LOGGER.atInfo().addArgument(visibility::getDescription)
- .addArgument(list::size)
- .log("Number of {} elements is {}");
- return list;
- }));
+ }).collect(Collectors.toList());
  }
 
  private List<WebElement> waitForElement(SearchContext searchContext, By locator, Visibility visibility)

vividus-extension-selenium/src/test/java/org/vividus/ui/action/search/AbstractElementSearchActionTests.java

@@ -18,12 +18,15 @@
 
 import static com.github.valfirst.slf4jtest.LoggingEvent.error;
 import static com.github.valfirst.slf4jtest.LoggingEvent.info;
+import static com.github.valfirst.slf4jtest.LoggingEvent.warn;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.equalTo;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.params.provider.Arguments.arguments;
 import static org.mockito.ArgumentMatchers.argThat;
 import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
@@ -34,6 +37,7 @@
 import java.util.List;
 import java.util.stream.Stream;
 
+import com.github.valfirst.slf4jtest.LoggingEvent;
 import com.github.valfirst.slf4jtest.TestLogger;
 import com.github.valfirst.slf4jtest.TestLoggerFactory;
 import com.github.valfirst.slf4jtest.TestLoggerFactoryExtension;
@@ -62,29 +66,33 @@
 @ExtendWith({ TestLoggerFactoryExtension.class, MockitoExtension.class })
 class AbstractElementSearchActionTests
 {
- private static final String TOTAL_NUMBER_OF_ELEMENTS = "Total number of elements found {} is {}";
- private static final String NUMBER_OF_VISIBLE_ELEMENTS = "Number of {} elements is {}";
+ private static final String TOTAL_NUMBER_OF_ELEMENTS = "The total number of elements found by \"{}\" is {}";
+ private static final String NUMBER_OF_VISIBLE_ELEMENTS =
+ TOTAL_NUMBER_OF_ELEMENTS + ", the number of {} elements is {}";
  private static final String EXCEPTION = "exception";
 
+ private static final String ID = "id1";
+ private static final By LOCATOR = By.id(ID);
+ private static final String LOGGED_LOCATOR = "id: " + ID;
+
  private final TestLogger logger = TestLoggerFactory.getTestLogger(AbstractElementAction.class);
 
  @Mock private SearchContext searchContext;
  @Mock private IWaitActions waitActions;
- @Mock private By locator;
  @Mock private ElementActions elementActions;
 
  @InjectMocks
  private final AbstractElementAction elementSearchAction = new AbstractElementAction(TestLocatorType.SEARCH) { };
 
  private void mockWaitForElements(List<WebElement> foundElements, List<WebElement> filteredElements)
  {
- Duration timeout = Duration.ofSeconds(1);
+ var timeout = Duration.ofSeconds(1);
  elementSearchAction.setWaitForElementTimeout(timeout);
  var waitResult = new WaitResult<List<WebElement>>();
  waitResult.setData(filteredElements);
  when(waitActions.wait(eq(searchContext), eq(timeout),
  argThat((ArgumentMatcher<IExpectedSearchContextCondition<List<WebElement>>>) condition -> {
- when(searchContext.findElements(locator)).thenReturn(foundElements);
+ when(searchContext.findElements(LOCATOR)).thenReturn(foundElements);
  condition.apply(searchContext);
  return true;
  }), eq(false))).thenReturn(waitResult);
@@ -100,7 +108,7 @@ void testGetAttributeType()
  void testFindElementsSearchContextIsNull()
  {
  var parameters = mock(SearchParameters.class);
- var elements = elementSearchAction.findElements(null, locator, parameters);
+ var elements = elementSearchAction.findElements(null, LOCATOR, parameters);
  assertThat(elements, empty());
  assertThat(logger.getLoggingEvents(), equalTo(List.of(
  error("Unable to locate elements, because search context is not set"))));
@@ -110,9 +118,11 @@ void testFindElementsSearchContextIsNull()
  void shouldFindNoElements()
  {
  mockWaitForElements(List.of(), null);
- var foundElements = elementSearchAction.findElements(searchContext, locator, new SearchParameters());
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR, new SearchParameters());
  assertThat(foundElements, empty());
- assertThat(logger.getLoggingEvents(), equalTo(List.of(info(TOTAL_NUMBER_OF_ELEMENTS, locator, 0))));
+ assertThat(logger.getLoggingEvents(), equalTo(List.of(
+ info(TOTAL_NUMBER_OF_ELEMENTS, LOGGED_LOCATOR, 0)
+ )));
  }
 
  @Test
@@ -122,11 +132,10 @@ void shouldFindVisibleElements()
  when(elementActions.isElementVisible(element)).thenReturn(true);
  var elements = List.of(element);
  mockWaitForElements(elements, elements);
- var foundElements = elementSearchAction.findElements(searchContext, locator, new SearchParameters());
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR, new SearchParameters());
  assertEquals(elements, foundElements);
  assertThat(logger.getLoggingEvents(), equalTo(List.of(
- info(TOTAL_NUMBER_OF_ELEMENTS, locator, 1),
- info(NUMBER_OF_VISIBLE_ELEMENTS, Visibility.VISIBLE.getDescription(), 1)
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 1, Visibility.VISIBLE.getDescription(), 1)
  )));
  }
 
@@ -136,11 +145,13 @@ void shouldFindAllElements()
  var element = mock(WebElement.class);
  var elements = List.of(element);
  mockWaitForElements(elements, elements);
- var foundElements = elementSearchAction.findElements(searchContext, locator,
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR,
  new SearchParameters().setVisibility(Visibility.ALL));
  assertEquals(elements, foundElements);
  verifyNoInteractions(element, elementActions);
- assertThat(logger.getLoggingEvents(), equalTo(List.of(info(TOTAL_NUMBER_OF_ELEMENTS, locator, 1))));
+ assertThat(logger.getLoggingEvents(), equalTo(List.of(
+ info(TOTAL_NUMBER_OF_ELEMENTS, LOGGED_LOCATOR, 1)
+ )));
  }
 
  @Test
@@ -152,12 +163,11 @@ void shouldFindInvisibleElements()
  when(elementActions.isElementVisible(element2)).thenReturn(Boolean.FALSE);
  var elements = List.of(element1, element2);
  mockWaitForElements(elements, List.of(element2));
- var foundElements = elementSearchAction.findElements(searchContext, locator,
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR,
  new SearchParameters().setVisibility(Visibility.INVISIBLE));
  assertEquals(List.of(element2), foundElements);
  assertThat(logger.getLoggingEvents(), equalTo(List.of(
- info(TOTAL_NUMBER_OF_ELEMENTS, locator, 2),
- info(NUMBER_OF_VISIBLE_ELEMENTS, Visibility.INVISIBLE.getDescription(), 1)
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 2, Visibility.INVISIBLE.getDescription(), 1)
  )));
  }
 
@@ -167,59 +177,64 @@ void testFindElementsDisplayedOnly()
  var element1 = mock(WebElement.class);
  var element2 = mock(WebElement.class);
  var elementsList = List.of(element1, element2);
- when(searchContext.findElements(locator)).thenReturn(elementsList);
+ when(searchContext.findElements(LOCATOR)).thenReturn(elementsList);
  when(elementActions.isElementVisible(element1)).thenReturn(Boolean.TRUE);
  when(elementActions.isElementVisible(element2)).thenReturn(Boolean.FALSE);
- var foundElements = elementSearchAction.findElements(searchContext, locator,
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR,
  new SearchParameters().setWaitForElement(false));
  assertEquals(List.of(element1), foundElements);
  assertThat(logger.getLoggingEvents(), equalTo(List.of(
- info(TOTAL_NUMBER_OF_ELEMENTS, locator, 2),
- info(NUMBER_OF_VISIBLE_ELEMENTS, Visibility.VISIBLE.getDescription(), 1)
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 2, Visibility.VISIBLE.getDescription(), 1)
  )));
  }
 
  @Test
  void testFindAllElementsWithException()
  {
  var element = mock(WebElement.class);
- var elements = List.of(element);
- Mockito.doThrow(new StaleElementReferenceException(EXCEPTION)).when(elementActions).isElementVisible(element);
- when(searchContext.findElements(locator)).thenReturn(elements);
- var foundElements = elementSearchAction.findElements(searchContext, locator,
+ var exception = new StaleElementReferenceException(EXCEPTION);
+ doThrow(exception).when(elementActions).isElementVisible(element);
+ when(searchContext.findElements(LOCATOR)).thenReturn(List.of(element));
+ var foundElements = elementSearchAction.findElements(searchContext, LOCATOR,
  new SearchParameters().setWaitForElement(false));
  assertEquals(0, foundElements.size());
  verify(element, Mockito.never()).getSize();
  verifyNoInteractions(waitActions);
- assertThat(logger.getLoggingEvents().get(0), equalTo(info(TOTAL_NUMBER_OF_ELEMENTS, locator, 1)));
+ assertThat(logger.getLoggingEvents(), equalTo(List.of(
+ warn(exception, exception.getMessage()),
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 1, Visibility.VISIBLE.getDescription(), 0)
+ )));
  }
 
  private static Stream<Arguments> provideStaleElementTestData()
  {
- Answer<Boolean> elementStale = invocation -> {
- throw new StaleElementReferenceException(EXCEPTION);
- };
- Answer<Boolean> elementDisplayed = invocation -> true;
- return Stream.of(Arguments.of(elementStale, 0, 2), Arguments.of(elementDisplayed, 1, 2));
+ var exception = new StaleElementReferenceException(EXCEPTION);
+ return Stream.of(
+ arguments((Answer<Boolean>) invocation -> { throw exception; }, 0, List.of(
+ warn(exception, exception.getMessage()),
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 1, Visibility.VISIBLE.getDescription(), 0)
+ )),
+ arguments((Answer<Boolean>) invocation -> true, 1, List.of(
+ info(NUMBER_OF_VISIBLE_ELEMENTS, LOGGED_LOCATOR, 1, Visibility.VISIBLE.getDescription(), 1)
+ ))
+ );
  }
 
  @ParameterizedTest
  @MethodSource("provideStaleElementTestData")
- void testStaleElementSearchRetry(Answer<Boolean> answer, int expectedSize,
- int isDisplayedMethodInvocations)
+ void testStaleElementSearchRetry(Answer<Boolean> answer, int expectedSize, List<LoggingEvent> loggingEvents)
  {
  elementSearchAction.setRetrySearchIfStale(true);
  var element = mock(WebElement.class);
- var elements = List.of(element);
- Mockito.doThrow(new StaleElementReferenceException(EXCEPTION)).doAnswer(answer)
- .when(elementActions).isElementVisible(element);
- when(searchContext.findElements(locator)).thenReturn(elements);
+ var exception = new StaleElementReferenceException(EXCEPTION);
+ doThrow(exception).doAnswer(answer).when(elementActions).isElementVisible(element);
+ when(searchContext.findElements(LOCATOR)).thenReturn(List.of(element));
  var foundElements = elementSearchAction
- .findElements(searchContext, locator, new SearchParameters().setWaitForElement(false));
+ .findElements(searchContext, LOCATOR, new SearchParameters().setWaitForElement(false));
  assertEquals(expectedSize, foundElements.size());
  verify(element, Mockito.never()).getSize();
  verifyNoInteractions(waitActions);
- verify(elementActions, times(isDisplayedMethodInvocations)).isElementVisible(element);
- assertThat(logger.getLoggingEvents().get(0), equalTo(info(TOTAL_NUMBER_OF_ELEMENTS, locator, 1)));
+ verify(elementActions, times(2)).isElementVisible(element);
+ assertThat(logger.getLoggingEvents(), equalTo(loggingEvents));
  }
 }

vividus-plugin-web-app/src/main/java/org/vividus/ui/web/action/search/AbstractWebElementSearchAction.java

@@ -54,7 +54,7 @@ protected List<WebElement> findElementsByText(SearchContext searchContext, By de
  if (elements.isEmpty())
  {
  String text = parameters.getValue();
- By newLocator = generateCaseInsensitiveLocator(text, tagNames);
+ By newLocator = By.xpath(generateCaseInsensitiveXpath(text, tagNames));
  return findElements(searchContext, newLocator, parameters)
  .stream()
  .filter(element -> matchesToText(element, text))
@@ -63,7 +63,7 @@ protected List<WebElement> findElementsByText(SearchContext searchContext, By de
  return elements;
  }
 
- protected static By generateCaseInsensitiveLocator(String text, String... tagNames)
+ protected static String generateCaseInsensitiveXpath(String text, String... tagNames)
  {
  @SuppressWarnings("PMD.InsufficientStringBufferDeclaration")
  StringBuilder locator = new StringBuilder();
@@ -77,7 +77,7 @@ protected static By generateCaseInsensitiveLocator(String text, String... tagNam
  .append(ELEMENT_WITH_ANY_ATTRIBUTE_OR_TEXT_CASE_INSENSITIVE)
  .append("])]|");
  }
- return XpathLocatorUtils.getXPathLocator(locator.substring(0, locator.length() - 1), text.toLowerCase());
+ return XpathLocatorUtils.getXPath(locator.substring(0, locator.length() - 1), text.toLowerCase());
  }
 
  protected boolean matchesToText(WebElement element, final String text)

vividus-plugin-web-app/src/main/java/org/vividus/ui/web/action/search/WebLocatorType.java

@@ -43,7 +43,7 @@ public enum WebLocatorType implements LocatorType
  @Override
  public By buildBy(String value)
  {
- return AbstractWebElementSearchAction.generateCaseInsensitiveLocator(value.toLowerCase(), "*");
+ return By.xpath(AbstractWebElementSearchAction.generateCaseInsensitiveXpath(value.toLowerCase(), "*"));
  }
  },
  TOOLTIP("Tooltip", TooltipFilter.class)