Skip to content

Commit 409c996

Browse files
barancevbarancev
committed
Refactoring Executable to update dynamic library path in CommandLine. The method setLibraryPath was a kind of utility method, its responsibilities are distributed now amongst FirefoxBinary and CommandLine.
1 parent 0e83280 commit 409c996

5 files changed

Lines changed: 19 additions & 54 deletions

File tree

‎java/client/src/org/openqa/selenium/firefox/FirefoxBinary.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,14 @@ public void startProfile(FirefoxProfile profile, File profileDir, String... comm
9797
cmdArray.addAll(Lists.newArrayList(commandLineFlags));
9898
CommandLine command = new CommandLine(getExecutable().getPath(), Iterables.toArray(cmdArray, String.class));
9999
command.setEnvironmentVariables(getExtraEnv());
100-
getExecutable().setLibraryPath(command, getExtraEnv());
100+
command.updateDynamicLibraryPath(getExtraEnv().get(CommandLine.getLibraryPathPropertyName()));
101+
// On Snow Leopard, beware of problems the sqlite library
102+
if (! (Platform.getCurrent().is(Platform.MAC) && Platform.getCurrent().getMinorVersion() > 5)) {
103+
String firefoxLibraryPath = System.getProperty(
104+
FirefoxDriver.SystemProperty.BROWSER_LIBRARY_PATH,
105+
getExecutable().getFile().getAbsoluteFile().getParentFile().getAbsolutePath());
106+
command.updateDynamicLibraryPath(firefoxLibraryPath);
107+
}
101108

102109
if (stream == null) {
103110
stream = getDefaultOutputStream();

‎java/client/src/org/openqa/selenium/firefox/internal/Executable.java‎

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.openqa.selenium.os.WindowsUtils;
2828

2929
import java.io.File;
30-
import java.util.Map;
3130

3231
import static org.openqa.selenium.Platform.MAC;
3332
import static org.openqa.selenium.Platform.UNIX;
@@ -81,35 +80,6 @@ public String getPath() {
8180
return binary.getAbsolutePath();
8281
}
8382

84-
public void setLibraryPath(CommandLine command, final Map<String, String> extraEnv) {
85-
final String propertyName = CommandLine.getLibraryPathPropertyName();
86-
StringBuilder libraryPath = new StringBuilder();
87-
88-
// If we have an env var set for the path, use it.
89-
String env = System.getenv(propertyName);
90-
if (env != null) {
91-
libraryPath.append(env).append(File.pathSeparator);
92-
}
93-
94-
// Check our extra env vars for the same var, and use it too.
95-
env = extraEnv.get(propertyName);
96-
if (env != null) {
97-
libraryPath.append(env).append(File.pathSeparator);
98-
}
99-
100-
// Last, add the contents of the specified system property, defaulting to the binary's path.
101-
102-
// On Snow Leopard, beware of problems the sqlite library
103-
String firefoxLibraryPath = System.getProperty(FirefoxDriver.SystemProperty.BROWSER_LIBRARY_PATH,
104-
binary.getAbsoluteFile().getParentFile().getAbsolutePath());
105-
if (! (Platform.getCurrent().is(Platform.MAC) && Platform.getCurrent().getMinorVersion() > 5)) {
106-
libraryPath.append(firefoxLibraryPath);
107-
}
108-
109-
// Add the library path to the builder.
110-
command.setEnvironmentVariable(propertyName, libraryPath.toString());
111-
}
112-
11383
/**
11484
* Locates the firefox binary from a system property. Will throw an exception if the binary cannot
11585
* be found.

‎java/client/src/org/openqa/selenium/os/CommandLine.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ public void setDynamicLibraryPath(String newLibraryPath) {
8282
}
8383
}
8484

85+
public void updateDynamicLibraryPath(String extraPath) {
86+
if (extraPath != null) {
87+
String existing = System.getenv(getLibraryPathPropertyName());
88+
String ldPath = existing != null ? existing + File.separator + extraPath : extraPath;
89+
setEnvironmentVariable(getLibraryPathPropertyName(), ldPath);
90+
}
91+
}
92+
8593
/**
8694
* @return The platform specific env property name which contains the library path.
8795
*/

‎java/client/test/org/openqa/selenium/BUCK‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ java_library(name = 'tests',
7676
':helpers',
7777
'//java/client/src/org/openqa/selenium:selenium',
7878
'//java/client/src/org/openqa/selenium/ie:ie',
79-
'//java/client/src/org/openqa/selenium/firefox:firefox',
8079
'//java/client/src/org/openqa/selenium/remote:remote',
8180
'//java/client/src/org/openqa/selenium/support:support',
8281
'//java/client/src/org/openqa/selenium/support/ui:wait',

‎java/client/test/org/openqa/selenium/os/CommandLineTest.java‎

Lines changed: 3 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertFalse;
2222

23-
import com.google.common.collect.ImmutableMap;
2423
import com.google.common.collect.Maps;
2524

2625
import org.junit.Assume;
@@ -29,9 +28,7 @@
2928
import org.junit.runner.RunWith;
3029
import org.junit.runners.JUnit4;
3130
import org.openqa.selenium.Platform;
32-
import org.openqa.selenium.firefox.internal.Executable;
3331

34-
import java.io.File;
3532
import java.util.Map;
3633

3734
@RunWith(JUnit4.class)
@@ -141,27 +138,11 @@ public void testDestroy() {
141138
}
142139

143140
@Test
144-
public void canSetLibraryPathFromExecutable() {
141+
public void canUpdateLibraryPath() {
145142
Assume.assumeTrue(Platform.getCurrent().is(Platform.WINDOWS));
146143
CommandLine commandLine = new CommandLine(testExecutable);
147-
Executable executable = new Executable(new File("C:\\windows\\system32\\ping.exe"));
148-
Map<String, String> extraEnv = new ImmutableMap.Builder<String, String>()
149-
.build();
150-
executable.setLibraryPath(commandLine, extraEnv);
151-
assertEquals(String.format("%s;%s", System.getenv("PATH"), "C:\\windows\\system32"),
152-
commandLine.getEnvironment().get(CommandLine.getLibraryPathPropertyName()));
153-
}
154-
155-
@Test
156-
public void canUpdateLibraryPathFromExtraEnv() {
157-
Assume.assumeTrue(Platform.getCurrent().is(Platform.WINDOWS));
158-
CommandLine commandLine = new CommandLine(testExecutable);
159-
Executable executable = new Executable(new File("C:\\windows\\system32\\ping.exe"));
160-
Map<String, String> extraEnv = new ImmutableMap.Builder<String, String>()
161-
.put("PATH", "C:\\My\\Tools")
162-
.build();
163-
executable.setLibraryPath(commandLine, extraEnv);
164-
assertEquals(String.format("%s;%s;%s", System.getenv("PATH"), "C:\\My\\Tools", "C:\\windows\\system32"),
144+
commandLine.updateDynamicLibraryPath("C:\\My\\Tools");
145+
assertEquals(String.format("%s;%s", System.getenv("PATH"), "C:\\My\\Tools"),
165146
commandLine.getEnvironment().get(CommandLine.getLibraryPathPropertyName()));
166147
}
167148
}

0 commit comments

Comments
 (0)