You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/12/08 21:30:46 UTC

[GitHub] [geode] demery-pivotal opened a new pull request #7176: GEODE-9872: Make test framework code assign ports

demery-pivotal opened a new pull request #7176:
URL: https://github.com/apache/geode/pull/7176


   PROBLEM
   
   `DistTXPersistentDebugDUnitTest ` failed in CI because it accidentally
   connected to a locator from another test
   (`ClusterConfigLocatorRestartDUnitTest`).
   
   CAUSE
   
   `ClusterConfigLocatorRestartDUnitTest` attempts to restart a
   locator on a port in the ephemeral port range.
   
   Here is the sequence of events:
   1. `ClusterConfigLocatorRestartDUnitTest ` started a locator on an
      ephemeral port. In this CI run it got port 37877.
   2. `ClusterConfigLocatorRestartDUnitTest` stopped the locator on port
      37877.
   3. `DistTXPersistentDebugDUnitTest` started a locator on an ephemeral
      port. In this CI run it got 37877.
   4. `ClusterConfigLocatorRestartDUnitTest ` attempted to restart the
      locator on port 37877. That port was already in use in
      `DistTXPersistentDebugDUnitTest`'s locator, and as a result the two
      tests became entangled.
   
   CONTRIBUTING FACTORS
   
   `DistTXPersistentDebugDUnitTest` uses `DUnitLauncher` to start its
   locator. By default, `DUnitLauncher` starts its locator on an ephemeral
   port.
   
   `ClusterConfigLocatorRestartDUnitTest` uses `ClusterStartupRule` to
   start several locators. By default, `ClusterStartupRule` starts each
   locator on an ephemeral port.
   
   SOLUTION
   
   Change `DUnitLauncher` and `ClusterStartupRule` to assign locator ports
   via `AvailablePortHelper` if the test does not specify a particular
   port.
   
   I considered changing only `ClusterConfigLogatorRestartDUnitTest` to
   assign the port that it intends to reuse. But:
   - That would fix only this one test, though an unknown number of tests
     similarly attempt to reuse ports assigned by framework code. Numerous
     of those tests have already been changed to assign ports explicitly,
     but an unknown number remain.
   - It is quite reasonable for this test and others to assume that, if the
     test framework assigns a port on the test's behalf, then the test will
     enjoy exclusive use of that port for the entire life of the test. I
     think the key problem is not that tests make this assumption, but that
     the framework code violates it.
   
   Changing the test framework classes that tacitly assign ports
   (`DUnitLauncher` and `ClusterStartupRule`) makes them behave in a way
   that tests expect.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] demery-pivotal edited a comment on pull request #7176: GEODE-9872: Make test framework code assign ports

Posted by GitBox <gi...@apache.org>.

demery-pivotal edited a comment on pull request #7176:
URL: https://github.com/apache/geode/pull/7176#issuecomment-989329878


   The upgrade test failure is a known failure. See https://issues.apache.org/jira/browse/GEODE-9622. It's similarly caused by misuse of ephemeral ports. But in this failure, the misuse is in the test itself, not in the framework.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] demery-pivotal commented on pull request #7176: GEODE-9872: Make test framework code assign ports

Posted by GitBox <gi...@apache.org>.

demery-pivotal commented on pull request #7176:
URL: https://github.com/apache/geode/pull/7176#issuecomment-989329878


   The upgrade test failure is a known failure. See [https://issues.apache.org/jira/browse/GEODE-9622](Geode 9622). It's similarly caused by misuse of ephemeral ports. But in this failure, the misuse is in the test itself, not in the framework.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [geode] demery-pivotal merged pull request #7176: GEODE-9872: Make test framework code assign ports

Posted by GitBox <gi...@apache.org>.

demery-pivotal merged pull request #7176:
URL: https://github.com/apache/geode/pull/7176


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org