----------------------------------------------------------- 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 > >