From 6baf3c2c9fb499e2768320cc70fffcf12af8ac62 Mon Sep 17 00:00:00 2001 From: l-1sqared <30831153+l-1squared@users.noreply.github.com> Date: Tue, 6 Jul 2021 08:05:42 +0200 Subject: [PATCH] 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> --- .../java/com/tngtech/jgiven/impl/Config.java | 18 ++- .../com/tngtech/jgiven/impl/ConfigTest.java | 104 ++++++++++++++++++ 2 files changed, 117 insertions(+), 5 deletions(-) create mode 100644 jgiven-core/src/test/java/com/tngtech/jgiven/impl/ConfigTest.java diff --git a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/Config.java b/jgiven-core/src/main/java/com/tngtech/jgiven/impl/Config.java index bb57bf2d925..8dc46932696 100644 --- a/jgiven-core/src/main/java/com/tngtech/jgiven/impl/Config.java +++ b/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 getReportDir() { String reportDirName = resolveProperty(JGIVEN_REPORT_DIR); if (reportDirName == null) { diff --git a/jgiven-core/src/test/java/com/tngtech/jgiven/impl/ConfigTest.java b/jgiven-core/src/test/java/com/tngtech/jgiven/impl/ConfigTest.java new file mode 100644 index 00000000000..20a72b13e62 --- /dev/null +++ b/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 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 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); + } + } +}