From 5839a4d38ae72e9045a8ba226c1d7b157141465e Mon Sep 17 00:00:00 2001 From: Sun Seng David TAN Date: Thu, 9 Jun 2022 11:20:59 +0200 Subject: [PATCH 1/3] chore(Config.getHomeDir): refactore to test Config.getHomeDir Signed-off-by: Sun Seng David TAN --- kubernetes-client-api/pom.xml | 4 +++ .../io/fabric8/kubernetes/client/Config.java | 29 ++++++++++--------- .../fabric8/kubernetes/client/ConfigTest.java | 11 +++++++ pom.xml | 8 +++++ 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/kubernetes-client-api/pom.xml b/kubernetes-client-api/pom.xml index e6b6ca02702..0b52740ad65 100644 --- a/kubernetes-client-api/pom.xml +++ b/kubernetes-client-api/pom.xml @@ -231,6 +231,10 @@ org.awaitility awaitility + + org.junit-pioneer + junit-pioneer + diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java index 5a0609af9b0..bf894f804fb 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -57,6 +57,7 @@ import java.util.Locale; import java.util.Map; import java.util.Optional; +import java.util.function.Predicate; import java.util.stream.Collectors; @JsonInclude(JsonInclude.Include.NON_NULL) @@ -867,34 +868,36 @@ private static boolean tryNamespaceFromPath(Config config) { } private static String getHomeDir() { + return getHomeDir(Config::isDirectoryAndExists); + } + + private static boolean isDirectoryAndExists(String filePath) { + File f = new File(filePath); + return f.exists() && f.isDirectory(); + } + + protected static String getHomeDir(Predicate directoryExists) { String osName = System.getProperty("os.name").toLowerCase(Locale.ROOT); if (osName.startsWith("win")) { String homeDrive = System.getenv("HOMEDRIVE"); String homePath = System.getenv("HOMEPATH"); if (homeDrive != null && !homeDrive.isEmpty() && homePath != null && !homePath.isEmpty()) { String homeDir = homeDrive + homePath; - File f = new File(homeDir); - if (f.exists() && f.isDirectory()) { + if (directoryExists.test(homeDir)) { return homeDir; } } String userProfile = System.getenv("USERPROFILE"); - if (userProfile != null && !userProfile.isEmpty()) { - File f = new File(userProfile); - if (f.exists() && f.isDirectory()) { - return userProfile; - } + if (userProfile != null && !userProfile.isEmpty() && directoryExists.test(userProfile)) { + return userProfile; } } String home = System.getenv("HOME"); - if (home != null && !home.isEmpty()) { - File f = new File(home); - if (f.exists() && f.isDirectory()) { - return home; - } + if (home != null && !home.isEmpty() && directoryExists.test(home)) { + return home; } - //Fall back to user.home should never really get here + // Fall back to user.home should never really get here return System.getProperty("user.home", "."); } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java index 76bfdf76954..ede1f85bd7e 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java @@ -25,6 +25,9 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.ClearEnvironmentVariable; +import org.junitpioneer.jupiter.SetEnvironmentVariable; +import org.junitpioneer.jupiter.SetSystemProperty; import java.io.File; import java.io.IOException; @@ -667,4 +670,12 @@ private String getTestPathValue(File commandFolder) { "/opt/apache-maven/bin"; } } + + @Test + @SetSystemProperty(key = "os.name", value = "Windows") + @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") + @SetEnvironmentVariable(key = "HOMEPATH", value = "user") + void getHomeDir_shouldUseHomedriveHomepathOnWindows() { + assertEquals("C:\\Users\\user", Config.getHomeDir(f -> true)); + } } diff --git a/pom.xml b/pom.xml index 0feff591126..1f259e6f0cc 100644 --- a/pom.xml +++ b/pom.xml @@ -96,6 +96,7 @@ 5.9.1 + 1.7.0 3.23.1 4.2.0 18.5.0 @@ -814,6 +815,13 @@ test + + org.junit-pioneer + junit-pioneer + ${junit-pioneer.version} + test + + org.assertj assertj-core From b763647efd84a4065b63be936cbcab5a3c3d3339 Mon Sep 17 00:00:00 2001 From: Sun Seng David TAN Date: Thu, 9 Jun 2022 11:33:52 +0200 Subject: [PATCH 2/3] fix(Config.getHomeDir): should behave like kubectl and use HOME env variable on windows if available Signed-off-by: Sun Seng David TAN --- .../io/fabric8/kubernetes/client/Config.java | 8 ++-- .../fabric8/kubernetes/client/ConfigTest.java | 45 ++++++++++++++++++- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java index bf894f804fb..0ebe9d8e35c 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -877,6 +877,10 @@ private static boolean isDirectoryAndExists(String filePath) { } protected static String getHomeDir(Predicate directoryExists) { + String home = System.getenv("HOME"); + if (home != null && !home.isEmpty() && directoryExists.test(home)) { + return home; + } String osName = System.getProperty("os.name").toLowerCase(Locale.ROOT); if (osName.startsWith("win")) { String homeDrive = System.getenv("HOMEDRIVE"); @@ -892,10 +896,6 @@ protected static String getHomeDir(Predicate directoryExists) { return userProfile; } } - String home = System.getenv("HOME"); - if (home != null && !home.isEmpty() && directoryExists.test(home)) { - return home; - } // Fall back to user.home should never really get here return System.getProperty("user.home", "."); diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java index ede1f85bd7e..17a23c2280d 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.EnabledOnOs; import org.junitpioneer.jupiter.ClearEnvironmentVariable; import org.junitpioneer.jupiter.SetEnvironmentVariable; import org.junitpioneer.jupiter.SetSystemProperty; @@ -49,6 +50,7 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.condition.OS.WINDOWS; public class ConfigTest { @@ -675,7 +677,48 @@ private String getTestPathValue(File commandFolder) { @SetSystemProperty(key = "os.name", value = "Windows") @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") @SetEnvironmentVariable(key = "HOMEPATH", value = "user") - void getHomeDir_shouldUseHomedriveHomepathOnWindows() { + @SetEnvironmentVariable(key = "USERPROFILE", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") + @ClearEnvironmentVariable(key = "HOME") + void getHomeDir_shouldUseHomedriveHomepathOnWindows_WhenHomeEnvVariableIsNotSet() { assertEquals("C:\\Users\\user", Config.getHomeDir(f -> true)); } + + @Test + @SetSystemProperty(key = "os.name", value = "Windows") + @ClearEnvironmentVariable(key = "HOMEDRIVE") + @ClearEnvironmentVariable(key = "HOMEPATH") + @SetEnvironmentVariable(key = "USERPROFILE", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") + @ClearEnvironmentVariable(key = "HOME") + void getHomeDir_shouldUseUserprofileOnWindows_WhenHomeHomedriveHomepathEnvVariablesAreNotSet() { + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + } + + @Test + @SetSystemProperty(key = "os.name", value = "Windows") + @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") + @SetEnvironmentVariable(key = "HOMEPATH", value = "user") + @SetEnvironmentVariable(key = "HOME", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") + void getHomeDir_shouldUseHomeEnvVariableOnWindows_WhenHomeEnvVariableIsSet() { + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + } + + @Test + @EnabledOnOs({ WINDOWS }) + @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") + @SetEnvironmentVariable(key = "HOMEPATH", value = "user") + @SetEnvironmentVariable(key = "HOME", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") + void getHomeDir_shouldUseHomeEnvVariable_WhenEnabledOnWindows_WhenHomeEnvVariableIsSet() { + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + } + + @Test + @SetSystemProperty(key = "user.home", value = "/home/user") + @ClearEnvironmentVariable(key = "HOMEDRIVE") + @ClearEnvironmentVariable(key = "HOMEPATH") + @ClearEnvironmentVariable(key = "HOME") + @ClearEnvironmentVariable(key = "USERPROFILE") + void getHomeDir_shouldReturnUserHomeProp_WhenHomeEnvVariablesAreNotSet() { + assertEquals("/home/user", Config.getHomeDir(f -> true)); + } + } From 2bfac50cd8a1cedbd6311aab0a5b41d2cba68911 Mon Sep 17 00:00:00 2001 From: Sun Seng David TAN Date: Tue, 20 Dec 2022 16:47:01 +0100 Subject: [PATCH 3/3] chore(test remove ext dep): reverting adding the junit pioneer external dependency in unit tests Signed-off-by: Sun Seng David TAN --- kubernetes-client-api/pom.xml | 4 - .../io/fabric8/kubernetes/client/Config.java | 17 ++-- .../fabric8/kubernetes/client/ConfigTest.java | 97 +++++++++++++------ pom.xml | 8 -- 4 files changed, 77 insertions(+), 49 deletions(-) diff --git a/kubernetes-client-api/pom.xml b/kubernetes-client-api/pom.xml index 0b52740ad65..e6b6ca02702 100644 --- a/kubernetes-client-api/pom.xml +++ b/kubernetes-client-api/pom.xml @@ -231,10 +231,6 @@ org.awaitility awaitility - - org.junit-pioneer - junit-pioneer - diff --git a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java index 0ebe9d8e35c..9fe713de125 100644 --- a/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java +++ b/kubernetes-client-api/src/main/java/io/fabric8/kubernetes/client/Config.java @@ -58,6 +58,7 @@ import java.util.Map; import java.util.Optional; import java.util.function.Predicate; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; @JsonInclude(JsonInclude.Include.NON_NULL) @@ -868,7 +869,7 @@ private static boolean tryNamespaceFromPath(Config config) { } private static String getHomeDir() { - return getHomeDir(Config::isDirectoryAndExists); + return getHomeDir(Config::isDirectoryAndExists, Config::getSystemEnvVariable); } private static boolean isDirectoryAndExists(String filePath) { @@ -876,22 +877,26 @@ private static boolean isDirectoryAndExists(String filePath) { return f.exists() && f.isDirectory(); } - protected static String getHomeDir(Predicate directoryExists) { - String home = System.getenv("HOME"); + private static String getSystemEnvVariable(String envVariableName) { + return System.getenv(envVariableName); + } + + protected static String getHomeDir(Predicate directoryExists, UnaryOperator getEnvVar) { + String home = getEnvVar.apply("HOME"); if (home != null && !home.isEmpty() && directoryExists.test(home)) { return home; } String osName = System.getProperty("os.name").toLowerCase(Locale.ROOT); if (osName.startsWith("win")) { - String homeDrive = System.getenv("HOMEDRIVE"); - String homePath = System.getenv("HOMEPATH"); + String homeDrive = getEnvVar.apply("HOMEDRIVE"); + String homePath = getEnvVar.apply("HOMEPATH"); if (homeDrive != null && !homeDrive.isEmpty() && homePath != null && !homePath.isEmpty()) { String homeDir = homeDrive + homePath; if (directoryExists.test(homeDir)) { return homeDir; } } - String userProfile = System.getenv("USERPROFILE"); + String userProfile = getEnvVar.apply("USERPROFILE"); if (userProfile != null && !userProfile.isEmpty() && directoryExists.test(userProfile)) { return userProfile; } diff --git a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java index 17a23c2280d..221594bc860 100644 --- a/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java +++ b/kubernetes-client-api/src/test/java/io/fabric8/kubernetes/client/ConfigTest.java @@ -26,9 +26,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.EnabledOnOs; -import org.junitpioneer.jupiter.ClearEnvironmentVariable; -import org.junitpioneer.jupiter.SetEnvironmentVariable; -import org.junitpioneer.jupiter.SetSystemProperty; import java.io.File; import java.io.IOException; @@ -674,51 +671,89 @@ private String getTestPathValue(File commandFolder) { } @Test - @SetSystemProperty(key = "os.name", value = "Windows") - @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") - @SetEnvironmentVariable(key = "HOMEPATH", value = "user") - @SetEnvironmentVariable(key = "USERPROFILE", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") - @ClearEnvironmentVariable(key = "HOME") void getHomeDir_shouldUseHomedriveHomepathOnWindows_WhenHomeEnvVariableIsNotSet() { - assertEquals("C:\\Users\\user", Config.getHomeDir(f -> true)); + String osNamePropToRestore = System.getProperty("os.name"); + try { + + System.setProperty("os.name", "Windows"); + + Map envVars = new HashMap(); + envVars.put("HOMEDRIVE", "C:\\Users\\"); + envVars.put("HOMEPATH", "user"); + envVars.put("USERPROFILE", "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\"); + + assertEquals("C:\\Users\\user", Config.getHomeDir(f -> true, envVars::get)); + + } finally { + System.setProperty("os.name", osNamePropToRestore); + } } @Test - @SetSystemProperty(key = "os.name", value = "Windows") - @ClearEnvironmentVariable(key = "HOMEDRIVE") - @ClearEnvironmentVariable(key = "HOMEPATH") - @SetEnvironmentVariable(key = "USERPROFILE", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") - @ClearEnvironmentVariable(key = "HOME") void getHomeDir_shouldUseUserprofileOnWindows_WhenHomeHomedriveHomepathEnvVariablesAreNotSet() { - assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + String osNamePropToRestore = System.getProperty("os.name"); + try { + + System.setProperty("os.name", "Windows"); + + Map envVars = new HashMap(); + envVars.put("USERPROFILE", "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\"); + + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", + Config.getHomeDir(f -> true, envVars::get)); + + } finally { + System.setProperty("os.name", osNamePropToRestore); + } } @Test - @SetSystemProperty(key = "os.name", value = "Windows") - @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") - @SetEnvironmentVariable(key = "HOMEPATH", value = "user") - @SetEnvironmentVariable(key = "HOME", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") void getHomeDir_shouldUseHomeEnvVariableOnWindows_WhenHomeEnvVariableIsSet() { - assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + String osNamePropToRestore = System.getProperty("os.name"); + try { + + System.setProperty("os.name", "Windows"); + + Map envVars = new HashMap(); + envVars.put("HOMEDRIVE", "C:\\Users\\"); + envVars.put("HOMEPATH", "user"); + envVars.put("HOME", "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\"); + + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", + Config.getHomeDir(f -> true, envVars::get)); + + } finally { + System.setProperty("os.name", osNamePropToRestore); + } } @Test @EnabledOnOs({ WINDOWS }) - @SetEnvironmentVariable(key = "HOMEDRIVE", value = "C:\\Users\\") - @SetEnvironmentVariable(key = "HOMEPATH", value = "user") - @SetEnvironmentVariable(key = "HOME", value = "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\") void getHomeDir_shouldUseHomeEnvVariable_WhenEnabledOnWindows_WhenHomeEnvVariableIsSet() { - assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", Config.getHomeDir(f -> true)); + + Map envVars = new HashMap(); + envVars.put("HOMEDRIVE", "C:\\Users\\"); + envVars.put("HOMEPATH", "user"); + envVars.put("HOME", "C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\"); + + assertEquals("C:\\Users\\user\\workspace\\myworkspace\\tools\\cygwin\\", + Config.getHomeDir(f -> true, envVars::get)); + } @Test - @SetSystemProperty(key = "user.home", value = "/home/user") - @ClearEnvironmentVariable(key = "HOMEDRIVE") - @ClearEnvironmentVariable(key = "HOMEPATH") - @ClearEnvironmentVariable(key = "HOME") - @ClearEnvironmentVariable(key = "USERPROFILE") void getHomeDir_shouldReturnUserHomeProp_WhenHomeEnvVariablesAreNotSet() { - assertEquals("/home/user", Config.getHomeDir(f -> true)); - } + String userHomePropToRestore = System.getProperty("user.home"); + try { + + System.setProperty("user.home", "/home/user"); + + Map emptyEnvVars = Collections.emptyMap(); + assertEquals("/home/user", Config.getHomeDir(f -> true, emptyEnvVars::get)); + + } finally { + System.setProperty("user.home", userHomePropToRestore); + } + } } diff --git a/pom.xml b/pom.xml index 1f259e6f0cc..0feff591126 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,6 @@ 5.9.1 - 1.7.0 3.23.1 4.2.0 18.5.0 @@ -815,13 +814,6 @@ test - - org.junit-pioneer - junit-pioneer - ${junit-pioneer.version} - test - - org.assertj assertj-core