Skip to content

Commit 938ec64

Browse files
Code style issues which were found by reviewer were fixed
1 parent 7307777 commit 938ec64

File tree

6 files changed

+54
-61
lines changed

6 files changed

+54
-61
lines changed

src/main/java/io/appium/java_client/pagefactory/AppiumElementLocator.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import org.openqa.selenium.SearchContext;
2929
import org.openqa.selenium.StaleElementReferenceException;
3030
import org.openqa.selenium.TimeoutException;
31-
import org.openqa.selenium.WebDriver;
3231
import org.openqa.selenium.WebDriverException;
3332
import org.openqa.selenium.WebElement;
3433
import org.openqa.selenium.support.ui.FluentWait;
@@ -42,8 +41,7 @@ class AppiumElementLocator implements CacheableLocator {
4241

4342
private final boolean shouldCache;
4443
private final By by;
45-
private final TimeOutDuration timeOutDuration;
46-
private final TimeOutDuration originalTimeOutDuration;
44+
private final TimeOutDuration duration;
4745
private final SearchContext searchContext;
4846
private WebElement cachedElement;
4947
private List<WebElement> cachedElementList;
@@ -58,25 +56,23 @@ class AppiumElementLocator implements CacheableLocator {
5856
* @param shouldCache is the flag that signalizes that elements which
5957
* are found once should be cached
6058
* @param duration is a POJO which contains timeout parameters for the element to be searched
61-
* @param originalDuration is a POJO which contains timeout parameters from page object which contains the element
6259
*/
6360

6461
public AppiumElementLocator(SearchContext searchContext, By by, boolean shouldCache,
65-
TimeOutDuration duration, TimeOutDuration originalDuration) {
62+
TimeOutDuration duration) {
6663
this.searchContext = searchContext;
6764
this.shouldCache = shouldCache;
68-
this.timeOutDuration = duration;
69-
this.originalTimeOutDuration = originalDuration;
65+
this.duration = duration;
7066
this.by = by;
7167
this.exceptionMessageIfElementNotFound = "Can't locate an element by this strategy: " + by.toString();
7268
}
7369

74-
private <T extends Object> T waitFor(Supplier<T> supplier) {
70+
private <T> T waitFor(Supplier<T> supplier) {
7571
WaitingFunction<T> function = new WaitingFunction<>();
7672
try {
7773
FluentWait<Supplier<T>> wait = new FluentWait<>(supplier)
7874
.ignoring(NoSuchElementException.class);
79-
wait.withTimeout(timeOutDuration.getTime(), timeOutDuration.getTimeUnit());
75+
wait.withTimeout(duration.getTime(), duration.getTimeUnit());
8076
return wait.until(function);
8177
} catch (TimeoutException e) {
8278
if (function.foundStaleElementReferenceException != null) {

src/main/java/io/appium/java_client/pagefactory/AppiumElementLocatorFactory.java

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package io.appium.java_client.pagefactory;
1818

19+
import static java.util.Optional.ofNullable;
20+
1921
import io.appium.java_client.pagefactory.bys.builder.AppiumByBuilder;
2022
import io.appium.java_client.pagefactory.locator.CacheableElementLocatorFactory;
2123
import io.appium.java_client.pagefactory.locator.CacheableLocator;
@@ -27,21 +29,21 @@
2729

2830
public class AppiumElementLocatorFactory implements CacheableElementLocatorFactory {
2931
private final SearchContext searchContext;
30-
private final TimeOutDuration timeOutDuration;
32+
private final TimeOutDuration duration;
3133
private final AppiumByBuilder builder;
3234

3335
/**
3436
* Creates a new mobile element locator factory.
3537
*
3638
* @param searchContext The context to use when finding the element
37-
* @param timeOutDuration is a POJO which contains timeout parameters for the element to be searched
38-
* @param builder is handler of Appium-specific page object annotations
39+
* @param duration is a POJO which contains timeout parameters for the element to be searched
40+
* @param builder is handler of Appium-specific page object annotations
3941
*/
4042

41-
public AppiumElementLocatorFactory(SearchContext searchContext, TimeOutDuration timeOutDuration,
43+
public AppiumElementLocatorFactory(SearchContext searchContext, TimeOutDuration duration,
4244
AppiumByBuilder builder) {
4345
this.searchContext = searchContext;
44-
this.timeOutDuration = timeOutDuration;
46+
this.duration = duration;
4547
this.builder = builder;
4648
}
4749

@@ -55,16 +57,15 @@ public CacheableLocator createLocator(Field field) {
5557
WithTimeout withTimeout = annotatedElement.getAnnotation(WithTimeout.class);
5658
customDuration = new TimeOutDuration(withTimeout.time(), withTimeout.unit());
5759
} else {
58-
customDuration = timeOutDuration;
60+
customDuration = duration;
5961
}
6062

6163
builder.setAnnotated(annotatedElement);
62-
By by = builder.buildBy();
63-
if (by != null) {
64-
return new AppiumElementLocator(searchContext, by, builder.isLookupCached(),
65-
customDuration, timeOutDuration);
66-
}
67-
return null;
64+
By byResult = builder.buildBy();
65+
66+
return ofNullable(byResult)
67+
.map(by -> new AppiumElementLocator(searchContext, by, builder.isLookupCached(), customDuration))
68+
.orElse(null);
6869
}
6970

7071

src/main/java/io/appium/java_client/pagefactory/AppiumFieldDecorator.java

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@ public class AppiumFieldDecorator implements FieldDecorator {
6969
private final AppiumElementLocatorFactory widgetLocatorFactory;
7070
private final String platform;
7171
private final String automation;
72-
private final TimeOutDuration timeOutDuration;
72+
private final TimeOutDuration duration;
7373
private final HasSessionDetails hasSessionDetails;
7474

7575

76-
public AppiumFieldDecorator(SearchContext context, long timeOut,
76+
public AppiumFieldDecorator(SearchContext context, long timeout,
7777
TimeUnit timeUnit) {
78-
this(context, new TimeOutDuration(timeOut, timeUnit));
78+
this(context, new TimeOutDuration(timeout, timeUnit));
7979
}
8080

8181
/**
@@ -84,9 +84,9 @@ public AppiumFieldDecorator(SearchContext context, long timeOut,
8484
* or {@link org.openqa.selenium.WebElement} or
8585
* {@link io.appium.java_client.pagefactory.Widget} or some other user's
8686
* extension/implementation.
87-
* @param timeOutDuration is a desired duration of the waiting for an element presence.
87+
* @param duration is a desired duration of the waiting for an element presence.
8888
*/
89-
public AppiumFieldDecorator(SearchContext context, TimeOutDuration timeOutDuration) {
89+
public AppiumFieldDecorator(SearchContext context, TimeOutDuration duration) {
9090
this.originalDriver = unpackWebDriverFromSearchContext(context);
9191
if (originalDriver == null
9292
|| !HasSessionDetails.class.isAssignableFrom(originalDriver.getClass())) {
@@ -99,10 +99,10 @@ public AppiumFieldDecorator(SearchContext context, TimeOutDuration timeOutDurati
9999
automation = hasSessionDetails.getAutomationName();
100100
}
101101

102-
this.timeOutDuration = timeOutDuration;
102+
this.duration = duration;
103103

104104
defaultElementFieldDecoracor = new DefaultFieldDecorator(
105-
new AppiumElementLocatorFactory(context, timeOutDuration,
105+
new AppiumElementLocatorFactory(context, duration,
106106
new DefaultElementByBuilder(platform, automation))) {
107107
@Override
108108
protected WebElement proxyForLocator(ClassLoader ignored, ElementLocator locator) {
@@ -139,7 +139,7 @@ protected List<WebElement> proxyForListLocator(ClassLoader ignored,
139139
};
140140

141141
widgetLocatorFactory =
142-
new AppiumElementLocatorFactory(context, timeOutDuration, new WidgetByBuilder(platform, automation));
142+
new AppiumElementLocatorFactory(context, duration, new WidgetByBuilder(platform, automation));
143143
}
144144

145145
public AppiumFieldDecorator(SearchContext context) {
@@ -203,14 +203,14 @@ private Object decorateWidget(Field field) {
203203
if (isAlist) {
204204
return getEnhancedProxy(ArrayList.class,
205205
new WidgetListInterceptor(locator, originalDriver, map, widgetType,
206-
timeOutDuration));
206+
duration));
207207
}
208208

209209
Constructor<? extends Widget> constructor =
210210
WidgetConstructorUtil.findConvenientConstructor(widgetType);
211211
return getEnhancedProxy(widgetType, new Class[] {constructor.getParameterTypes()[0]},
212212
new Object[] {proxyForAnElement(locator)},
213-
new WidgetInterceptor(locator, originalDriver, null, map, timeOutDuration));
213+
new WidgetInterceptor(locator, originalDriver, null, map, duration));
214214
}
215215

216216
private WebElement proxyForAnElement(ElementLocator locator) {

src/test/java/io/appium/java_client/ChromeDriverPathUtil.java

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,17 +18,13 @@ public final class ChromeDriverPathUtil {
1818
* @return the choromedriver file which depends on platform.
1919
*/
2020
public static File getChromeDriver() {
21-
Path resultPath;
2221
Platform current = getCurrent();
23-
2422
if (current.is(WINDOWS)) {
25-
resultPath = ROOT_TEST_PATH.resolve("chromedriver.exe");
23+
return ROOT_TEST_PATH.resolve("chromedriver.exe").toFile();
2624
} else if (current.is(MAC)) {
27-
resultPath = ROOT_TEST_PATH.resolve("chromedriver_mac");
25+
return ROOT_TEST_PATH.resolve("chromedriver_mac").toFile();
2826
} else {
29-
resultPath = ROOT_TEST_PATH.resolve("chromedriver_linux");
27+
return ROOT_TEST_PATH.resolve("chromedriver_linux").toFile();
3028
}
31-
32-
return resultPath.toFile();
3329
}
3430
}

src/test/java/io/appium/java_client/pagefactory_tests/MobileBrowserCompatibilityTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public class MobileBrowserCompatibilityTest {
4444
private AppiumDriverLocalService service;
4545

4646
@AndroidFindBy(className = "someClass") @AndroidFindBy(xpath = "//someTag")
47-
private RemoteWebElement btnK; //this element should be found by id = 'btnG' or name = 'btnG'
47+
private RemoteWebElement btnG; //this element should be found by id = 'btnG' or name = 'btnG'
4848

4949
@FindBy(name = "q")
5050
@AndroidFindBy(uiAutomator = "new UiSelector().resourceId(\"android:id/someId\")")
@@ -87,7 +87,7 @@ public class MobileBrowserCompatibilityTest {
8787
driver.get("https://www.google.com");
8888

8989
searchTextField.sendKeys("Hello");
90-
btnK.click();
90+
btnG.click();
9191
Assert.assertNotEquals(0, foundLinks.size());
9292
}
9393

src/test/java/io/appium/java_client/pagefactory_tests/TimeOutTest.java renamed to src/test/java/io/appium/java_client/pagefactory_tests/TimeoutTest.java

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import static io.appium.java_client.pagefactory.AppiumFieldDecorator.DEFAULT_TIMEUNIT;
2222
import static java.lang.Math.abs;
2323
import static java.lang.String.format;
24+
import static java.lang.System.currentTimeMillis;
2425
import static java.lang.System.setProperty;
2526
import static java.util.concurrent.TimeUnit.MICROSECONDS;
2627
import static java.util.concurrent.TimeUnit.MILLISECONDS;
@@ -42,13 +43,12 @@
4243
import org.openqa.selenium.support.FindAll;
4344
import org.openqa.selenium.support.FindBy;
4445

45-
import java.util.Calendar;
4646
import java.util.List;
4747
import java.util.concurrent.TimeUnit;
4848

49-
public class TimeOutTest {
49+
public class TimeoutTest {
5050

51-
private static final long ACCEPTABLE_DELTA_MILLS = 1500;
51+
private static final long ACCEPTABLE_TIME_DIFF = 1500;
5252
private static final String MESSAGE = "Check difference from the expected waiting duration %s %s";
5353

5454
private WebDriver driver;
@@ -65,10 +65,10 @@ public class TimeOutTest {
6565

6666
private TimeOutDuration timeOutDuration;
6767

68-
private static long getBenchMark(List<WebElement> stubElements) {
69-
long startMark = Calendar.getInstance().getTimeInMillis();
70-
stubElements.size();
71-
long endMark = Calendar.getInstance().getTimeInMillis();
68+
private static long getBenchmark(Runnable runnable) {
69+
long startMark = currentTimeMillis();
70+
runnable.run();
71+
long endMark = currentTimeMillis();
7272
return endMark - startMark;
7373
}
7474

@@ -96,37 +96,37 @@ private static long getExpectedMillis(long value, TimeUnit sourceTimeUnit) {
9696

9797
@Test public void defaultTimeOutTest() {
9898
assertThat(format(MESSAGE, DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT),
99-
abs(getExpectedMillis(DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT) - getBenchMark(stubElements)),
100-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
99+
abs(getExpectedMillis(DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT) - getBenchmark(() -> stubElements.size())),
100+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
101101

102102
timeOutDuration.setTime(15500000, MICROSECONDS);
103103
assertThat(format(MESSAGE, 15500000, MICROSECONDS),
104-
abs(getExpectedMillis(15500000, MICROSECONDS) - getBenchMark(stubElements)),
105-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
104+
abs(getExpectedMillis(15500000, MICROSECONDS) - getBenchmark(() -> stubElements.size())),
105+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
106106

107107
timeOutDuration.setTime(3, SECONDS);
108108
assertThat(format(MESSAGE, 3, SECONDS),
109-
abs(getExpectedMillis(3, SECONDS) - getBenchMark(stubElements)),
110-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));;
109+
abs(getExpectedMillis(3, SECONDS) - getBenchmark(() -> stubElements.size())),
110+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
111111
}
112112

113113
@Test public void withCustomizedTimeOutTest() {
114114
assertThat(format(MESSAGE, DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT),
115-
abs(getExpectedMillis(DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT) - getBenchMark(stubElements)),
116-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
115+
abs(getExpectedMillis(DEFAULT_TIMEOUT, DEFAULT_TIMEUNIT) - getBenchmark(() -> stubElements.size())),
116+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
117117

118118
assertThat(format(MESSAGE, 5, SECONDS),
119-
abs(getExpectedMillis(5, SECONDS) - getBenchMark(stubElements2)),
120-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
119+
abs(getExpectedMillis(5, SECONDS) - getBenchmark(() -> stubElements2.size())),
120+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
121121

122122
timeOutDuration.setTime(15500000, MICROSECONDS);
123123

124124
assertThat(format(MESSAGE, 15500000, MICROSECONDS),
125-
abs(getExpectedMillis(15500000, MICROSECONDS) - getBenchMark(stubElements)),
126-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
125+
abs(getExpectedMillis(15500000, MICROSECONDS) - getBenchmark(() -> stubElements.size())),
126+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
127127

128128
assertThat(format(MESSAGE, 5, SECONDS),
129-
abs(getExpectedMillis(5, SECONDS) - getBenchMark(stubElements2)),
130-
lessThanOrEqualTo(ACCEPTABLE_DELTA_MILLS));
129+
abs(getExpectedMillis(5, SECONDS) - getBenchmark(() -> stubElements2.size())),
130+
lessThanOrEqualTo(ACCEPTABLE_TIME_DIFF));
131131
}
132132
}

0 commit comments

Comments
 (0)