[GitHub] [pulsar] BewareMyPower edited a comment on pull request #11571: [C++] Fix testConnectTimeout test by shutting down clients

[GitBox <gi...@apache.org>]

BewareMyPower edited a comment on pull request #11571:
URL: https://github.com/apache/pulsar/pull/11571#issuecomment-893563738


   The tests still failed:
   
   ```
       1522 ms: ./main ClientTest.testConnectTimeout (try #1)
       1522 ms: ./main ClientTest.testConnectTimeout (try #2)
   ```
   
   See https://github.com/apache/pulsar/pull/11571/checks?check_run_id=3252289848
   
   The `testConnectTimeout` failed not because `shutdown()` is not called. It's because `connectTimeoutTask_ ` started again even if the the next endpoint iterator is the end iterator, see https://github.com/apache/pulsar/blob/27dcb0a6febd0d88438e18fae72aad8ee38c0738/pulsar-client-cpp/lib/ClientConnection.cc#L435-L443
   
   We should increase `endpointIterator` first and then check `if (endpointIterator != tcp::resolver::iterator())`.
   
   To verify my point, you can change
   
   ```c++
   ASSERT_EQ(futureLow.wait_for(std::chrono::milliseconds(1500)), std::future_status::ready);
   ```
   
   to
   
   ```c++
   ASSERT_EQ(futureLow.wait_for(std::chrono::milliseconds(2500)), std::future_status::ready);
   ```
   
   The tests will pass.
   
   BTW, current CI for C++ client is broken (see https://github.com/apache/pulsar/issues/11549). And I've fixed all failed C++ tests in https://github.com/apache/pulsar/pull/11557.


-- 
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: commits-unsubscribe@pulsar.apache.org

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