Skip to content

Commit

Permalink
Fix failing tests by checking correct type assumptions
Browse files Browse the repository at this point in the history
  • Loading branch information
@shs96c
shs96c committed May 8, 2018
1 parent 63ce4f2 commit 3bc6af4
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 64 deletions.
12 changes: 6 additions & 6 deletions java/server/src/org/openqa/grid/internal/utils/configuration/GridConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,23 +80,23 @@ public void merge(GridConfiguration other) {
}
super.merge(other);

if (isMergeAble(other.cleanUpCycle, cleanUpCycle)) {
if (isMergeAble(Integer.class, other.cleanUpCycle, cleanUpCycle)) {
cleanUpCycle = other.cleanUpCycle;
}
if (isMergeAble(other.custom, custom)) {
if (isMergeAble(Map.class, other.custom, custom)) {
if (custom == null) {
custom = new HashMap<>();
}
custom.putAll(other.custom);
}
if (isMergeAble(other.maxSession, maxSession) &&
other.maxSession.intValue() > 0) {
if (isMergeAble(Integer.class, other.maxSession, maxSession) &&
other.maxSession > 0) {
maxSession = other.maxSession;
}
if (isMergeAble(other.servlets, servlets)) {
if (isMergeAble(List.class, other.servlets, servlets)) {
servlets = other.servlets;
}
if (isMergeAble(other.withoutServlets, withoutServlets)) {
if (isMergeAble(List.class, other.withoutServlets, withoutServlets)) {
withoutServlets = other.withoutServlets;
}
}
Expand Down
10 changes: 5 additions & 5 deletions java/server/src/org/openqa/grid/internal/utils/configuration/GridHubConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,19 +136,19 @@ public void merge(GridHubConfiguration other) {
}
super.merge(other);

if (isMergeAble(other.capabilityMatcher, capabilityMatcher)) {
if (isMergeAble(CapabilityMatcher.class, other.capabilityMatcher, capabilityMatcher)) {
capabilityMatcher = other.capabilityMatcher;
}
if (isMergeAble(other.newSessionWaitTimeout, newSessionWaitTimeout)) {
if (isMergeAble(Integer.class, other.newSessionWaitTimeout, newSessionWaitTimeout)) {
newSessionWaitTimeout = other.newSessionWaitTimeout;
}
if (isMergeAble(other.prioritizer, prioritizer)) {
if (isMergeAble(Prioritizer.class, other.prioritizer, prioritizer)) {
prioritizer = other.prioritizer;
}
if (isMergeAble(other.throwOnCapabilityNotPresent, throwOnCapabilityNotPresent)) {
if (isMergeAble(Boolean.class, other.throwOnCapabilityNotPresent, throwOnCapabilityNotPresent)) {
throwOnCapabilityNotPresent = other.throwOnCapabilityNotPresent;
}
if (isMergeAble(other.registry, registry)) {
if (isMergeAble(String.class, other.registry, registry)) {
registry = other.registry;
}
}
Expand Down
26 changes: 13 additions & 13 deletions java/server/src/org/openqa/grid/internal/utils/configuration/GridNodeConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -316,43 +316,43 @@ public void merge(GridNodeConfiguration other) {
}
super.merge(other);

if (isMergeAble(other.capabilities, capabilities)) {
if (isMergeAble(List.class, other.capabilities, capabilities)) {
capabilities = other.capabilities;
}
if (isMergeAble(other.downPollingLimit, downPollingLimit)) {
if (isMergeAble(Integer.class, other.downPollingLimit, downPollingLimit)) {
downPollingLimit = other.downPollingLimit;
}
if (isMergeAble(other.hub, hub)) {
if (isMergeAble(String.class, other.hub, hub)) {
hub = other.hub;
}
if (isMergeAble(other.hubHost, hubHost)) {
if (isMergeAble(String.class, other.hubHost, hubHost)) {
hubHost = other.hubHost;
}
if (isMergeAble(other.hubPort, hubPort)) {
if (isMergeAble(Integer.class, other.hubPort, hubPort)) {
hubPort = other.hubPort;
}
if (isMergeAble(other.id, id)) {
if (isMergeAble(String.class, other.id, id)) {
id = other.id;
}
if (isMergeAble(other.nodePolling, nodePolling)) {
if (isMergeAble(Integer.class, other.nodePolling, nodePolling)) {
nodePolling = other.nodePolling;
}
if (isMergeAble(other.nodeStatusCheckTimeout, nodeStatusCheckTimeout)) {
if (isMergeAble(Integer.class, other.nodeStatusCheckTimeout, nodeStatusCheckTimeout)) {
nodeStatusCheckTimeout = other.nodeStatusCheckTimeout;
}
if (isMergeAble(other.proxy, proxy)) {
if (isMergeAble(String.class, other.proxy, proxy)) {
proxy = other.proxy;
}
if (isMergeAble(other.register, register)) {
if (isMergeAble(Boolean.class, other.register, register)) {
register = other.register;
}
if (isMergeAble(other.registerCycle, registerCycle)) {
if (isMergeAble(Integer.class, other.registerCycle, registerCycle)) {
registerCycle = other.registerCycle;
}
if (isMergeAble(other.remoteHost, remoteHost)) {
if (isMergeAble(String.class, other.remoteHost, remoteHost)) {
remoteHost = other.remoteHost;
}
if (isMergeAble(other.unregisterIfStillDownAfter, unregisterIfStillDownAfter)) {
if (isMergeAble(Integer.class, other.unregisterIfStillDownAfter, unregisterIfStillDownAfter)) {
unregisterIfStillDownAfter = other.unregisterIfStillDownAfter;
}

Expand Down
20 changes: 10 additions & 10 deletions java/server/src/org/openqa/grid/internal/utils/configuration/StandaloneConfiguration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,13 @@ public void merge(StandaloneConfiguration other) {
return;
}

if (isMergeAble(other.browserTimeout, browserTimeout)) {
if (isMergeAble(Integer.class, other.browserTimeout, browserTimeout)) {
browserTimeout = other.browserTimeout;
}
if (isMergeAble(other.jettyMaxThreads, jettyMaxThreads)) {
if (isMergeAble(Integer.class, other.jettyMaxThreads, jettyMaxThreads)) {
jettyMaxThreads = other.jettyMaxThreads;
}
if (isMergeAble(other.timeout, timeout)) {
if (isMergeAble(Integer.class, other.timeout, timeout)) {
timeout = other.timeout;
}
// role, host, port, log, debug, version, enablePassThrough, and help are not merged,
Expand All @@ -209,11 +209,12 @@ public void merge(StandaloneConfiguration other) {
* Determines if one object can be merged onto another object. Checks for {@code null},
* and empty (Collections & Maps) to make decision.
*
* @param other the object to merge. must be the same type as the 'target'
* @param target the object to merge on to. must be the same type as the 'other'
* @return whether the 'other' can be merged onto the 'target'
* @param targetType The type that both {@code other} and {@code target} must be assignable to.
* @param other the object to merge. must be the same type as the 'target'.
* @param target the object to merge on to. must be the same type as the 'other'.
* @return whether the 'other' can be merged onto the 'target'.
*/
protected boolean isMergeAble(Object other, Object target) {
protected boolean isMergeAble(Class<?> targetType, Object other, Object target) {
// don't merge a null value
if (other == null) {
return false;
Expand All @@ -226,9 +227,8 @@ protected boolean isMergeAble(Object other, Object target) {

// we know we have two objects with value.. Make sure the types are the same and
// perform additional checks.

if (! target.getClass().getSuperclass().getTypeName()
.equals(other.getClass().getSuperclass().getTypeName())) {
if (!targetType.isAssignableFrom(target.getClass()) ||
!targetType.isAssignableFrom(other.getClass())) {
return false;
}

Expand Down
62 changes: 32 additions & 30 deletions ...server/test/org/openqa/grid/internal/utils/configuration/StandaloneConfigurationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class StandaloneConfigurationTest {
Expand Down Expand Up @@ -91,57 +92,59 @@ public void testIsMergeAble() {
StandaloneConfiguration sc = new StandaloneConfiguration();

// can't merge null onto null
assertFalse(sc.isMergeAble(null, null));
assertFalse(sc.isMergeAble(String.class,null, null));

// test with Character
assertTrue(sc.isMergeAble('a','a'));
assertTrue(sc.isMergeAble('a', 'b'));
assertTrue(sc.isMergeAble('a', null));
assertFalse(sc.isMergeAble(null, 'b'));
assertTrue(sc.isMergeAble(Character.class,'a','a'));
assertTrue(sc.isMergeAble(Character.class, 'a', 'b'));
assertTrue(sc.isMergeAble(Character.class, 'a', null));
assertFalse(sc.isMergeAble(Character.class, null, 'b'));

// test with Integer
assertTrue(sc.isMergeAble(1, 1));
assertTrue(sc.isMergeAble(1, 2));
assertTrue(sc.isMergeAble(1, null));
assertFalse(sc.isMergeAble(null, 2));
assertTrue(sc.isMergeAble(Integer.class,1, 1));
assertTrue(sc.isMergeAble(Integer.class,1, 2));
assertTrue(sc.isMergeAble(Integer.class,1, null));
assertFalse(sc.isMergeAble(Integer.class,null, 2));

// test with Boolean
assertTrue(sc.isMergeAble(true, true));
assertTrue(sc.isMergeAble(true, false));
assertTrue(sc.isMergeAble(true, null));
assertFalse(sc.isMergeAble(null, false));
assertTrue(sc.isMergeAble(Boolean.class, true, true));
assertTrue(sc.isMergeAble(Boolean.class, true, false));
assertTrue(sc.isMergeAble(Boolean.class, true, null));
assertFalse(sc.isMergeAble(Boolean.class, null, false));

// test with String
assertTrue(sc.isMergeAble("a", "a"));
assertTrue(sc.isMergeAble("a", "b"));
assertTrue(sc.isMergeAble("a", null));
assertFalse(sc.isMergeAble(null, "b"));
assertTrue(sc.isMergeAble(String.class, "a", "a"));
assertTrue(sc.isMergeAble(String.class, "a", "b"));
assertTrue(sc.isMergeAble(String.class, "a", null));
assertFalse(sc.isMergeAble(String.class, null, "b"));

// test with Collections
assertTrue(sc.isMergeAble(Arrays.asList("a", "b"),
assertTrue(sc.isMergeAble(List.class,
Arrays.asList("a", "b"),
Arrays.asList("b", "c")));
assertTrue(sc.isMergeAble(Arrays.asList("a", "b"),
assertTrue(sc.isMergeAble(List.class,
Arrays.asList("a", "b"),
Arrays.asList("a", "b")));
assertTrue(sc.isMergeAble(Arrays.asList("b", "c"), Collections.emptyList()));
assertTrue(sc.isMergeAble(Arrays.asList("b", "c"), null));
assertFalse(sc.isMergeAble(Collections.emptyList(), Arrays.asList("b", "c")));
assertFalse(sc.isMergeAble(null, Arrays.asList("b", "c")));
assertTrue(sc.isMergeAble(List.class, Arrays.asList("b", "c"), Collections.emptyList()));
assertTrue(sc.isMergeAble(List.class, Arrays.asList("b", "c"), null));
assertFalse(sc.isMergeAble(List.class, Collections.emptyList(), Arrays.asList("b", "c")));
assertFalse(sc.isMergeAble(List.class, null, Arrays.asList("b", "c")));

// test with Maps
Map<String, Integer> map = new ImmutableMap.Builder<String, Integer>()
.put("one", 1).put("two", 2).build();
Map<String, Integer> map2 = new ImmutableMap.Builder<String, Integer>()
.put("three", 3).put("four", 4).build();
assertTrue(sc.isMergeAble(map, map));
assertTrue(sc.isMergeAble(map, map2));
assertTrue(sc.isMergeAble(map, null));
assertTrue(sc.isMergeAble(Map.class, map, map));
assertTrue(sc.isMergeAble(Map.class, map, map2));
assertTrue(sc.isMergeAble(Map.class, map, null));

Map<String, Integer> map3 = new HashMap<>();
map3.put("five", 5);
assertTrue(sc.isMergeAble(map3, new HashMap<>()));
assertTrue(sc.isMergeAble(Map.class, map3, new HashMap<>()));

assertFalse(sc.isMergeAble(new ImmutableMap.Builder<String, Integer>().build(), map3));
assertFalse(sc.isMergeAble(null, map3));
assertFalse(sc.isMergeAble(Map.class, new ImmutableMap.Builder<String, Integer>().build(), map3));
assertFalse(sc.isMergeAble(Map.class, null, map3));
}

@Test
Expand All @@ -166,5 +169,4 @@ public void testMergeWithRealValues() {
assertNotEquals(other.log, sc.log);
assertNotEquals(other.role, sc.role);
}

}

0 comments on commit 3bc6af4

Please sign in to comment.