Skip to content

Commit cffa6a8

Browse files
mach6lukeis
authored andcommitted
Fix StandaloneConfiguration#toString() (#2697)
The toString method was incorrectly processing Collections and Maps. This change set adds considerations for them. Also in this change set; * Updated tests for GridHubConfigurationTest * Trim the result of getting coreVersion in ConsoleServlet
1 parent 4c75ef0 commit cffa6a8

File tree

3 files changed

+56
-7
lines changed

3 files changed

+56
-7
lines changed

java/server/src/org/openqa/grid/internal/utils/configuration/StandaloneConfiguration.java

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
import com.beust.jcommander.Parameter;
2424

2525
import java.util.Arrays;
26+
import java.util.Collection;
2627
import java.util.List;
28+
import java.util.Map;
2729

2830
public class StandaloneConfiguration {
2931

@@ -135,8 +137,10 @@ public StringBuilder toString(String format, String name, Object value) {
135137
iterator = Arrays.asList(value);
136138
}
137139
for (Object v : iterator) {
138-
if (value != null) {
139-
sb.append(String.format(format, name, value));
140+
if (v != null &&
141+
!(v instanceof Map && ((Map) v).isEmpty()) &&
142+
!(v instanceof Collection && ((Collection) v).isEmpty())) {
143+
sb.append(String.format(format, name, v));
140144
}
141145
}
142146
return sb;

java/server/src/org/openqa/grid/web/servlet/beta/ConsoleServlet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ private void getVersion() {
261261
}
262262
try {
263263
Manifest manifest = new Manifest(stream);
264-
coreVersion = manifest.getEntries().get("Build-Info").getValue("Selenium-Version");
264+
coreVersion = manifest.getEntries().get("Build-Info").getValue("Selenium-Version").trim();
265265
} catch (IOException e) {
266266
log.severe("Cannot load version from VERSION.txt" + e.getMessage());
267267
}

java/server/test/org/openqa/grid/internal/utils/GridHubConfigurationTest.java

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static org.junit.Assert.assertEquals;
2121
import static org.junit.Assert.assertFalse;
2222
import static org.junit.Assert.assertTrue;
23+
import static org.junit.Assert.assertNull;
2324

2425
import com.beust.jcommander.JCommander;
2526

@@ -32,17 +33,15 @@
3233
public class GridHubConfigurationTest {
3334

3435
@Test
35-
public void testGetTimeout() throws Exception {
36+
public void testSetTimeout() throws Exception {
3637
GridHubConfiguration gridHubConfiguration = new GridHubConfiguration();
37-
assertEquals(1800, gridHubConfiguration.timeout.longValue()); // From the configuration default value
3838
gridHubConfiguration.timeout = 123;
3939
assertEquals(123, gridHubConfiguration.timeout.longValue());
4040
}
4141

4242
@Test
43-
public void testGetBrowserTimeout() throws Exception {
43+
public void testSetBrowserTimeout() throws Exception {
4444
GridHubConfiguration gridHubConfiguration = new GridHubConfiguration();
45-
assertEquals(0, gridHubConfiguration.browserTimeout.longValue());// From DefaultHub.json file
4645
gridHubConfiguration.browserTimeout = 1233;
4746
assertEquals(1233, gridHubConfiguration.browserTimeout.longValue());
4847
}
@@ -65,4 +64,50 @@ public void testWithOutServlets() {
6564
ghc.withoutServlets.add(ResourceServlet.class.getCanonicalName());
6665
assertTrue(ghc.isWithOutServlet(ResourceServlet.class));
6766
}
67+
68+
@Test
69+
public void testDefaults() {
70+
GridHubConfiguration ghc = new GridHubConfiguration();
71+
assertEquals("standalone", ghc.role);
72+
assertEquals(0L, ghc.browserTimeout.longValue());
73+
assertEquals(false, ghc.debug);
74+
assertEquals(false, ghc.help);
75+
assertEquals(false, ghc.logLongForm);
76+
assertEquals(1800L, ghc.timeout.longValue());
77+
assertEquals(5000L, ghc.cleanUpCycle.longValue());
78+
assertEquals(1L, ghc.maxSession.longValue());
79+
assertEquals(-1L, ghc.jettyMaxThreads.longValue());
80+
assertEquals("org.openqa.grid.internal.utils.DefaultCapabilityMatcher",
81+
ghc.capabilityMatcher.getClass().getCanonicalName());
82+
assertEquals(-1L, ghc.newSessionWaitTimeout.longValue());
83+
assertEquals(true, ghc.throwOnCapabilityNotPresent);
84+
assertTrue(ghc.servlets.isEmpty());
85+
assertTrue(ghc.custom.isEmpty());
86+
assertNull(ghc.withoutServlets);
87+
assertNull(ghc.hubConfig);
88+
assertNull(ghc.log);
89+
assertNull(ghc.prioritizer);
90+
assertNull(ghc.host);
91+
assertNull(ghc.port);
92+
}
93+
94+
@Test
95+
public void testToString() {
96+
GridHubConfiguration ghc = new GridHubConfiguration();
97+
98+
assertTrue(ghc.toString().contains("-role standalone "));
99+
assertFalse(ghc.toString().contains("-servlets"));
100+
assertFalse(ghc.toString().contains("custom"));
101+
102+
ghc = new GridHubConfiguration();
103+
String[] args = ("-servlet com.foo.bar.ServletA -servlet com.foo.bar.ServletB"
104+
+ " -custom foo=bar,bar=baz").split(" ");
105+
new JCommander(ghc, args);
106+
107+
assertTrue(ghc.toString().contains("-servlets com.foo.bar.ServletA"
108+
+ " -servlets com.foo.bar.ServletB"));
109+
assertTrue(ghc.toString().contains("custom {"));
110+
assertTrue(ghc.toString().contains("bar=baz"));
111+
assertTrue(ghc.toString().contains("foo=bar"));
112+
}
68113
}

0 commit comments

Comments
 (0)