[geode] 01/01: GEODE-5407: fix JMXMBeanReconnectDUnitTest timing (#2833)

[he...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

heybales pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit f873d3482e06e883dd560e90ed1de1a4cbc06325
Merge: d80b6e0 065d11c
Author: Helena Bales <hb...@pivotal.io>
AuthorDate: Wed Nov 28 09:25:31 2018 -0800

    GEODE-5407: fix JMXMBeanReconnectDUnitTest timing (#2833)
    
    * GEODE-5407: fix JMXMBeanReconnectDUnitTest timing
    
    This test class had several timing isses for the remote MBean tests.
    This PR fixes all known timing issues, which were MBean inconsistencies
    in the before() and trouble reconnecting stopped members, as well as
    insufficiently strict assertions (comparing lists with
    .containsExactlyElementsOf() is not commutative). This commit also
    changes MemberVM's forceDisconnect to use a blackboard mailbox instead
    of a fixed amount of time to decid when to start reconnecting. It also
    adds functionality to the ConcurrencyRule to act as an awaitility.
    
    * In waitForLocatorsToAgreeOnMembership, called from the before, the
    MBeans that each locator has were only compared by list size. Now they
    are compared element by element, and the sizes are checked to be
    correct.
    
    * In waitForLocatorToAgreeOnMembership, the wait was returning
    prematurely after the first pass due to Awaitlity.untilAsserted not
    working correctly. For this method, a simple implementation using
    assertions and Awaitility caused memory issues, so the current
    implementation uses do/while loops, compares the lists element by
    element, and starts a single thread that is allowed to run for 300
    seconds.
    
    * In the tests, the timing was not strictly enforced. The disconnect and
    starting the reconnect now happen in different threads. The thread that
    starts the reconnect checks that the disconnect has happened, gathers
    relevant information, then signals that reconnect can start, which the
    reconnect thread now checks for. This ensures that we don't gather
    MBeans before the member has fully stopped, and that reconnect does not
    start until we've gathered the MBeans.
    
    * Assertions have been made more strict by checking that lists against
    each other twice, since Assertions.containsExactlyElementsOf() is not
    commutative.
    
    * Use a blackboard entry to check if the stopped member should start
    reconnecting in the reconnect listener attached from MemberVM when
    forcing a member to disconnect.
    
    * Add functionality to ConcurrencyRule to act as an awaitility,
    expecting a value to eventually be true. This was added during the
    process of trying to solve this bug but is not used in the final
    implementation.
    
    * fail the test when waits timeout
    
    * create a single connection per locator and reuse it
    
    * make suggested changes from PR comments

 .../management/JMXMBeanReconnectDUnitTest.java     | 306 +++++++++++----------
 .../apache/geode/test/dunit/rules/MemberVM.java    |  66 +++--
 .../geode/test/junit/rules/ConcurrencyRule.java    |  40 ++-
 .../test/junit/rules/ConcurrencyRuleTest.java      |  47 +++-
 4 files changed, 280 insertions(+), 179 deletions(-)