Skip to content

Commit

Permalink
More tests and tweaks for RegistrationRequest
Browse files Browse the repository at this point in the history
  • Loading branch information
@mach6
mach6 committed Nov 17, 2016
1 parent 9f2c118 commit c0a3b39
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 59 deletions.
85 changes: 50 additions & 35 deletions java/server/src/org/openqa/grid/common/RegistrationRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,16 +65,22 @@ public RegistrationRequest() {

/**
* Create a new registration request using the supplied {@link GridNodeConfiguration}
* @param configuration the {@link GridNodeConfiguration} to use
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
*/
public RegistrationRequest(GridNodeConfiguration configuration) {
this(configuration, null, null);
}

/**
* Create a new registration request using the supplied {@link GridNodeConfiguration}, and name
* @param configuration the {@link GridNodeConfiguration} to use
* @param name the name for the remote
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
* @param name the name for the remote
*/
public RegistrationRequest(GridNodeConfiguration configuration, String name) {
this(configuration, name, null);
Expand All @@ -83,12 +89,15 @@ public RegistrationRequest(GridNodeConfiguration configuration, String name) {
/**
* Create a new registration request using the supplied {@link GridNodeConfiguration}, name, and
* description
* @param configuration the {@link GridNodeConfiguration} to use
* @param name the name for the remote
* @param description the description for the remote host
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
* @param name the name for the remote
* @param description the description for the remote host
*/
public RegistrationRequest(GridNodeConfiguration configuration, String name, String description) {
this.configuration = configuration;
this.configuration = (configuration == null) ? new GridNodeConfiguration() : configuration;
this.name = name;
this.description = description;

Expand Down Expand Up @@ -155,55 +164,57 @@ public static RegistrationRequest fromJson(String json) throws JsonSyntaxExcepti
}

/**
* Build a RegistrationRequest. This is different than {@code new RegistrationRequest()} because
* it will "fixup" the resulting RegistrationRequest before returning the result
* Build a RegistrationRequest.
* @return
*/
public static RegistrationRequest build() {
return RegistrationRequest.build(null, null, null);
return RegistrationRequest.build(new GridNodeConfiguration(), null, null);
}

/**
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}. This is different
* than {@code new RegistrationRequest(GridNodeConfiguration)} because it will merge any
* specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided configuration and it
* will "fixup" the resulting RegistrationRequest before returning the result
* @param configuration the {@link GridNodeConfiguration} to use
* @return
* than {@code new RegistrationRequest(GridNodeConfiguration)} because it will first load any
* specified {@link GridNodeConfiguration#nodeConfigFile} and then merge the provided
* configuration onto it.
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
*/
public static RegistrationRequest build(GridNodeConfiguration configuration) {
return RegistrationRequest.build(configuration, null, null);
}

/**
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided name.
* This is different than {@code new RegistrationRequest(GridNodeConfiguration, String)} because it
* will merge any specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided
* configuration and it will "fixup" the resulting RegistrationRequest before returning the result
* @param configuration the {@link GridNodeConfiguration} to use
* @param name the name for the remote
* @return
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided
* name. This is different than {@code new RegistrationRequest(GridNodeConfiguration, String)}
* because it will first load any specified {@link GridNodeConfiguration#nodeConfigFile} and then
* merge the provided configuration onto it.
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
* @param name the name for the remote
*/
public static RegistrationRequest build(GridNodeConfiguration configuration, String name) {
return RegistrationRequest.build(configuration, name, null);
}

/**
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided name
* and description. This is different than
* {@code new RegistrationRequest(GridNodeConfiguration, String, String)} because it will merge any
* specified {@link GridNodeConfiguration#nodeConfigFile} onto the provided configuration and it
* will "fixup" the resulting RegistrationRequest before returning the result
* @param configuration the {@link GridNodeConfiguration} to use
* @param name the name for the remote
* @param description the description for the remote host
* @return
* Build a RegistrationRequest from the provided {@link GridNodeConfiguration}, use the provided
* name and description. This is different than {@code new RegistrationRequest(GridNodeConfiguration,
* String, String)} because it will first load any specified {@link
* GridNodeConfiguration#nodeConfigFile} and then merge the provided configuration onto it.
*
* @param configuration the {@link GridNodeConfiguration} to use. Internally calls {@code new
* GridNodeConfiguration()} if a {@code null} value is provided since a
* request without configuration is not valid.
* @param name the name for the remote
* @param description the description for the remote host
*/
public static RegistrationRequest build(GridNodeConfiguration configuration, String name, String description) {
GridNodeConfiguration pendingConfiguration = (configuration == null) ?
new GridNodeConfiguration() : configuration;

RegistrationRequest pendingRequest = new RegistrationRequest(pendingConfiguration, name, description);
RegistrationRequest pendingRequest = new RegistrationRequest(configuration, name, description);
GridNodeConfiguration pendingConfiguration = pendingRequest.configuration;

if (pendingConfiguration.nodeConfigFile != null) {
pendingRequest.configuration = GridNodeConfiguration.loadFromJSON(pendingConfiguration.nodeConfigFile);
Expand All @@ -227,6 +238,10 @@ public static RegistrationRequest build(GridNodeConfiguration configuration, Str
}

private void fixUpCapabilities() {
if (configuration.capabilities == null) {
return; // assumes the caller set it/wants it this way
}

Platform current = Platform.getCurrent();
for (DesiredCapabilities cap : configuration.capabilities) {
if (cap.getPlatform() == null) {
Expand Down
130 changes: 106 additions & 24 deletions java/server/test/org/openqa/grid/common/RegistrationRequestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertNotSame;

import com.beust.jcommander.JCommander;

Expand Down Expand Up @@ -151,44 +153,124 @@ public void validateWithException() {
*/
@Test
public void testBuildWithConfiguration() {
GridNodeConfiguration config = new GridNodeConfiguration();
config.maxSession = 50;
config.timeout = 10;
GridNodeConfiguration actualConfig = new GridNodeConfiguration();
actualConfig.maxSession = 50;
actualConfig.timeout = 10;
actualConfig.host = "dummyhost";
actualConfig.port = 1234;
actualConfig.capabilities.set(0, DesiredCapabilities.operaBlink());
actualConfig.nodeConfigFile = GridNodeConfiguration.DEFAULT_NODE_CONFIG_FILE;

RegistrationRequest req = RegistrationRequest.build(actualConfig);
assertConstruction(req);

// we should get a new config object ref back, since the nodeConfigFile was processed
assertNotSame(req.getConfiguration(), actualConfig);
// reset to new config which build() produced
actualConfig = req.getConfiguration();

// make sure the nodeConfigFile was processed by ensuring that it now is null;
assertNull(actualConfig.nodeConfigFile);

// make sure the first capability is for operaBlink
assertEquals(DesiredCapabilities.operaBlink().getBrowserName(),
actualConfig.capabilities.get(0).getBrowserName());

// make sure this merge protected value was preserved, then remove it for the final assert
assertEquals("dummyhost", actualConfig.host);
actualConfig.host = null;
// make sure this merge protected value was preserved, then reset it for the final assert
assertEquals(1234, actualConfig.port.intValue());
actualConfig.port = 5555;

// merge actualConfig onto it.. which is what build(config) should do
GridNodeConfiguration expectedConfig = new GridNodeConfiguration();
expectedConfig.merge(actualConfig);

RegistrationRequest req = RegistrationRequest.build(config);
assertEquals(expectedConfig.toString(), actualConfig.toString());
}

// should have the default capabilities
assertEquals(3, req.getConfiguration().capabilities.size());
/**
* Tests that new RegistrationRequest(config) performs as expected
*/
@Test
public void testConstructorWithConfiguration() {
GridNodeConfiguration actualConfig = new GridNodeConfiguration();
actualConfig.host = "dummyhost";
RegistrationRequest req = new RegistrationRequest(actualConfig);
assertConstruction(req);
// make sure the provided config was used.
assertSame(req.getConfiguration(), actualConfig);
}

// host is "fixed up" by the .build(config) call
// verify this happened and remove it for the final assert.
assertNotNull(req.getConfiguration().host);
req.getConfiguration().host = null;
/**
* Tests that RegistrationRequest.build() performs as expected
*/
@Test
public void testBuild() {
assertConstruction(RegistrationRequest.build());
}

// capabilities are seeded from the default node config and "fixed up" by the .build(config)
// call. They should now contain a "platform".
// verify this and remove them for the final assert
assertTrue(req.getConfiguration().capabilities.size() > 0);
for (DesiredCapabilities capabilities : req.getConfiguration().capabilities) {
assertNotNull(capabilities.getPlatform());
assertNotNull(capabilities.getCapability("seleniumProtocol"));
}
/**
* Tests that RegistrationRequest.build(null) performs as expected
*/
@Test
public void testBuildWithNullConfiguration() {
assertConstruction(RegistrationRequest.build(null));
}

GridNodeConfiguration expectedConfig = new GridNodeConfiguration();
expectedConfig.merge(config);
/**
* Tests that new RegistrationRequest() performs as expected
*/
@Test
public void testNoArgConstructor() {
assertConstruction(new RegistrationRequest());
}

assertEquals(expectedConfig.toString(), req.getConfiguration().toString());
/**
* Tests that new RegistrationRequest(null) performs as expected
*/
@Test
public void testConstructorWithNullConfiguration() {
assertConstruction(new RegistrationRequest(null));
}

/**
* Tests that RegistrationRequest.build() performs as expected
* Should not result in any NPE during the internal call to fixUpCapabilities
*/
@Test
public void testBuild() {
RegistrationRequest req = RegistrationRequest.build();
public void testConstructorWithConfigurationAndNullCapabilities() {
GridNodeConfiguration config = new GridNodeConfiguration();
config.capabilities = null;
RegistrationRequest req = new RegistrationRequest(config);
assertNull(req.getConfiguration().capabilities);
}

/**
* Should not result in any NPE during the internal call to fixUpCapabilities
*/
@Test
public void testBuildWithConfigurationAndNullCapabilities() {
GridNodeConfiguration config = new GridNodeConfiguration();
config.capabilities = null;
RegistrationRequest req = RegistrationRequest.build(config);
assertNull(req.getConfiguration().capabilities);
}

private void assertConstruction(RegistrationRequest req) {
assertNotNull(req);
assertNotNull(req.getConfiguration());
assertNull(req.getName());
assertNull(req.getDescription());
// fixUpHost should have been internally called
assertNotNull(req.getConfiguration().host);
assertNotNull(req.getConfiguration().capabilities);
// should have the default capabilities
// fixUpCapabilities should have been internally called
assertEquals(3, req.getConfiguration().capabilities.size());
for (DesiredCapabilities capabilities : req.getConfiguration().capabilities) {
assertNotNull(capabilities.getPlatform());
assertNotNull(capabilities.getCapability("seleniumProtocol"));
}
}
}

0 comments on commit c0a3b39

Please sign in to comment.