----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58388/#review171919 -----------------------------------------------------------
I'm finding lots of instances where "withRegion" was changed to "createRegion" but it's an API or Util class that isn't related to your Rule changes. You probably don't want to change all of these instances of "withRegion" -- rather than keep flagging more of these, I'll send the review as is. geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseDataExportTest.java Line 42 (original), 41 (patched) <https://reviews.apache.org/r/58388/#comment244928> Does server.getHttpPort() require that Rule before was already invoked on server? If so then we need RuleChain to guarantee ordering. If construcing ServerStarterRule provides a HttpPort, then it should be ok like this. geode-core/src/test/java/org/apache/geode/cache/ConnectionPoolDUnitTest.java Line 5043 (original), 5043 (patched) <https://reviews.apache.org/r/58388/#comment244929> Can we just delete the dead code in this test? geode-core/src/test/java/org/apache/geode/cache/ProxyJUnitTest.java Line 1034 (original), 1034 (patched) <https://reviews.apache.org/r/58388/#comment244930> Can we just delete the dead code in this test? geode-core/src/test/java/org/apache/geode/cache/partition/PartitionRegionHelperDUnitTest.java Line 77 (original), 77 (patched) <https://reviews.apache.org/r/58388/#comment244931> Did you mean to make these changes? These are just the string names for the SerializableRunnables. geode-core/src/test/java/org/apache/geode/cache/query/BaseLineAndCompareQueryPerfJUnitTest.java Line 321 (original), 321 (patched) <https://reviews.apache.org/r/58388/#comment244935> This looks like someone defined a colocation API "withRegion" and may be an existing API on one of the Cache interfaces or the implementations. You probably don't want to change these instances of "withRegion" geode-core/src/test/java/org/apache/geode/cache/query/dunit/RemoteQueryDUnitTest.java Line 524 (original), 524 (patched) <https://reviews.apache.org/r/58388/#comment244936> These are more instances of the withRegion API. geode-core/src/test/java/org/apache/geode/cache/query/functional/IndexWithSngleFrmAndMultCondQryJUnitTest.java Line 641 (original), 641 (patched) <https://reviews.apache.org/r/58388/#comment244937> Another withRegion API you probably don't want to change. - Kirk Lund On April 12, 2017, 11:32 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, 11:32 p.m.) > > > Review request for geode, Jared Stewart, Ken Howe, Kirk Lund, and Patrick > Rhomberg. > > > Repository: geode > > > Description > ------- > > Finally the reviewboard is behaving now. This is good for review now. I will > close the PR. > > * 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 > 3c53dfed1c1fbf8a5cb83eaac16f5bfc96d44a0a > > 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 > 02bad30e97f0a3550eac0bf00ce0c6833d761dae > > 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/3/ > > > Testing > ------- > > precheckin successful. > > > Thanks, > > Jinmei Liao > >
