You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/12/10 00:36:45 UTC
Review Request 54617: GEODE-2196: add more tests to cover import
cluster-configuration and deploy
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54617/
-----------------------------------------------------------
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
Re: Review Request 54617: GEODE-2196: add more tests to cover import
cluster-configuration and deploy
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
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
>
>
Re: Review Request 54617: GEODE-2196: add more tests to cover import
cluster-configuration and deploy
Posted by Jared Stewart <js...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54617/#review158939
-----------------------------------------------------------
Ship it!
Ship It!
- Jared Stewart
On Dec. 12, 2016, 8:13 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54617/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 8:13 p.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/DistributionConfig.java 5eb070dd99be6d4a8f710341001879f424d7183c
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java 0c6603dba932ff8c23312e422d677e8650e0cb17
> geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java a45e84362f411ff2908dd6b1a8978a0d9eb8df43
> geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java f6a6e8ebf1a130a799f36b1b041162769c787ec5
> 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 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/DistributedRestoreSystemProperties.java 221b52a714d9df1106edb40f26f8b542283ca735
> 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
>
>
Re: Review Request 54617: GEODE-2196: add more tests to cover import
cluster-configuration and deploy
Posted by Kevin Duling <kd...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54617/#review159014
-----------------------------------------------------------
Ship it!
Ship It!
- Kevin Duling
On Dec. 12, 2016, 12:13 p.m., Jinmei Liao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54617/
> -----------------------------------------------------------
>
> (Updated Dec. 12, 2016, 12:13 p.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/DistributionConfig.java 5eb070dd99be6d4a8f710341001879f424d7183c
> geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java 0c6603dba932ff8c23312e422d677e8650e0cb17
> geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java a45e84362f411ff2908dd6b1a8978a0d9eb8df43
> geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java f6a6e8ebf1a130a799f36b1b041162769c787ec5
> 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 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/DistributedRestoreSystemProperties.java 221b52a714d9df1106edb40f26f8b542283ca735
> 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
>
>
Re: Review Request 54617: GEODE-2196: add more tests to cover import
cluster-configuration and deploy
Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54617/
-----------------------------------------------------------
(Updated Dec. 12, 2016, 8:13 p.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 (updated)
-----
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java 5eb070dd99be6d4a8f710341001879f424d7183c
geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfigImpl.java 0c6603dba932ff8c23312e422d677e8650e0cb17
geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java a45e84362f411ff2908dd6b1a8978a0d9eb8df43
geode-core/src/main/java/org/apache/geode/internal/JarDeployer.java f6a6e8ebf1a130a799f36b1b041162769c787ec5
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 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/DistributedRestoreSystemProperties.java 221b52a714d9df1106edb40f26f8b542283ca735
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