Issue-271: unit test the configuration

* Make value holding the properties an instance field
* Make constructor private

The above measures make the singleton class more aligned with the singleton pattern while also enable unit testing of the config class.

Due to the amount of classes in the impl package a private constructor is preferred over a package private one as the latter might be seen as an invitation to just instantiate the class, when this is clearly not desired.

Eager reading of the properties file has been deemed acceptable, because the `static` block within the class will ensure that the file is read early anyway.

Signed-off-by: l-1sqared <30831153+l-1squared@users.noreply.github.com>

jgiven-core/src/main/java/com/tngtech/jgiven/impl/Config.java

@@ -1,9 +1,6 @@
 package com.tngtech.jgiven.impl;
 
 import com.tngtech.jgiven.config.ConfigValue;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import java.io.File;
 import java.io.IOException;
 import java.io.Reader;
@@ -12,13 +9,14 @@
 import java.nio.file.Paths;
 import java.util.Optional;
 import java.util.Properties;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * Helper class to access all system properties to configure JGiven.
  */
 public class Config {
     private static final Logger log = LoggerFactory.getLogger(Config.class);
-    private static final Properties CONFIG_FILE_PROPERTIES = loadConfigFileProperties();
     private static final Config INSTANCE = new Config();
 
     private static final String TRUE = "true";
@@ -33,6 +31,8 @@ public class Config {
     private static final String JGIVEN_CONFIG_PATH = "jgiven.config.path";
     private static final String JGIVEN_CONFIG_CHARSET = "jgiven.config.charset";
 
+    private final Properties configFileProperties = loadConfigFileProperties();
+
     public static Config config() {
         return INSTANCE;
     }
@@ -43,6 +43,9 @@ public static Config config() {
         }
     }
 
+    private Config() {
+    }
+
     private static Properties loadConfigFileProperties() {
         String path = System.getProperty(JGIVEN_CONFIG_PATH, "jgiven.properties");
         String charset = System.getProperty(JGIVEN_CONFIG_CHARSET, "UTF-8");
@@ -62,9 +65,14 @@ private String resolveProperty(String name) {
     }
 
     private String resolveProperty(String name, String defaultValue) {
-        return System.getProperty(name, CONFIG_FILE_PROPERTIES.getProperty(name, defaultValue));
+        return System.getProperty(name, configFileProperties.getProperty(name, defaultValue));
     }
 
+    /**
+     * Returns the directory set either via a configuration file or a system property.
+     * If no value is specified and the surefire test classpath is set, the default maven directory will be used,
+     * otherwise a default is returned.
+     */
     public Optional<File> getReportDir() {
         String reportDirName = resolveProperty(JGIVEN_REPORT_DIR);
         if (reportDirName == null) {

jgiven-core/src/test/java/com/tngtech/jgiven/impl/ConfigTest.java

@@ -0,0 +1,104 @@
+package com.tngtech.jgiven.impl;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.google.common.base.Charsets;
+import com.google.common.io.CharSink;
+import com.google.common.io.FileWriteMode;
+import com.google.common.io.Files;
+import com.tngtech.jgiven.config.ConfigValue;
+import java.io.File;
+import java.lang.reflect.Constructor;
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
+
+public class ConfigTest {
+
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
+    private final Map<String, String> systemPropertiesBackup = new HashMap<>();
+    private CharSink jgivenConfig;
+
+    @Before
+    public void setupPropertiesFile() throws Exception {
+        File configFile = temporaryFolder.newFile();
+        jgivenConfig = Files.asCharSink(configFile, Charsets.UTF_8, FileWriteMode.APPEND);
+        setSystemProperty("jgiven.config.path", configFile.getAbsolutePath());
+        setSystemProperty("jgiven.report.dir", null);
+    }
+
+    @Test
+    public void configValuesHaveDefaults() throws Exception {
+        Config underTest = createNewTestInstance();
+
+        assertThat(underTest.isReportEnabled()).isTrue();
+        assertThat(underTest.getReportDir()).get().extracting(File::getPath).isEqualTo("jgiven-reports");
+        assertThat(underTest.textColorEnabled()).extracting(Enum::name).isEqualTo("AUTO");
+        assertThat(underTest.filterStackTrace()).isTrue();
+    }
+
+    @Test
+    public void configFileValuesAreRecognized() throws Exception {
+        File reportPath = temporaryFolder.newFolder();
+        jgivenConfig.write("jgiven.report.enabled=false\n");
+        jgivenConfig.write("jgiven.report.dir="
+            + reportPath.getAbsolutePath().replace("\\", "/") + "\n");
+        jgivenConfig.write("jgiven.report.text=false\n");
+        jgivenConfig.write("jgiven.report.text.color=true\n");
+        jgivenConfig.write("jgiven.report.filterStackTrace=false\n");
+
+        Config underTest = createNewTestInstance();
+
+        assertThat(underTest.isReportEnabled()).isFalse();
+        assertThat(underTest.getReportDir()).contains(reportPath);
+        assertThat(underTest.textReport()).isFalse();
+        assertThat(underTest.textColorEnabled()).isEqualTo(ConfigValue.TRUE);
+        assertThat(underTest.filterStackTrace()).isFalse();
+
+    }
+
+    @Test
+    public void testCommandLinePropertiesTakePrecedenceOverConfigFile() throws Exception {
+        jgivenConfig.write("jgiven.report.enabled=false\n");
+        setSystemProperty("jgiven.report.enabled", "true");
+
+        Config underTest = createNewTestInstance();
+
+        assertThat(underTest.isReportEnabled()).isTrue();
+    }
+
+    @After
+    public void cleanupSystemProperties() {
+        systemPropertiesBackup.entrySet()
+            .stream()
+            .peek(entry -> System.clearProperty(entry.getKey()))
+            .filter(entry -> entry.getValue() != null)
+            .forEach(entry -> System.setProperty(entry.getKey(), entry.getValue()));
+    }
+
+    private static Config createNewTestInstance() throws Exception {
+        Constructor<Config> constructor = Config.class.getDeclaredConstructor();
+        try {
+            constructor.setAccessible(true);
+            return constructor.newInstance();
+        } finally {
+            constructor.setAccessible(false);
+        }
+    }
+
+    private void setSystemProperty(String key, String value) {
+        String originalValue = System.getProperty(key);
+        systemPropertiesBackup.put(key, originalValue);
+        if (value == null) {
+            System.clearProperty(key);
+        } else {
+            System.setProperty(key, value);
+        }
+    }
+}