Skip to content

Commit

Permalink
Ignoring configurations that does not match current platform. Also, a…
Browse files Browse the repository at this point in the history 
…pparently, node does not read configuration from json resource, it is defined in the code.
  • Loading branch information
@barancev
barancev committed Nov 22, 2017
1 parent 4ce74eb commit dcb4792
Showing 1 changed file with 21 additions and 10 deletions.
31 changes: 21 additions & 10 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 @@ -51,7 +51,9 @@
import java.net.URL;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;

public class GridNodeConfiguration extends GridConfiguration {
public static final String DEFAULT_NODE_CONFIG_FILE = "defaults/DefaultNodeWebDriver.json";
Expand Down Expand Up @@ -129,15 +131,24 @@ static final List<MutableCapabilities> getCapabilities() {

DesiredCapabilities firefox = new DesiredCapabilities();
firefox.setBrowserName("firefox");
firefox.setCapability("marionette", true);
firefox.setCapability("maxInstances", 5);
firefox.setCapability("seleniumProtocol", "WebDriver");

DesiredCapabilities ie = new DesiredCapabilities();
ie.setBrowserName("internet explorer");
ie.setPlatform(Platform.WINDOWS);
ie.setCapability("maxInstances", 1);
ie.setCapability("seleniumProtocol", "WebDriver");

return Lists.newArrayList(chrome, firefox, ie);
DesiredCapabilities safari = new DesiredCapabilities();
safari.setBrowserName("safari");
safari.setCapability("technologyPreview", false);
safari.setPlatform(Platform.MAC);
safari.setCapability("maxInstances", 1);
safari.setCapability("seleniumProtocol", "WebDriver");

return Lists.newArrayList(chrome, firefox, ie, safari);
}
}

Expand Down Expand Up @@ -515,15 +526,15 @@ public void fixUpCapabilities() {
}

Platform current = Platform.getCurrent();
for (MutableCapabilities cap : capabilities) {
if (cap.getPlatform() == null) {
cap.setCapability(CapabilityType.PLATFORM, current);
}
if (cap.getCapability(RegistrationRequest.SELENIUM_PROTOCOL) == null) {
cap.setCapability(RegistrationRequest.SELENIUM_PROTOCOL, SeleniumProtocol.WebDriver.toString());
}
cap.setCapability(CONFIG_UUID_CAPABILITY, UUID.randomUUID().toString());
}
capabilities = capabilities.stream()
.peek(cap -> cap.setCapability(CapabilityType.PLATFORM,
Optional.ofNullable(cap.getPlatform()).orElse(current)))
.filter(cap -> current.is(cap.getPlatform()))
.peek(cap -> cap.setCapability(RegistrationRequest.SELENIUM_PROTOCOL,
Optional.ofNullable(cap.getCapability(RegistrationRequest.SELENIUM_PROTOCOL))
.orElse(SeleniumProtocol.WebDriver.toString())))
.peek(cap -> cap.setCapability(CONFIG_UUID_CAPABILITY, UUID.randomUUID().toString()))
.collect(Collectors.toList());
}

public void fixUpHost() {
Expand Down

2 comments on commit dcb4792

@mach6
Copy link
Member

@mach6 mach6 commented on dcb4792 Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@barancev The reason for this is JCommander will not invoke new GridNodeConfiguration() when it determines the default value. Thus, the expectation (which is a comment in the class file) is to keep the default JSON in sync with the coded defaults -- in case someone calls GridNodeConfiguration foo = GridNodeConfiguration.loadFromJson(DEFAULT_NODE_CONFIG_FILE) or equivalent for the other config classes. Also FYI - the Test classes should assert that new GridNodeConfiguration() sets the same defaults as GridNodeConfiguration.loadFromJson(DEFAULT_NODE_CONFIG_FILE)

@barancev
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't see any reason why DefaultDesiredCapabilitiesBuilder cannot read capabilities from default JSON...

Please sign in to comment.