----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39585/#review104173 -----------------------------------------------------------
We need to have you remove usage of hydra.Log. All hydra classes are either only on the closed side or need to be removed from open. Please add comments to every commented out dead code block so we know why it was commented out instead of being deleted. We also need to get rid of any thread sleeps and replace them with WaitCriterion from DistributedTestCase or the test will fail intermittently on heavily loaded machines (including ASF infrastructure). gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/CommandTestBase.java (line 163) <https://reviews.apache.org/r/39585/#comment162453> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/CommandTestBase.java (line 298) <https://reviews.apache.org/r/39585/#comment162454> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/IntegratedSecDUnitTest.java (line 3) <https://reviews.apache.org/r/39585/#comment162455> need to remove this usage of hydra because it should be in open yet -- maybe just use a log4j2 logger: private static final Logger logger = LogService.getLogger(); gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/IntegratedSecDUnitTest.java (line 134) <https://reviews.apache.org/r/39585/#comment162456> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/IntegratedSecDUnitTest.java (line 230) <https://reviews.apache.org/r/39585/#comment162457> change this to use DistributedTestCase.WaitCriterion with check to see if mbeans have been registered or this will fail intermittently because of thread sleep gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/IntegratedSecDUnitTest.java (line 640) <https://reviews.apache.org/r/39585/#comment162458> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java (line 3) <https://reviews.apache.org/r/39585/#comment162459> same as the last test -- need to get rid of all hydra.Log imports and use log4j2 Logger or cache.getLogger instead gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java (line 148) <https://reviews.apache.org/r/39585/#comment162460> please add comment explaining if this code is incomplete or if it works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java (line 225) <https://reviews.apache.org/r/39585/#comment162461> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java (line 281) <https://reviews.apache.org/r/39585/#comment162462> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java (line 328) <https://reviews.apache.org/r/39585/#comment162463> please add comment explaining if this code works in old ant build but not gradle gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestAccessControl.java (line 3) <https://reviews.apache.org/r/39585/#comment162464> need to get rid of using hydra.Log gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestAuthenticator.java (line 3) <https://reviews.apache.org/r/39585/#comment162465> need to get rid of using hydra.Log - Kirk Lund On Oct. 27, 2015, 3:39 p.m., Tushar Khairnar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39585/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2015, 3:39 p.m.) > > > Review request for geode, Anthony Baker, Jens Deppe, Kirk Lund, and Dan Smith. > > > Repository: geode > > > Description > ------- > > 1. CommandTestBase - Base class for all the CLI/gfsh command dunit tests. > 2. HeadlessGfsh - Gfsh as an API (and not a shell in terminal) which can be > used to submit random commands and get command-result object, output string > and errors > 3. IntegratedSecDUnitTest - Tests gfsh data-commands, monitoring commands by > granting/revoking different Roles - DATA_READ/DATA_WRITE/ADMIN/MONITOR, JMX > as well as GemFire cache APIs > 4. MBeanSecurityJUnitTest - Test all mbean operations by granting and > revoking the access levels required for performing that operation > 5. TestAccessControl & TestAuthenticator - In-memory implementation of > GemFire Security Plging with verbose logging used for testing. > > > Diffs > ----- > > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/CommandTestBase.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/HeadlessGfsh.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/HeadlessGfshJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/IntegratedSecDUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/MBeanSecurityJUnitTest.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/ResultHandler.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestAccessControl.java > PRE-CREATION > > gemfire-core/src/test/java/com/gemstone/gemfire/management/internal/security/TestAuthenticator.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/39585/diff/ > > > Testing > ------- > > > File Attachments > ---------------- > > Dunit Patch > > https://reviews.apache.org/media/uploaded/files/2015/10/23/7e17def8-8451-459e-b5fe-56aa753fdc8e__int-sec-dunit.patch > > > Thanks, > > Tushar Khairnar > >
