-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50245/#review143452
-----------------------------------------------------------



-1 There is considerable refactoring and use of Mockito missing in this diff 
that is present in the PR. Using GeodeSecurityUtil directly rather than a mock 
turns the Unit Test for GMSAuthenticator into an IntegrationTest. A Unit Test 
requires that GMSAuthenticator be completely isolated from the actual concrete 
implementations of collaborators and replacing them with test doubles including 
mocks and spies.

- Kirk Lund


On July 20, 2016, 4:51 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50245/
> -----------------------------------------------------------
> 
> (Updated July 20, 2016, 4:51 p.m.)
> 
> 
> Review request for geode, Grace Meilen, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * This closes #210
> * add more unit tests
> 
> 
> Diffs
> -----
> 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticator.java
>  f16a7221e929434748a8a039bb6114b75448ec82 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/interfaces/Authenticator.java
>  fe961279b7a6fd8d1837d3f854a6d670043b7122 
>   
> geode-core/src/main/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeave.java
>  e5cac2d4132a0898250b1d8aad7ccf7fb44bc3df 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/AcceptorImpl.java
>  43f90d57b853cf0831e58de555c287d9e24ad5a4 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/cache/tier/sockets/HandShake.java
>  2dcf8e7fd11685c23fdd5d789fa1ed243a882c05 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/i18n/LocalizedStrings.java
>  be1ff17cda5edc0659e63cdad32817b796967b70 
>   
> geode-core/src/main/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtil.java
>  d439b19e09fd372e19e21c824881cb1386d4d287 
>   
> geode-core/src/main/java/org/apache/geode/security/templates/SampleSecurityManager.java
>  f0275e476a8ed8236aa01f6830d43bc4f6193e36 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/DistributionConfigJUnitTest.java
>  d2b0d51898b33b86c79619ac6fbf559d669b2e6f 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/auth/GMSAuthenticatorJUnitTest.java
>  d52b261b5f1399aff2def02a4efef5db8bd057cb 
>   
> geode-core/src/test/java/com/gemstone/gemfire/distributed/internal/membership/gms/membership/GMSJoinLeaveJUnitTest.java
>  732d7a18bf486ca8ba2abd5bfde050f510d72691 
>   
> geode-core/src/test/java/com/gemstone/gemfire/internal/security/GeodeSecurityUtilTest.java
>  a26f06a2bc762ed59c31904823f449e622b6b36f 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/cli/commands/CliCommandTestBase.java
>  75d88aa3c3d14f4ea98142e90b9c8d298cf27f76 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/AccessControlMBeanJUnitTest.java
>  ac5c65a7b2da2b13b3dc70bf34ff2bf72ba674b3 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java
>  f38f2fc97d9ff33a8ee0b5e1b4e29c98a4a5b9f2 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java
>  3ded1dc91e95cfa1ea74c3e4e0017bc3d51162e7 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/CliCommandsSecurityTest.java
>  abcafaf2ea9951d1e6774364da7d1bfa5a43a007 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DataCommandsSecurityTest.java
>  01575b148f1364de15c59d64699eed3997125ba2 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java
>  05d3e3d4fc43ae383fe4edb6fb48aea4c28db57e 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GatewayReceiverMBeanSecurityTest.java
>  6c9769493c8d3e09bf966504b648a7aa7b490087 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GatewaySenderMBeanSecurityTest.java
>  48064645e52be75664038f21a8ee09ebd5e0d1c0 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GeodeSecurityUtilCustomRealmJUnitTest.java
>  5627c9e781f3de0e12ef40701d4339e1ba293225 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsPostProcessorTest.java
>  07bd1c162911085a892b71e0a18795c9e9e2dd7a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/GfshCommandsSecurityTest.java
>  6d42aa8f187eab94f87f9967ec8eb5965308f919 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JSONAuthorization.java
>  b97cf85ea87e8c28f43ff0dc4d082969d0eb4e8a 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/JsonAuthorizationCacheStartRule.java
>  d64e2eecf5c615fce31f62b5104fe23c16896b39 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java
>  f07358b698085f142b0c3b8e92b871df09fedee9 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java
>  6f8ee34fbb738c3c1572258f8d1a28975c6fc2f2 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java
>  425c4672a571c6480478fdb6b47a97130d3c4fb9 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MemberMBeanSecurityJUnitTest.java
>  e32b6ca4ce79679c1e931b847bc1584f77c6e671 
>   
> geode-core/src/test/java/com/gemstone/gemfire/management/internal/security/MultiUserDUnitTest.java
>  1c55a3cd944f5ee9ad0388e707ffeb8f6625d361 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/AbstractIntegratedClientAuthDistributedTest.java
>  10c316a275afe6ba59b6c090e168532b948d34d1 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/IntegratedSecurityCacheLifecycleDistributedTest.java
>  9f064821c1e6217112610762f61bb6aefbd121e8 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/IntegratedSecurityCacheLifecycleIntegrationTest.java
>  712329d2c76098c37356ffbe6528c000fb1d1699 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/IntegratedSecurityPeerAuthDistributedTest.java
>  PRE-CREATION 
>   
> geode-core/src/test/java/com/gemstone/gemfire/security/SpySecurityManager.java
>  PRE-CREATION 
>   geode-core/src/test/resources/com/gemstone/gemfire/security/peerAuth.json 
> PRE-CREATION 
>   geode-pulse/src/test/java/com/vmware/gemfire/tools/pulse/tests/Server.java 
> 7385e7fb8b4843e9deed1afce71f3278df26a14b 
> 
> Diff: https://reviews.apache.org/r/50245/diff/
> 
> 
> Testing
> -------
> 
> precheckin
> 
> This is based on the pull request 210. 
> https://github.com/apache/incubator-geode/pull/210
> Changes to the GMSAuthenticator to do the integrated security is minimal.
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>

Reply via email to