Skip to content

Commit

Permalink
big cleanup of unit-tests
Browse files Browse the repository at this point in the history
* create names for test-methods describing the case or behaviour they
are testing
* avoid names of test methods starting with "test". It's enough to have
annotation @test.
* mock with "any()" and verify with concrete parameters
* rename "Cleanup.of.wrap" to "wrapInvalidSelectorException"
* remove useless unit-tests for constructors and getters
* avoid using helper methods with boolean parameters and asserts inside
* use static import instead of extending WithAssertions
  • Loading branch information
asolntsev committed Sep 27, 2021
1 parent 6bd7095 commit 8b32a24
Show file tree
Hide file tree
Showing 141 changed files with 1,222 additions and 1,362 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ It defines concise fluent API, natural language assertions and does some magic f
Selenide is based on and is compatible to Selenium WebDriver 2.0+ and 3.0+

@Test
public void testLogin() {
public void login() {
open("/login");
$(By.name("user.name")).setValue("johny");
$("#submit").click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,16 @@

import com.codeborne.selenide.logevents.ErrorsCollector;
import com.codeborne.selenide.logevents.SelenideLogger;
import org.assertj.core.api.WithAssertions;
import org.testng.IClass;
import org.testng.ITestNGMethod;
import org.testng.ITestResult;
import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Listeners;
import org.testng.annotations.Test;
import org.testng.internal.ConstructorOrMethod;

import static com.codeborne.selenide.logevents.ErrorsCollector.LISTENER_SOFT_ASSERT;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.RETURNS_DEEP_STUBS;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doNothing;
Expand All @@ -24,7 +23,7 @@
import static org.mockito.Mockito.when;
import static org.testng.ITestResult.FAILURE;

final class SoftAssertsTest implements WithAssertions {
final class SoftAssertsTest {
private final SoftAsserts listener = new SoftAsserts();

@AfterMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ protected void waitUntil(CollectionCondition condition, Duration timeout) {
}
catch (WebDriverException | IndexOutOfBoundsException | UIAssertionError elementNotFound) {
if (Cleanup.of.isInvalidSelectorError(elementNotFound)) {
throw Cleanup.of.wrap(elementNotFound);
throw Cleanup.of.wrapInvalidSelectorException(elementNotFound);
}
if (condition.missingElementSatisfiesCondition()) {
return;
Expand Down
8 changes: 5 additions & 3 deletions src/main/java/com/codeborne/selenide/commands/Exists.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ public Boolean execute(SelenideElement proxy, WebElementSource locator, @Nullabl
//noinspection ResultOfMethodCallIgnored
locator.getWebElement();
return true;
} catch (WebDriverException | ElementNotFound elementNotFound) {
}
catch (WebDriverException | ElementNotFound elementNotFound) {
if (Cleanup.of.isInvalidSelectorError(elementNotFound)) {
throw Cleanup.of.wrap(elementNotFound);
throw Cleanup.of.wrapInvalidSelectorException(elementNotFound);
}
return false;
} catch (IndexOutOfBoundsException invalidElementIndex) {
}
catch (IndexOutOfBoundsException invalidElementIndex) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,10 @@ public class GetLastChild implements Command<SelenideElement> {
private final Find find;

public GetLastChild() {
find = new Find();
this(new Find());
}

public GetLastChild(Find find) {
GetLastChild(Find find) {
this.find = find;
}

Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/codeborne/selenide/commands/GetParent.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import com.codeborne.selenide.Command;
import com.codeborne.selenide.SelenideElement;
import com.codeborne.selenide.impl.WebElementSource;

import org.openqa.selenium.By;

import javax.annotation.CheckReturnValue;
Expand All @@ -16,7 +15,7 @@ public class GetParent implements Command<SelenideElement> {
private final Find find;

GetParent() {
this.find = new Find();
this(new Find());
}

GetParent(Find find) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ public class GetPreceding implements Command<SelenideElement> {
@CheckReturnValue
@Nonnull
public SelenideElement execute(SelenideElement proxy, WebElementSource locator, @Nullable Object[] args) {
assert args != null;

int siblingIndex = (int) firstOf(args) + 1;
return locator.find(proxy, By.xpath(String.format("preceding-sibling::*[%d]", siblingIndex)), 0);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,24 +8,23 @@
import javax.annotation.CheckReturnValue;
import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import java.io.IOException;

@ParametersAreNonnullByDefault
public class GetSelectedValue implements Command<String> {
private final Command<SelenideElement> getSelectedOption;
private final GetSelectedOption getSelectedOption;

public GetSelectedValue() {
this.getSelectedOption = new GetSelectedOption();
this(new GetSelectedOption());
}

public GetSelectedValue(Command<SelenideElement> getSelectedOption) {
GetSelectedValue(GetSelectedOption getSelectedOption) {
this.getSelectedOption = getSelectedOption;
}

@Override
@CheckReturnValue
@Nullable
public String execute(SelenideElement proxy, WebElementSource selectElement, @Nullable Object[] args) throws IOException {
public String execute(SelenideElement proxy, WebElementSource selectElement, @Nullable Object[] args) {
WebElement option = getSelectedOption.execute(proxy, selectElement, args);
return option == null ? null : option.getAttribute("value");
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/com/codeborne/selenide/commands/GetText.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ public class GetText implements Command<String> {
private final GetSelectedText getSelectedText;

public GetText() {
this.getSelectedText = new GetSelectedText();
this(new GetSelectedText());
}

public GetText(GetSelectedText getSelectedtext) {
GetText(GetSelectedText getSelectedtext) {
this.getSelectedText = getSelectedtext;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public Boolean execute(SelenideElement proxy, WebElementSource locator, @Nullabl
}
catch (WebDriverException | ElementNotFound elementNotFound) {
if (Cleanup.of.isInvalidSelectorError(elementNotFound)) {
throw Cleanup.of.wrap(elementNotFound);
throw Cleanup.of.wrapInvalidSelectorException(elementNotFound);
}
return false;
}
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/com/codeborne/selenide/commands/Matches.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,14 @@ public Boolean execute(SelenideElement proxy, WebElementSource locator, @Nullabl
protected WebElement getElementOrNull(WebElementSource locator) {
try {
return locator.getWebElement();
} catch (WebDriverException | ElementNotFound elementNotFound) {
}
catch (WebDriverException | ElementNotFound elementNotFound) {
if (Cleanup.of.isInvalidSelectorError(elementNotFound))
throw Cleanup.of.wrap(elementNotFound);
throw Cleanup.of.wrapInvalidSelectorException(elementNotFound);
return null;
} catch (IndexOutOfBoundsException ignore) {
}
catch (IndexOutOfBoundsException ignore) {
return null;
} catch (RuntimeException e) {
throw Cleanup.of.wrap(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

import javax.annotation.Nullable;
import javax.annotation.ParametersAreNonnullByDefault;
import java.util.Arrays;

import static com.codeborne.selenide.Condition.exist;

Expand All @@ -22,11 +23,15 @@ public Void execute(SelenideElement proxy, WebElementSource selectField, @Nullab
}
else if (args[0] instanceof String[]) {
selectOptionsByTexts(selectField, (String[]) args[0]);
return null;
}
else if (args[0] instanceof int[]) {
selectOptionsByIndexes(selectField, (int[]) args[0]);
return null;
}
else {
throw new IllegalArgumentException("Unsupported argument (expected String or Integer): " + Arrays.toString(args));
}
return null;
}

private void selectOptionsByTexts(WebElementSource selectField, String[] texts) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ public class SelectRadio implements Command<SelenideElement> {
private final Click click;

public SelectRadio() {
this.click = new Click();
this(new Click());
}

public SelectRadio(Click click) {
SelectRadio(Click click) {
this.click = click;
}

Expand Down
25 changes: 14 additions & 11 deletions src/main/java/com/codeborne/selenide/commands/SetValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,10 @@ public class SetValue implements Command<SelenideElement> {
private final SelectRadio selectRadio;

public SetValue() {
this.selectOptionByValue = new SelectOptionByValue();
this.selectRadio = new SelectRadio();
this(new SelectOptionByValue(), new SelectRadio());
}

public SetValue(SelectOptionByValue selectOptionByValue, SelectRadio selectRadio) {
SetValue(SelectOptionByValue selectOptionByValue, SelectRadio selectRadio) {
this.selectOptionByValue = selectOptionByValue;
this.selectRadio = selectRadio;
}
Expand All @@ -34,19 +33,23 @@ public SetValue(SelectOptionByValue selectOptionByValue, SelectRadio selectRadio
public SelenideElement execute(SelenideElement proxy, WebElementSource locator, @Nullable Object[] args) {
String text = firstOf(args);
WebElement element = locator.findAndAssertElementIsInteractable();
Driver driver = locator.driver();

if (locator.driver().config().versatileSetValue()
&& "select".equalsIgnoreCase(element.getTagName())) {
selectOptionByValue.execute(proxy, locator, args);
if (!driver.config().versatileSetValue()) {
setValueForTextInput(driver, element, text);
return proxy;
}
if (locator.driver().config().versatileSetValue()
&& "input".equalsIgnoreCase(element.getTagName()) && "radio".equals(element.getAttribute("type"))) {

String tagName = element.getTagName();
if ("select".equalsIgnoreCase(tagName)) {
selectOptionByValue.execute(proxy, locator, args);
}
else if ("input".equalsIgnoreCase(tagName) && "radio".equals(element.getAttribute("type"))) {
selectRadio.execute(proxy, locator, args);
return proxy;
}

setValueForTextInput(locator.driver(), element, text);
else {
setValueForTextInput(driver, element, text);
}
return proxy;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/codeborne/selenide/commands/Val.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public Val() {
this.setValue = new SetValue();
}

public Val(GetValue getValue, SetValue setValue) {
Val(GetValue getValue, SetValue setValue) {
this.getValue = getValue;
this.setValue = setValue;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/codeborne/selenide/impl/Cleanup.java
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ private boolean isInvalidSelectorMessage(String message) {
message.contains("INVALID_EXPRESSION_ERR");
}

public InvalidSelectorException wrap(Throwable error) {
public InvalidSelectorException wrapInvalidSelectorException(Throwable error) {
return (error instanceof InvalidSelectorException) ?
(InvalidSelectorException) error :
new InvalidSelectorException("Invalid selector", error);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ protected Object dispatchAndRetry(long timeoutMs, long pollingIntervalMs,
}

if (Cleanup.of.isInvalidSelectorError(lastError)) {
throw Cleanup.of.wrap(lastError);
throw Cleanup.of.wrapInvalidSelectorException(lastError);
}
else if (!shouldRetryAfterError(lastError)) {
throw lastError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ protected List<WebElement> evaluateSizzleSelector(Driver driver, SearchContext c
injectSizzleIfNeeded(driver);

String sizzleSelector = sizzleCssSelector.toString()
.replace("By.selector: ", "")
.replace("By.cssSelector: ", "");
.replace("By.selector: ", "")
.replace("By.cssSelector: ", "");

if (context instanceof WebElement)
return driver.executeJavaScript("return Sizzle(arguments[0], arguments[1])", sizzleSelector, context);
Expand All @@ -110,7 +110,8 @@ protected void injectSizzleIfNeeded(Driver driver) {
protected Boolean sizzleLoaded(Driver driver) {
try {
return driver.executeJavaScript("return typeof Sizzle != 'undefined'");
} catch (WebDriverException e) {
}
catch (WebDriverException e) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public WebElement checkCondition(String prefix, Condition condition, boolean inv
}

if (lastError != null && Cleanup.of.isInvalidSelectorError(lastError)) {
throw Cleanup.of.wrap(lastError);
throw Cleanup.of.wrapInvalidSelectorException(lastError);
}

if (element == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import javax.annotation.ParametersAreNonnullByDefault;

import static com.codeborne.selenide.logevents.LogEvent.EventStatus.IN_PROGRESS;
import static java.util.concurrent.TimeUnit.NANOSECONDS;

@ParametersAreNonnullByDefault
public class SelenideLog implements LogEvent {
Expand Down Expand Up @@ -47,7 +48,7 @@ public String getElement() {

@Override
public long getDuration() {
return (endNs - startNs) / 1_000_000;
return NANOSECONDS.toMillis(endNs - startNs);
}

@Override
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/codeborne/selenide/ClickOptionsTest.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package com.codeborne.selenide;

import org.assertj.core.api.WithAssertions;
import org.junit.jupiter.api.Test;

import static com.codeborne.selenide.ClickMethod.DEFAULT;
import static com.codeborne.selenide.ClickMethod.JS;
import static com.codeborne.selenide.ClickOptions.usingDefaultMethod;
import static com.codeborne.selenide.ClickOptions.usingJavaScript;
import static org.assertj.core.api.Assertions.assertThat;

final class ClickOptionsTest implements WithAssertions {
final class ClickOptionsTest {
@Test
void javaScriptMethod() {
assertThat(usingJavaScript().clickOption()).isEqualTo(JS);
Expand Down
Loading

0 comments on commit 8b32a24

Please sign in to comment.