----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171741 -----------------------------------------------------------
Reviewed the first page of diffs geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java Lines 41 (patched) <https://reviews.apache.org/r/58388/#comment244761> A symbolic constant would be better than a hardcoded "magic number" (i.e. a constant without explanation.) DEFAULT_HTTP_SERVICE_PORT is defined in geode-core/src/main/java/org/apache/geode/distributed/internal/DistributionConfig.java. RestInterfaceJUnitTest defines its own DEFAULT_HTTP_SERVICE_PORT. Consistency would be good. geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java Lines 21 (patched) <https://reviews.apache.org/r/58388/#comment244758> Basic "how to use" javadocs on the Rule classes would be a big help for developing new tests. I think one problem with consistent use of our rules is a lack of understanding how they are intended to be used. Adoption of rules seems to have been somewhat haphazard up to now. This commnent applies to all our rules, not just this one. - Ken Howe On April 12, 2017, 3:08 p.m., Jinmei Liao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58388/ > ----------------------------------------------------------- > > (Updated April 12, 2017, 3:08 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > Repository: geode > > > Description > ------- > > * consolidated the two sets of rules. > * do not allow member start up at test initialization time. > * validate properties in @Before > * use provider in the chained rules to get the appropriate ports in @Before > > Jared and I tried to use ServerLauncher/LocatorLauncher to start the > respective member in the rules, but precheckin yields many failures. Looks > like it has two problems: > 1) by passing in the workingDir in the launcher alone does not make all the > logs go there. I still have to set the user.dir env variable to make some > tests pass. > 2) launcher.stop does not stop the cache/locator/server cleanly. Subsequent > tests fail due to insufficient cleanup. > Due to the above two reasons, I reverted back to use the old way to start > server/locator. This looks like a lean and mean way to get what we needed. > > We also investigated the use of Builder in the rules. At least for now, it > doesn't buy us much since we need to do validation/startup servers in @Before > of the rules. And it yeilds some duplication code and make the usage not that > intuitive. > > As for using AvailablePort.Keeper, Jared and I found out it doesn't really > keep the ports, so revert that back as well. > > > Diffs > ----- > > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java > cbd83e382116dcfc06b0199b69bf91486be22f06 > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java > e93561c0b5fc4fa7f89c03ff4055515d6bbb21be > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestSecurityWithSSLTest.java > 14113c06532d42833ee67f4fbf97ba24d535f2d3 > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/RestServersJUnitTest.java > 2a3a036782fbde22f8969337973ca5794a7172b0 > > geode-assembly/src/test/java/org/apache/geode/rest/internal/web/SwaggerVerificationTest.java > a8ab19c0ac7f5cb94148fff725093c5f07858e93 > > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/EmbeddedPulseRule.java > PRE-CREATION > > geode-assembly/src/test/java/org/apache/geode/test/dunit/rules/HttpClientRule.java > 49351d9bba854eef08c20d47b99eb3ce467cabaa > > geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java > 10ca43b8fc171f75887519deb72700f464f24149 > > geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java > 5a300a15b06b948603b5e82acba1190e1905b41a > > geode-core/src/test/java/org/apache/geode/cache/ConnectionPoolDUnitTest.java > 2ac5120a673ed53f7a8f4b1ed077902566e7865d > geode-core/src/test/java/org/apache/geode/cache/ProxyJUnitTest.java > 05228addb74b070a4eac13bd61e84b055916a503 > > geode-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java > c57ce5beebc7bc4e4d7b826d8795d5a8e367f245 > > geode-core/src/test/java/org/apache/geode/cache/query/BaseLineAndCompareQueryPerfJUnitTest.java > de2f8d3e159ed199c1bd4b740ce7dd8ed5b82a5f > > geode-core/src/test/java/org/apache/geode/cache/query/dunit/QueryIndexDUnitTest.java > d121fe9993e13e07dcea4817a173b80a8342e169 > > geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java > 5f48dae54cd14beb2affeae8a8f3387f3e7bfae0 > > geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexWithSngleFrmAndMultCondQryJUnitTest.java > d5195a6b2c8f1b62955ddd127e28ee74bc63ab21 > > geode-core/src/test/java/org/apache/geode/cache/query/functional/LimitClauseJUnitTest.java > 1661cc8d5ba42b782e08251338605c001c526c97 > > geode-core/src/test/java/org/apache/geode/cache/query/internal/QueryUtilsJUnitTest.java > 85305560669544d5fb3493a88f7bf93d86b8ac91 > > geode-core/src/test/java/org/apache/geode/cache/query/internal/index/IndexMaintenanceJUnitTest.java > b7b590ac2918e3176f2aa9625f4bbe7d06113c87 > > geode-core/src/test/java/org/apache/geode/cache/query/partitioned/PRQueryDUnitHelper.java > e07990d88061d174f2a7a69e4158abb1ed3ac679 > geode-core/src/test/java/org/apache/geode/cache30/CacheLoaderTestCase.java > 164221776bcb187fc5d2d1fe199d40cfd37a0c63 > geode-core/src/test/java/org/apache/geode/cache30/CacheXml66DUnitTest.java > 738ef3f778da58375d495bb1ad294f07161133db > geode-core/src/test/java/org/apache/geode/cache30/DiskRegionDUnitTest.java > c3553b7ece3b91ce142ad5e0acd4ecc1845e9869 > geode-core/src/test/java/org/apache/geode/cache30/DiskRegionTestImpl.java > 301c23267995dd0201566bac9472d4b7db3c60fd > > geode-core/src/test/java/org/apache/geode/cache30/DistributedAckRegionCCEDUnitTest.java > dcb6cf37a5458e40d4638b944c1394fb63a52da2 > > geode-core/src/test/java/org/apache/geode/cache30/DistributedMulticastRegionDUnitTest.java > f22886b7a9b5c89d2023ef5cafbb1332f1fe08b3 > > geode-core/src/test/java/org/apache/geode/cache30/GlobalRegionCCEDUnitTest.java > 6c3f952581cdc2760f6d07ae5e8b7355bc9583c6 > > geode-core/src/test/java/org/apache/geode/cache30/MultiVMRegionTestCase.java > f57bc96f4410ce9925b28370cd125237fbc5a13d > geode-core/src/test/java/org/apache/geode/cache30/RegionTestCase.java > c4cca348cb183ed0613b9a863eea5b5daa7579ab > > geode-core/src/test/java/org/apache/geode/distributed/internal/Bug40751DUnitTest.java > 008c03e8d66332725d7b7b28dfe144c672014f7f > geode-core/src/test/java/org/apache/geode/disttx/DistTXDebugDUnitTest.java > 7415f4070eedfd8ca505dfe8fcc97f8cbc63a616 > > geode-core/src/test/java/org/apache/geode/internal/cache/CacheAdvisorDUnitTest.java > 6ac85d9d0d829d45a67d01649d791942736245aa > > geode-core/src/test/java/org/apache/geode/internal/cache/DeltaSizingDUnitTest.java > a879b774a1f670a057b0ad4f37b93151f162ec6b > > geode-core/src/test/java/org/apache/geode/internal/cache/EventTrackerDUnitTest.java > 219949bc7577cc1a0a507f97b2983e6a6863fe21 > geode-core/src/test/java/org/apache/geode/internal/cache/PRTXJUnitTest.java > 1bca936ea40529873cc3839f21438dbcf2633c6a > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionListenerDUnitTest.java > 9c389483df3bd31e4bbbe8ea1cc1de0729ff55d3 > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionHADUnitTest.java > 2b3a5a95ded7758be50eb9a74cfc96e31a8929f8 > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionRedundancyZoneDUnitTest.java > 404bf27720397962761694e82cf1702c81f0fc2f > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionStatsDUnitTest.java > ce09d11f49fb0e825dace186bbd5ebff57ac7cd7 > > geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestUtilsDUnitTest.java > 358bece025100dc379a2a89bb091fe94ec3f90b3 > > geode-core/src/test/java/org/apache/geode/internal/cache/control/RebalanceOperationDUnitTest.java > 694fc61699a207736df4976517aab9c2db0625fa > > geode-core/src/test/java/org/apache/geode/internal/cache/ha/HARQueueNewImplDUnitTest.java > 83f3b3f2fa0509e2d224dc2e15b485221ccef9ab > > geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/Bug39356DUnitTest.java > 1d412914d67e0060a8ec73a1fe44fe71d3aaa0a7 > > geode-core/src/test/java/org/apache/geode/internal/cache/partitioned/ShutdownAllDUnitTest.java > 2cdd23bd3a81581fd3dca82b96ecd3e21a0b5e91 > > geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgrade2DUnitTest.java > 6d964716a4074e6e328ca0eedc13323fed3db7bc > > geode-core/src/test/java/org/apache/geode/internal/cache/rollingupgrade/RollingUpgradeDUnitTest.java > 0fffb130c72db5374dc9ea67e88588222fbaf2a2 > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/Bug36829DUnitTest.java > d90d41d2ae6f16cdfd4868376fd238125d5532bb > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/DataSerializerPropogationDUnitTest.java > 4a67e384c16a140198e873b2acc329a2141556b7 > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/HAStartupAndFailoverDUnitTest.java > 6aea5096cdd08ec477848366d5b0e274443334d0 > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/InstantiatorPropagationDUnitTest.java > 698f79525d20db0d6f8a81553405c7c0494c809e > > geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/RedundancyLevelTestBase.java > 4a9829869b5d36ead3ea8714354d3ba202e66249 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/commands/ExportLogsIntegrationTest.java > 9a39298a21a4593e8939ddfcac11664508d20df3 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/functions/ExportLogsFunctionIntegrationTest.java > af9a1e60cde38e5f358878d86ef8bf7d99beed59 > > geode-core/src/test/java/org/apache/geode/management/internal/cli/shell/GfshMultilineCommandTest.java > 0d33058961930207120bd990453e9a89f5c783aa > > geode-core/src/test/java/org/apache/geode/management/internal/cli/util/LogExporterIntegrationTest.java > a831ac1e649e97cd2cf659def3151e190665b2dc > > geode-core/src/test/java/org/apache/geode/management/internal/security/AccessControlMBeanJUnitTest.java > d992e9351c2075fea9c2a4d0cc03d75573466d2b > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthenticationJUnitTest.java > 97436e01076bdde71f76e0b3b9ad71d8b293303b > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanAuthorizationJUnitTest.java > c9e35531ac4d8a3221650ef805dffc35124d6e99 > > geode-core/src/test/java/org/apache/geode/management/internal/security/CacheServerMBeanShiroJUnitTest.java > 91efeace6a5f7adc3b27d959f958807523a8beae > > geode-core/src/test/java/org/apache/geode/management/internal/security/CliCommandsSecurityTest.java > f1321639a392490003a3bc93dfbe3c684b2b9e5a > > geode-core/src/test/java/org/apache/geode/management/internal/security/DataCommandsSecurityTest.java > f6b5a2e04d14e311d5a94a9c8c2e45b1383a3db5 > > geode-core/src/test/java/org/apache/geode/management/internal/security/DeployCommandsSecurityTest.java > b6190320811b282f03368278f5086c0cafad67f0 > > geode-core/src/test/java/org/apache/geode/management/internal/security/DiskStoreMXBeanSecurityJUnitTest.java > feb00d4507dc37df0d7bcc3b4767a6e77b367b89 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GatewayReceiverMBeanSecurityTest.java > e7fe1d6b1a51466e3570ba51667848f9a30b3dca > > geode-core/src/test/java/org/apache/geode/management/internal/security/GatewaySenderMBeanSecurityTest.java > 67cb1ccefd9800c7d5e979fcee7b00a90f499a43 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsPostProcessorTest.java > 6f2754f46c703074661b89a8005531c5c0a00b43 > > geode-core/src/test/java/org/apache/geode/management/internal/security/GfshCommandsSecurityTest.java > 3fe83144077d829473dbe48c555ab3be5bf67550 > > geode-core/src/test/java/org/apache/geode/management/internal/security/JavaRmiServerNameTest.java > 66e2702d5a10ba7b7ba5aa4652476a58da91c8c8 > > geode-core/src/test/java/org/apache/geode/management/internal/security/LockServiceMBeanAuthorizationJUnitTest.java > d2f0cae36d6bd8bb00615275942d764bcf6d7a8f > > geode-core/src/test/java/org/apache/geode/management/internal/security/MBeanSecurityJUnitTest.java > 9dcba9465b64f55a190494cc405b6c073228a707 > > geode-core/src/test/java/org/apache/geode/management/internal/security/ManagerMBeanAuthorizationJUnitTest.java > b7447c9216a401ea44e1624407ec86678265d739 > > geode-core/src/test/java/org/apache/geode/management/internal/security/MemberMBeanSecurityJUnitTest.java > 11e202637817d8718ec42ea96730c695974cb907 > geode-core/src/test/java/org/apache/geode/security/ClientAuthDUnitTest.java > 76efae50f7e5d01668481c672e1282ba2e3aa038 > > geode-core/src/test/java/org/apache/geode/security/ClientContainsKeyAuthDUnitTest.java > 0366673df77d7db70bc9370f478adace4315a686 > > geode-core/src/test/java/org/apache/geode/security/ClientDestroyInvalidateAuthDUnitTest.java > 841d98eba9d531d881eb641d5bbff3f7d83c0e42 > > geode-core/src/test/java/org/apache/geode/security/ClientDestroyRegionAuthDUnitTest.java > 5ea65d80be6eeceffeba4832de8ea84a57291a96 > > geode-core/src/test/java/org/apache/geode/security/ClientExecuteFunctionAuthDUnitTest.java > 2b4c27c335346a88fbbd1622d7975c21dfbfae56 > > geode-core/src/test/java/org/apache/geode/security/ClientExecuteRegionFunctionAuthDUnitTest.java > a018f4ccaa8d31765cedff9bd167af734345f68c > > geode-core/src/test/java/org/apache/geode/security/ClientGetAllAuthDUnitTest.java > 92eda186ea609c41f7942464ccc7c7628a742d6b > > geode-core/src/test/java/org/apache/geode/security/ClientGetEntryAuthDUnitTest.java > fad77f59fd46c2051a340f61c2468f2b8661ea46 > > geode-core/src/test/java/org/apache/geode/security/ClientGetPutAuthDUnitTest.java > a816a125b73bc55761208c0e9a2d0724c42d7eb7 > > geode-core/src/test/java/org/apache/geode/security/ClientRegionClearAuthDUnitTest.java > 99a77b679bd23e7b567dcc2905bbd66cd89a5000 > > geode-core/src/test/java/org/apache/geode/security/ClientRegisterInterestAuthDUnitTest.java > 9c24d42f2998c82d10b1073fb8b407e8131efec8 > > geode-core/src/test/java/org/apache/geode/security/ClientRemoveAllAuthDUnitTest.java > 44887d974f1309732fcc64030e6c147eeccc674e > > geode-core/src/test/java/org/apache/geode/security/ClientUnregisterInterestAuthDUnitTest.java > e62aa6ab7d36149c69b368256ebe687e5406aeb8 > > geode-core/src/test/java/org/apache/geode/security/NoShowValue1PostProcessorDUnitTest.java > c6b2735a7d488fd802350c8ec62b1d1b95f50084 > > geode-core/src/test/java/org/apache/geode/security/PDXPostProcessorDUnitTest.java > c735313adf1a2c5c0b001ed36712addf48624ecc > > geode-core/src/test/java/org/apache/geode/security/PostProcessorDUnitTest.java > 89a675246536a64b4c0ab5be267e77a12ae8da54 > > geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java > 4f84f7b7fda8968973f043b153085d9e475e4e92 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/GfshShellConnectionRule.java > 381b7515c48b82cdd231d54323b0b6db3228fc41 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocalLocatorStarterRule.java > 21be585e648b3f25944f542499837727c45f5aa7 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocalServerStarterRule.java > 3106948196b39500a795c106d1cfb14cbc30dd80 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerStartupRule.java > 7350241538112c9bc1dfa8e4dfc9a82f18de8033 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterBuilder.java > 169d41e0730aa3b4874bf105c9d495db7aba0387 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorStarterRule.java > 097adb787422aa0c7741ddc147a6d1a8c28f6c03 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MBeanServerConnectionRule.java > 9c6f81e2bdeb5078330cecc41493522feefccd11 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/MemberStarterRule.java > 3fda85a4814f87995e29aea0ce2c64baad86ac7a > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterBuilder.java > 5d8947c7a1a635865cf61b31972df5abf56f8430 > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/ServerStarterRule.java > fea2e9db1e57727336edd486aeddff884d481c3b > > geode-core/src/test/java/org/apache/geode/test/dunit/rules/test/MemberStarterRuleTest.java > PRE-CREATION > > geode-cq/src/test/java/org/apache/geode/cache/query/dunit/QueryMonitorDUnitTest.java > 49d7c28663c48581c7b92628351d35479e9e92ee > > geode-cq/src/test/java/org/apache/geode/internal/cache/ha/CQListGIIDUnitTest.java > 7a100467ba45a743d2e30de9c4a89ff5318d8fdd > > geode-cq/src/test/java/org/apache/geode/management/CacheServerManagementDUnitTest.java > ae5570f994c26fde82b940ed1fa14809df714acb > geode-cq/src/test/java/org/apache/geode/security/CQClientAuthDUnitTest.java > 68e4c69fd218b9af0b94473c766a3b6063b09d0e > > geode-cq/src/test/java/org/apache/geode/security/CQPDXPostProcessorDUnitTest.java > 86d467107a4d0fe1ede6059bde7e1d95139de505 > > geode-cq/src/test/java/org/apache/geode/security/CQPostProcessorDunitTest.java > 935c48c6b6d2f9c36aff59a1fdc3bdcf6e0a5a2a > > geode-cq/src/test/java/org/apache/geode/security/ClientQueryAuthDUnitTest.java > aea543d4d2a539a2f3f1fa66c897c9ce863a98a1 > > geode-web/src/test/java/org/apache/geode/management/internal/cli/commands/QueryNamesOverHttpDUnitTest.java > 86deb27f8094e8fc2fd5968ffd2705fdb7a6b890 > > geode-web/src/test/java/org/apache/geode/management/internal/security/GfshCommandsOverHttpSecurityTest.java > 7fd839c967314e20897b305270de9b72f27149f3 > > > Diff: https://reviews.apache.org/r/58388/diff/2/ > > > Testing > ------- > > precheckin successful. > > > Thanks, > > Jinmei Liao > >