You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/06/03 11:38:26 UTC
[GitHub] [pulsar] lhotari opened a new pull request, #15924: [CPP tests] Wait for mockZTS to start before connecting to it
lhotari opened a new pull request, #15924:
URL: https://github.com/apache/pulsar/pull/15924
Fixes #6298
### Motivation
There's a long outstanding flaky CPP test AuthPluginTest.testAthenz which causes CPP builds to fail frequently.
There's a race condition in the test. mockZTS might not be started. Error message is
`ERROR [140642816536256] ZTSClient:367 | Response failed for url http://localhost:9999/zts/v1/domain/pulsar.test.provider/token. Error Code 7` .
Error Code 7 is `CURLE_COULDNT_CONNECT (7) - Failed to connect() to host or proxy.` [in libcurl error codes](https://curl.se/libcurl/c/libcurl-errors.html).
### Modifications
Wait for mockZTS to start before connecting to it.
--
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
[GitHub] [pulsar] lhotari commented on pull request #15924: [CPP tests] Wait for mockZTS to start before connecting to it
Posted by GitBox <gi...@apache.org>.
lhotari commented on PR #15924:
URL: https://github.com/apache/pulsar/pull/15924#issuecomment-1146242922
@merlimat That's for the review and suggestions. I applied the changes. PTAL
--
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
[GitHub] [pulsar] merlimat commented on a diff in pull request #15924: [CPP tests] Wait for mockZTS to start before connecting to it
Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #15924:
URL: https://github.com/apache/pulsar/pull/15924#discussion_r889205621
##########
pulsar-client-cpp/tests/AuthPluginTest.cc:
##########
@@ -226,7 +228,8 @@ void mockZTS(int port) {
} // namespace testAthenz
TEST(AuthPluginTest, testAthenz) {
- std::thread zts(std::bind(&testAthenz::mockZTS, 9999));
+ Latch latch(1);
+ std::thread zts(std::bind(&testAthenz::mockZTS, &latch, 9999));
Review Comment:
```suggestion
std::thread zts(std::bind(&testAthenz::mockZTS, std::ref(latch), 9999));
```
##########
pulsar-client-cpp/tests/AuthPluginTest.cc:
##########
@@ -190,14 +191,15 @@ TEST(AuthPluginTest, testTlsDetectHttpsWithHostNameValidation) {
namespace testAthenz {
std::string principalToken;
-void mockZTS(int port) {
+void mockZTS(Latch* latch, int port) {
Review Comment:
Passing pointers around can result ambiguous on on the ownership of the object. We should use a reference instead:
```suggestion
void mockZTS(Latch& latch, int port) {
```
##########
pulsar-client-cpp/tests/AuthPluginTest.cc:
##########
@@ -296,7 +300,8 @@ TEST(AuthPluginTest, testAuthFactoryTls) {
}
TEST(AuthPluginTest, testAuthFactoryAthenz) {
- std::thread zts(std::bind(&testAthenz::mockZTS, 9998));
+ Latch latch(1);
+ std::thread zts(std::bind(&testAthenz::mockZTS, &latch, 9998));
Review Comment:
```suggestion
std::thread zts(std::bind(&testAthenz::mockZTS, std::ref(latch), 9998));
```
--
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
[GitHub] [pulsar] lhotari commented on a diff in pull request #15924: [CPP tests] Wait for mockZTS to start before connecting to it
Posted by GitBox <gi...@apache.org>.
lhotari commented on code in PR #15924:
URL: https://github.com/apache/pulsar/pull/15924#discussion_r889221187
##########
pulsar-client-cpp/tests/AuthPluginTest.cc:
##########
@@ -190,14 +191,15 @@ TEST(AuthPluginTest, testTlsDetectHttpsWithHostNameValidation) {
namespace testAthenz {
std::string principalToken;
-void mockZTS(int port) {
+void mockZTS(Latch* latch, int port) {
LOG_INFO("-- MockZTS started");
boost::asio::io_service io;
boost::asio::ip::tcp::iostream stream;
boost::asio::ip::tcp::acceptor acceptor(io,
boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port));
LOG_INFO("-- MockZTS waiting for connnection");
+ latch->countdown();
Review Comment:
```suggestion
latch.countdown();
```
--
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
[GitHub] [pulsar] lhotari merged pull request #15924: [CPP tests] Wait for mockZTS to start before connecting to it
Posted by GitBox <gi...@apache.org>.
lhotari merged PR #15924:
URL: https://github.com/apache/pulsar/pull/15924
--
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