Fixed an issue that caused null pointers to occur when reading enviro…

…nment configurations in parallel execution

serenity-core/src/main/java/net/thucydides/core/webdriver/capabilities/W3CCapabilities.java

@@ -30,9 +30,11 @@
 public class W3CCapabilities {
 
     private final EnvironmentVariables environmentVariables;
-    private static final Logger LOGGER = LoggerFactory.getLogger(W3CCapabilities.class);
     private String prefix;
-    private final Set<String> STRING_CONFIG_PROPERTIES = new HashSet<>(Arrays.asList("platformName","platformVersion"));
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(W3CCapabilities.class);
+    private static final Set<String> STRING_CONFIG_PROPERTIES = new HashSet<>(Arrays.asList("platformName","platformVersion"));
+    private static final List<String> BASE_PROPERTIES = Arrays.asList("browserName", "browserVersion", "platformName");
 
     public W3CCapabilities(EnvironmentVariables environmentVariables) {
         this.environmentVariables = environmentVariables;
@@ -42,8 +44,6 @@ public static W3CCapabilities definedIn(EnvironmentVariables environmentVariable
         return new W3CCapabilities(environmentVariables);
     }
 
-    private final List<String> BASE_PROPERTIES = Arrays.asList("browserName", "browserVersion", "platformName");
-
     public W3CCapabilities withPrefix(String prefix) {
         this.prefix = prefix;
         return this;
@@ -142,7 +142,7 @@ private void addCapabilityValue(Config config, DesiredCapabilities capabilities,
         if (STRING_CONFIG_PROPERTIES.contains(fieldName)) {
             capabilities.setPlatform(Platform.fromString(value.unwrapped().toString()));
         } else {
-            capabilities.setCapability(fieldName, asObject(value));
+            capabilities.setCapability(stripQuotesFrom(fieldName), asObject(value));
         }
     }
 

serenity-core/src/test/java/net/thucydides/core/util/WhenLoadingPreferencesFromALocalPropertiesFile.java → serenity-core/src/test/java/net/thucydides/core/util/WhenLoadingPreferencesFromAPropertiesFile.java

@@ -1,7 +1,6 @@
 package net.thucydides.core.util;
 
 import net.thucydides.core.environment.MockEnvironmentVariables;
-import net.thucydides.core.environment.SystemEnvironmentVariables;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
@@ -12,35 +11,37 @@
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.HashMap;
+import java.util.Map;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
 
-public class WhenLoadingPreferencesFromALocalPropertiesFile {
+public class WhenLoadingPreferencesFromAPropertiesFile {
 
-    Logger LOGGER = LoggerFactory.getLogger(WhenLoadingPreferencesFromALocalPropertiesFile.class);
+    Logger LOGGER = LoggerFactory.getLogger(WhenLoadingPreferencesFromAPropertiesFile.class);
 
     @Rule
     public ExtendedTemporaryFolder temporaryFolder = new ExtendedTemporaryFolder();
 
     File homeDirectory;
     File thucydidesPropertiesFile;
-    EnvironmentVariables environmentVariables;
-    PropertiesFileLocalPreferences localPreferences;
+    Map<String,String> environmentVariables;
+    PropertiesLocalPreferences localPreferences;
 
     @Before
     public void setupDirectories() throws IOException {
-        environmentVariables = new MockEnvironmentVariables();
-        localPreferences = new PropertiesFileLocalPreferences(environmentVariables);
+        environmentVariables = new HashMap<>();
+        localPreferences = new PropertiesLocalPreferences(environmentVariables);
 
         homeDirectory = temporaryFolder.newFolder();
         localPreferences.setHomeDirectory(homeDirectory);
     }
 
     @Test
-    public void the_default_preferences_directory_is_the_users_home_directory() throws Exception {
-        PropertiesFileLocalPreferences localPreferences = new PropertiesFileLocalPreferences(environmentVariables);
+    public void the_default_preferences_directory_is_the_users_home_directory() {
+        PropertiesLocalPreferences localPreferences = new PropertiesLocalPreferences(environmentVariables);
 
         String homeDirectory = System.getProperty("user.home");
 
@@ -55,7 +56,7 @@ public void should_load_property_values_from_local_preferences() throws Exceptio
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("webdriver.driver"), is("opera"));
+        assertThat(environmentVariables.get("webdriver.driver"), is("opera"));
     }
 
     @Test
@@ -66,19 +67,19 @@ public void home_properties_should_override_classpath_properties() throws Except
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("test.property"), is("reset"));
+        assertThat(environmentVariables.get("test.property"), is("reset"));
     }
 
     @Test
     public void local_preferences_should_not_override_system_preferences() throws Exception {
         writeToPropertiesFile("webdriver.driver = opera");
 
-        environmentVariables.setProperty("webdriver.driver", "iexplorer");
+        environmentVariables.put("webdriver.driver", "iexplorer");
         localPreferences.setHomeDirectory(homeDirectory);
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("webdriver.driver"), is("iexplorer"));
+        assertThat(environmentVariables.get("webdriver.driver"), is("iexplorer"));
     }
 
     @Test
@@ -91,7 +92,7 @@ public void should_load_preferences_from_a_designated_properties_file_if_specifi
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("webdriver.driver"), is("safari"));
+        assertThat(environmentVariables.get("webdriver.driver"), is("safari"));
 
         System.clearProperty("properties");
     }
@@ -106,7 +107,7 @@ public void should_load_preferences_from_a_designated_properties_filepath_if_spe
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("webdriver.driver"), is("safari"));
+        assertThat(environmentVariables.get("webdriver.driver"), is("safari"));
 
         System.clearProperty("properties");
     }
@@ -120,7 +121,7 @@ public void should_ignore_preferences_if_specified_file_does_not_exist() throws
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("webdriver.driver"), is(nullValue()));
+        assertThat(environmentVariables.get("webdriver.driver"), is(nullValue()));
 
         System.clearProperty("properties");
 
@@ -143,7 +144,7 @@ public void should_load_property_values_from_typesafe_config() throws Exception
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("serenity.logging"), is("VERBOSE"));
+        assertThat(environmentVariables.get("serenity.logging"), is("VERBOSE"));
     }
 
     @Test
@@ -152,23 +153,9 @@ public void should_load_arbitrary_property_values_from_typesafe_config() throws
 
         localPreferences.loadPreferences();
 
-        assertThat(environmentVariables.getProperty("environment.uat"), is("uat-server"));
+        assertThat(environmentVariables.get("environment.uat"), is("uat-server"));
     }
-
-    @Test
-    public void users_can_define_optional_custom_properties() {
-
-        // WHEN
-        environmentVariables.setProperty("env","QA");
-
-        // THEN
-        assertThat(environmentVariables.optionalProperty("env").isPresent(), is(true));
-
-        // BUT
-        assertThat(environmentVariables.optionalProperty("undefined").isPresent(), is(false));
-
-    }
-
+
     @SuppressWarnings("static-access")
     private String writeToPropertiesFileCalled(String filename, String... lines) throws IOException, InterruptedException {
         thucydidesPropertiesFile = new File(homeDirectory, filename);

serenity-core/src/test/java/net/thucydides/core/webdriver/capabilities/WhenConvertingW3CPropertiesToChromeOptions.java

@@ -30,7 +30,7 @@
 class WhenConvertingW3CPropertiesToChromeOptions {
     private static EnvironmentVariables from(String testConfig) {
         Path configFilepath = new File(Resources.getResource(testConfig).getPath()).toPath();
-        return SystemEnvironmentVariables.createEnvironmentVariables(configFilepath, new SystemEnvironmentVariables());
+        return SystemEnvironmentVariables.createEnvironmentVariables(configFilepath);
     }
 
     @DisplayName("If no webdriver section is present, use a standard ChromeOptions object")

serenity-core/src/test/java/net/thucydides/core/webdriver/capabilities/WhenSettingW3CProperties.java

@@ -31,7 +31,9 @@ private static EnvironmentVariables from(String testConfig) {
     @Test
     public void shouldReadW3COptionsFromConfFile() {
         EnvironmentVariables environmentVariables = from("sample-conf-files/simple.conf");
-        DesiredCapabilities caps = W3CCapabilities.definedIn(environmentVariables).withPrefix("webdriver.capabilities").asDesiredCapabilities();
+        DesiredCapabilities caps = W3CCapabilities.definedIn(environmentVariables)
+                .withPrefix("webdriver.capabilities")
+                .asDesiredCapabilities();
 
         assertThat(caps.getBrowserName()).isEqualTo("Chrome");
         assertThat(caps.getBrowserVersion()).isEqualTo("103.0");

serenity-jira-plugin/src/main/java/net/serenitybdd/plugins/jira/service/SystemPropertiesJIRAConfiguration.java

@@ -3,7 +3,7 @@
 import net.serenitybdd.core.di.SerenityInfrastructure;
 import net.thucydides.core.util.EnvironmentVariables;
 import net.thucydides.core.util.LocalPreferences;
-import net.thucydides.core.util.PropertiesFileLocalPreferences;
+import net.thucydides.core.util.PropertiesLocalPreferences;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -38,7 +38,7 @@ public SystemPropertiesJIRAConfiguration(EnvironmentVariables environmentVariabl
 
 
     private void updateEnvironmentVariablesFromPropertiesFiles(EnvironmentVariables environmentVariables) {
-        LocalPreferences localPreferences = new PropertiesFileLocalPreferences(environmentVariables);
+        LocalPreferences localPreferences = new PropertiesLocalPreferences(environmentVariables);
         try {
             localPreferences.loadPreferences();
         } catch (IOException e) {

serenity-model/src/main/java/net/thucydides/core/environment/MockEnvironmentVariables.java

@@ -1,46 +1,44 @@
 package net.thucydides.core.environment;
 
 import com.typesafe.config.Config;
-import net.serenitybdd.core.collect.NewMap;
 import net.serenitybdd.core.environment.ConfiguredEnvironment;
 import net.thucydides.core.util.EnvironmentVariables;
-import net.thucydides.core.util.PropertiesUtil;
 import org.apache.commons.lang3.StringUtils;
 
 import java.util.*;
 import java.util.stream.Collectors;
 
 public class MockEnvironmentVariables implements EnvironmentVariables {
 
-    private Properties properties = new Properties();
+    private Map<String, String> properties = new HashMap<>();
     private Map<String, String> values = new HashMap<>();
 
     public MockEnvironmentVariables() {
-        this.properties.setProperty("user.home", System.getProperty("user.home"));
-        this.properties.setProperty("feature.file.encoding", "UTF-8");
+        this.properties.put("user.home", System.getProperty("user.home"));
+        this.properties.put("feature.file.encoding", "UTF-8");
         if (localEnvironment().getProperty("phantomjs.binary.path") != null) {
-            this.properties.setProperty("phantomjs.binary.path", localEnvironment().getProperty("phantomjs.binary.path"));
+            this.properties.put("phantomjs.binary.path", localEnvironment().getProperty("phantomjs.binary.path"));
         }
         if (localEnvironment().getProperty("webdriver.chrome.driver") != null) {
-            this.properties.setProperty("webdriver.chrome.driver", localEnvironment().getProperty("webdriver.chrome.driver"));
+            this.properties.put("webdriver.chrome.driver", localEnvironment().getProperty("webdriver.chrome.driver"));
         }
     }
 
     private EnvironmentVariables localEnvironment() {
         return ConfiguredEnvironment.getEnvironmentVariables();
     }
 
-    protected MockEnvironmentVariables(Properties properties) {
-        this.properties = PropertiesUtil.copyOf(properties);
+    protected MockEnvironmentVariables(Map<String, String> properties) {
+        this.properties = new HashMap<>(properties);
     }
 
-    protected MockEnvironmentVariables(Properties properties, Map<String, String> values) {
-        this.properties = PropertiesUtil.copyOf(properties);
-        this.values = NewMap.copyOf(values);
+    protected MockEnvironmentVariables(Map<String, String> properties, Map<String, String> values) {
+        this.properties = new HashMap<>(properties);
+        this.values = new HashMap<>(values);
     }
 
     public static EnvironmentVariables fromSystemEnvironment() {
-        return new MockEnvironmentVariables(SystemEnvironmentVariables.createEnvironmentVariables().getProperties());
+        return new MockEnvironmentVariables(SystemEnvironmentVariables.createEnvironmentVariables().properties());
     }
 
     public boolean propertySetIsEmpty() {
@@ -65,9 +63,9 @@ public String getValue(Enum<?> property, String defaultValue) {
     }
 
     public Integer getPropertyAsInteger(String name, Integer defaultValue) {
-        String value = (String) properties.get(name);
+        String value = properties.get(name);
         if (StringUtils.isNumeric(value)) {
-            return Integer.parseInt(properties.getProperty(name));
+            return Integer.parseInt(properties.get(name));
         } else {
             return defaultValue;
         }
@@ -79,10 +77,10 @@ public Integer getPropertyAsInteger(Enum<?> property, Integer defaultValue) {
     }
 
     public Boolean getPropertyAsBoolean(String name, boolean defaultValue) {
-        if (properties.getProperty(name) == null) {
+        if (properties.get(name) == null) {
             return defaultValue;
         } else {
-            return Boolean.parseBoolean(properties.getProperty(name, "false"));
+            return Boolean.parseBoolean(properties.get(name));
         }
     }
 
@@ -93,7 +91,7 @@ public Boolean getPropertyAsBoolean(Enum<?> property, boolean defaultValue) {
 
     public String getProperty(String name) {
         if (name != null) {
-            return properties.getProperty(name);
+            return properties.get(name);
         } else {
             return null;
         }
@@ -110,7 +108,7 @@ public String getProperty(Enum<?> property) {
     }
 
     public String getProperty(String name, String defaultValue) {
-        return properties.getProperty(name, defaultValue);
+        return properties.get(name) == null ? defaultValue : properties.get(name);
     }
 
 
@@ -119,7 +117,7 @@ public String getProperty(Enum<?> property, String defaultValue) {
     }
 
     public void setProperty(String name, String value) {
-        properties.setProperty(name, value);
+        properties.put(name, value);
     }
 
     public void setProperties(Map<String, String> newProperties) {
@@ -144,7 +142,9 @@ public List<String> getKeys() {
 
     @Override
     public Properties getProperties() {
-        return new Properties(properties);
+        Properties props = new Properties();
+        props.putAll(properties);
+        return props;
     }
 
     @Override
@@ -154,12 +154,12 @@ public Properties getPropertiesWithPrefix(String prefix) {
 
     @Override
     public boolean aValueIsDefinedFor(Enum<?> property) {
-        return properties.contains(property.toString());
+        return properties.containsKey(property.toString());
     }
 
     @Override
     public boolean aValueIsDefinedFor(String property) {
-        return properties.contains(property);
+        return properties.containsKey(property);
     }
 
     @Override
@@ -179,8 +179,8 @@ public Map<String, String> asMap() {
         values.keySet().forEach(
                 key -> environmentValues.put(key, values.get(key))
         );
-        properties.stringPropertyNames().forEach(
-                key -> environmentValues.put(key, properties.getProperty(key))
+        properties.keySet().forEach(
+                key -> environmentValues.put(key, properties.get(key))
         );
         return environmentValues;
     }
@@ -204,6 +204,11 @@ public Config getConfig(String prefix) {
         return EnvironmentVariables.super.getConfig(prefix);
     }
 
+    @Override
+    public Map<String, String> properties() {
+        return properties;
+    }
+
     public void setValue(String name, String value) {
         values.put(name, value);
     }