Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[java] minor performance improvements and code cleanup #14054

Merged
merged 21 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
73f3d78
replaced empty string comparison with isEmpty() invoking
iampopovich May 29, 2024
0935a65
replaced manual array copy with System.arraycopy()
iampopovich May 29, 2024
3c8a520
replaced redundant String.format invoking with printf()
iampopovich May 29, 2024
2a9537f
replaced deprecated setScriptTimeout with scriptTimeout
iampopovich May 29, 2024
674e764
replaced iterators with bulk methods inoking
iampopovich May 30, 2024
1181d73
replaced list creations with List.of()
iampopovich May 30, 2024
fbf048a
Merge branch 'trunk' into performance-improvements
iampopovich May 30, 2024
bae701e
simplifying statement according to pr recommendation
iampopovich May 30, 2024
5cb6284
applying format.sh
iampopovich May 30, 2024
4829d74
Merge branch 'trunk' into performance-improvements
iampopovich Jun 4, 2024
0456bfa
Merge branch 'trunk' into performance-improvements
pujagani Jun 5, 2024
dd989b6
trying to fix test
iampopovich Jun 5, 2024
47f4fe1
handle case when args is null
iampopovich Jun 5, 2024
12faf15
Merge branch 'trunk' into performance-improvements
iampopovich Jun 5, 2024
132aead
applying format.sh
iampopovich Jun 5, 2024
21adbdd
Merge branch 'trunk' into performance-improvements
iampopovich Jun 5, 2024
1864028
Merge branch 'trunk' into performance-improvements
iampopovich Jun 7, 2024
15962c4
Merge branch 'trunk' into performance-improvements
iampopovich Jun 11, 2024
b9452c6
Merge branch 'trunk' into performance-improvements
iampopovich Jun 15, 2024
27ca016
Merge branch 'trunk' into performance-improvements
iampopovich Jun 19, 2024
0dd4c1e
Merge branch 'trunk' into performance-improvements
iampopovich Jun 20, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions java/src/org/openqa/selenium/Cookie.java
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public Cookie(
String sameSite) {
this.name = name;
this.value = value;
this.path = path == null || "".equals(path) ? "/" : path;
this.path = path == null || path.isEmpty() ? "/" : path;

this.domain = stripPort(domain);
this.isSecure = isSecure;
Expand Down Expand Up @@ -203,7 +203,7 @@ private static String stripPort(String domain) {
}

public void validate() {
if (name == null || "".equals(name) || value == null || path == null) {
if (name == null || name.isEmpty() || value == null || path == null) {
throw new IllegalArgumentException(
"Required attributes are not set or " + "any non-null attribute set to null");
}
Expand Down
3 changes: 1 addition & 2 deletions java/src/org/openqa/selenium/chromium/ChromiumOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,7 @@ protected Object getExtraCapability(String capabilityName) {
return null;
}

Map<String, Object> options = new TreeMap<>();
experimentalOptions.forEach(options::put);
Map<String, Object> options = new TreeMap<>(experimentalOptions);

if (binary != null) {
options.put("binary", binary);
Expand Down
2 changes: 1 addition & 1 deletion java/src/org/openqa/selenium/firefox/FileExtension.java
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public Iterator<String> getPrefixes(String uri) {
id = idNode.getTextContent();
}

if (id == null || "".equals(id.trim())) {
if (id == null || id.trim().isEmpty()) {
throw new FileNotFoundException("Cannot install extension with ID: " + id);
}
return id;
Expand Down
21 changes: 9 additions & 12 deletions java/src/org/openqa/selenium/grid/commands/CompletionCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,8 @@ private void outputZshCompletions(PrintStream out) {
.sorted(Comparator.comparing(CliCommand::getName))
.forEach(
cmd -> {
out.println(
String.format(
" '%s:%s'",
cmd.getName(), cmd.getDescription().replace("'", "'\\''")));
out.printf(
" '%s:%s'%n", cmd.getName(), cmd.getDescription().replace("'", "'\\''"));
});

out.println(" )");
Expand All @@ -143,8 +141,8 @@ private void outputZshCompletions(PrintStream out) {
.forEach(
cmd -> {
String shellName = cmd.getName().replace('-', '_');
out.println(String.format(" (%s)", cmd.getName()));
out.println(String.format(" _selenium_%s", shellName));
out.printf(" (%s)%n", cmd.getName());
out.printf(" _selenium_%s%n", shellName);
out.println(" ;;");
});

Expand All @@ -155,7 +153,7 @@ private void outputZshCompletions(PrintStream out) {

allCommands.forEach(
(cmd, options) -> {
out.println(String.format("_selenium_%s() {", cmd.getName().replace('-', '_')));
out.printf("_selenium_%s() {%n", cmd.getName().replace('-', '_'));
out.println(" args=(");

options.stream()
Expand All @@ -170,17 +168,16 @@ private void outputZshCompletions(PrintStream out) {
}

if (opt.flags().size() == 1) {
out.println(
String.format(
" '%s[%s]%s'",
opt.flags().iterator().next(), quotedDesc, getZshType(opt)));
out.printf(
" '%s[%s]%s'%n",
opt.flags().iterator().next(), quotedDesc, getZshType(opt));
} else {
out.print(" '");
out.print(opt.flags.stream().collect(joining(" ", "(", ")")));
out.print("'");
out.print(opt.flags.stream().collect(joining(",", "{", "}")));
out.print("'");
out.print(String.format("[%s]", quotedDesc));
out.printf("[%s]", quotedDesc);
out.print(getZshType(opt));
out.print("'\n");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class ConcatenatingConfig implements Config {
private final Map<String, String> values;

public ConcatenatingConfig(String prefix, char separator, Map<?, ?> values) {
this.prefix = prefix == null || "".equals(prefix) ? "" : (prefix + separator);
this.prefix = prefix == null || prefix.isEmpty() ? "" : (prefix + separator);
this.separator = separator;

this.values =
Expand Down
2 changes: 1 addition & 1 deletion java/src/org/openqa/selenium/logging/LogLevelMapping.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public static String getName(Level level) {
}

public static Level toLevel(String logLevelName) {
if (logLevelName == null || "".equals(logLevelName)) {
if (logLevelName == null || logLevelName.isEmpty()) {
// Default the log level to info.
return Level.INFO;
}
Expand Down
4 changes: 1 addition & 3 deletions java/src/org/openqa/selenium/print/PrintOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,7 @@ public void setPageRanges(String firstRange, String... ranges) {

this.pageRanges[0] = firstRange;

for (int i = 1; i < ranges.length; i++) {
this.pageRanges[i] = ranges[i - 1];
}
if (ranges.length > 0) System.arraycopy(ranges, 0, this.pageRanges, 1, ranges.length - 1);
}

public void setPageRanges(List<String> ranges) {
Expand Down
4 changes: 1 addition & 3 deletions java/src/org/openqa/selenium/remote/RemoteLogs.java
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,7 @@ public Set<String> getAvailableLogTypes() {
@SuppressWarnings("unchecked")
List<String> rawList = (List<String>) raw;
Set<String> builder = new LinkedHashSet<>();
for (String logType : rawList) {
builder.add(logType);
}
builder.addAll(rawList);
builder.addAll(getAvailableLocalLogs());
return Set.copyOf(builder);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.List;
import java.util.Objects;
import java.util.logging.Level;
import java.util.logging.Logger;
import org.openqa.selenium.Alert;
Expand Down Expand Up @@ -245,14 +246,10 @@ private void fireBeforeEvents(
int argsLength = args != null ? args.length : 0;
Object[] args2 = new Object[argsLength + 1];
args2[0] = target.getOriginal();
for (int i = 0; i < argsLength; i++) {
args2[i + 1] = args[i];
}
if (args != null) System.arraycopy(args, 0, args2, 1, argsLength);

Method m = findMatchingMethod(listener, methodName, args2);
if (m != null) {
callListenerMethod(m, listener, args2);
}
if (m != null) callListenerMethod(m, listener, args2);
}

private void fireAfterEvents(
Expand All @@ -261,20 +258,15 @@ private void fireAfterEvents(

boolean isVoid =
method.getReturnType() == Void.TYPE || method.getReturnType() == WebDriver.Timeouts.class;

int argsLength = args != null ? args.length : 0;
Object[] args2 = new Object[argsLength + 1 + (isVoid ? 0 : 1)];
args2[0] = target.getOriginal();
for (int i = 0; i < argsLength; i++) {
args2[i + 1] = args[i];
}
if (!isVoid) {
args2[args2.length - 1] = res;
}
if (args != null) System.arraycopy(Objects.requireNonNull(args), 0, args2, 1, argsLength);
if (!isVoid) args2[args2.length - 1] = res;

Method m = findMatchingMethod(listener, methodName, args2);
if (m != null) {
callListenerMethod(m, listener, args2);
}
if (m != null) callListenerMethod(m, listener, args2);

try {
if (target.getOriginal() instanceof WebDriver) {
Expand Down
25 changes: 12 additions & 13 deletions java/test/org/openqa/selenium/ExecutingAsyncJavascriptTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import static org.openqa.selenium.testing.drivers.Browser.SAFARI;

import java.time.Duration;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -45,7 +44,7 @@ class ExecutingAsyncJavascriptTest extends JupiterTestBase {
public void setUp() {
assumeTrue(driver instanceof JavascriptExecutor);
executor = (JavascriptExecutor) driver;
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
}

@Test
Expand All @@ -56,7 +55,7 @@ public void setUp() {
public void shouldSetAndGetScriptTimeout() {
Duration timeout = driver.manage().timeouts().getScriptTimeout();
assertThat(timeout).hasMillis(30000);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(3000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(3000));
Duration timeout2 = driver.manage().timeouts().getScriptTimeout();
assertThat(timeout2).hasMillis(3000);
}
Expand Down Expand Up @@ -185,7 +184,7 @@ public void shouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout() {
@Test
@NotYetImplemented(SAFARI)
public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(500));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(500));
driver.get(pages.ajaxyPage);
assertThatExceptionOfType(ScriptTimeoutException.class)
.isThrownBy(
Expand All @@ -199,7 +198,7 @@ public void shouldTimeoutIfScriptDoesNotInvokeCallbackWithLongTimeout() {
@Ignore(IE)
public void shouldDetectPageLoadsWhileWaitingOnAnAsyncScriptAndReturnAnError() {
driver.get(pages.ajaxyPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(100));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(100));
assertThatExceptionOfType(WebDriverException.class)
.isThrownBy(
() -> executor.executeAsyncScript("window.location = '" + pages.dynamicPage + "';"));
Expand Down Expand Up @@ -244,7 +243,7 @@ public void shouldCatchErrorsWithMessageAndStacktraceWhenExecutingInitialScript(
t -> {
Throwable rootCause = getRootCause(t);
assertThat(rootCause).hasMessageContaining("errormessage");
assertThat(Arrays.asList(rootCause.getStackTrace()))
assertThat(List.of(rootCause.getStackTrace()))
.extracting(StackTraceElement::getMethodName)
.contains("functionB");
});
Expand All @@ -266,7 +265,7 @@ void shouldBeAbleToExecuteAsynchronousScripts() {
"There should only be 1 DIV at this point, which is used for the butter message")
.isEqualTo(1);

driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(15));
driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(15));
String text =
(String)
executor.executeAsyncScript(
Expand Down Expand Up @@ -317,7 +316,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
+ "xhr.send('');"; // empty string to stop firefox 3 from choking

driver.get(pages.ajaxyPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(3));
driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(3));
String response = (String) executor.executeAsyncScript(script, pages.sleepingPage + "?time=2");
assertThat(response.trim())
.isEqualTo("<html><head><title>Done</title></head><body>Slept for 2s</body></html>");
Expand All @@ -331,7 +330,7 @@ void shouldBeAbleToMakeXMLHttpRequestsAndWaitForTheResponse() {
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
public void throwsIfScriptTriggersAlert() {
driver.get(pages.simpleTestPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
assertThatExceptionOfType(UnhandledAlertException.class)
.isThrownBy(
() ->
Expand All @@ -350,7 +349,7 @@ public void throwsIfScriptTriggersAlert() {
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
public void throwsIfAlertHappensDuringScript() {
driver.get(pages.slowLoadingAlertPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
assertThatExceptionOfType(UnhandledAlertException.class)
.isThrownBy(() -> executor.executeAsyncScript("setTimeout(arguments[0], 1000);"));
// Shouldn't throw
Expand All @@ -365,7 +364,7 @@ public void throwsIfAlertHappensDuringScript() {
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
public void throwsIfScriptTriggersAlertWhichTimesOut() {
driver.get(pages.simpleTestPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
assertThatExceptionOfType(UnhandledAlertException.class)
.isThrownBy(
() ->
Expand All @@ -383,7 +382,7 @@ public void throwsIfScriptTriggersAlertWhichTimesOut() {
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
driver.get(pages.slowLoadingAlertPage);
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
assertThatExceptionOfType(UnhandledAlertException.class)
.isThrownBy(() -> executor.executeAsyncScript(""));
// Shouldn't throw
Expand All @@ -397,7 +396,7 @@ public void throwsIfAlertHappensDuringScriptWhichTimesOut() {
@Ignore(FIREFOX)
@Ignore(value = SAFARI, reason = "Does not support alerts yet")
public void includesAlertTextInUnhandledAlertException() {
driver.manage().timeouts().setScriptTimeout(Duration.ofMillis(5000));
driver.manage().timeouts().scriptTimeout(Duration.ofMillis(5000));
String alertText = "Look! An alert!";
assertThatExceptionOfType(UnhandledAlertException.class)
.isThrownBy(
Expand Down
2 changes: 1 addition & 1 deletion java/test/org/openqa/selenium/ExecutingJavascriptTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ public void testShouldThrowAnExceptionWithMessageAndStacktraceWhenTheJavascriptI
t -> {
Throwable rootCause = getRootCause(t);
assertThat(rootCause).hasMessageContaining("errormessage");
assertThat(Arrays.asList(rootCause.getStackTrace()))
assertThat(List.of(rootCause.getStackTrace()))
.extracting(StackTraceElement::getMethodName)
.contains("functionB");
});
Expand Down
2 changes: 1 addition & 1 deletion java/test/org/openqa/selenium/build/BazelBuild.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void build(String target) {
return;
}

if (target == null || "".equals(target)) {
if (target == null || target.isEmpty()) {
throw new IllegalStateException("No targets specified");
}
LOG.info("\nBuilding " + target + " ...");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.net.URISyntaxException;
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -163,7 +162,7 @@ void shouldBeAbleToDeleteTimedoutSessions() {
try {
Callable<HttpResponse> sessionCreationTask = () -> createSession(caps);
List<Future<HttpResponse>> futureList =
fixedThreadPoolService.invokeAll(Arrays.asList(sessionCreationTask));
fixedThreadPoolService.invokeAll(List.of(sessionCreationTask));

for (Future<HttpResponse> future : futureList) {
HttpResponse httpResponse = future.get(10, SECONDS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void testPersistentHoverCanBeTurnedOff() throws Exception {
// Intentionally wait to make sure hover DOES NOT persist.
Thread.sleep(1000);

wait.until(d -> item.getText().equals(""));
wait.until(d -> item.getText().isEmpty());

assertThat(item.getText()).isEmpty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,7 @@ void canHandleGetTimeoutsCommand() {
void canHandleSetScriptTimeoutCommand() {
WebDriverFixture fixture = new WebDriverFixture(echoCapabilities, nullValueResponder);

fixture.driver.manage().timeouts().setScriptTimeout(Duration.ofSeconds(10));
fixture.driver.manage().timeouts().scriptTimeout(Duration.ofSeconds(10));

fixture.verifyCommands(
new CommandPayload(DriverCommand.SET_TIMEOUT, ImmutableMap.of("script", 10000L)));
Expand Down
Loading