----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54617/#review158739 -----------------------------------------------------------
geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigResult.java (line 87) <https://reviews.apache.org/r/54617/#comment229543> I think there should be asserts that `properties` and `xml` both exist like with `configDir`. geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigResult.java (line 150) <https://reviews.apache.org/r/54617/#comment229545> Maybe rename to `verifyFileExists` to make the tests read more fluently. Same with `verifyFileDoesNotExist` below. I think we could also make these methods take a `Member` instead of a `workingDir` and push the `.getWorkingDir()` calls inside, so the tests would look like ``` ClusterConfigResult.verifyFileExists(locator, "cluster_config/group2/group2.jar") `` - Jared Stewart On Dec. 10, 2016, 12:36 a.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54617/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2016, 12:36 a.m.) > > > Review request for geode, Jared Stewart, Kevin Duling, and Kirk Lund. > > > Repository: geode > > > Description > ------- > > GEODE-2196: add more tests to cover import cluster-configuration and deploy > > > Diffs > ----- > > > geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java > a45e84362f411ff2908dd6b1a8978a0d9eb8df43 > > geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java > 883a0360559e13a946579660d7e4cc81a94d8841 > > geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java > 142ce17a144547168f7ee89fecc6caf35665296d > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDUnitTest.java > 418999ae1500bcd136ad69b4cb8512483cfb1cce > > geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigResult.java > PRE-CREATION > > geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java > b040751e0c9b93560aae7d69726c627094978740 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > 74909284ccb07cc9ed582b7e7423c107c29dbfde > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > c56f7abbba7382fbd2542596e8337c5a83280d47 > > geode-core/src/test/resources/org/apache/geode/management/internal/configuration/cluster_config.zip > 37cacbf82a6fcdf284a61c78178a02e6ef20e632 > > Diff: https://reviews.apache.org/r/54617/diff/ > > > Testing > ------- > > -- added tests for import cluster config and deploy jars > -- refactor the GfshConnectorRule to do the retry logic inside the rule so > that users won't have worry about it. > -- moved all the expected data/verification code into ClusterConfigResult > class to make the main test class cleaner. > > > Thanks, > > Jinmei Liao > >
