chenBright commented on code in PR #3568:
URL: https://github.com/apache/brpc/pull/3568#discussion_r4101200961
##########
test/brpc_naming_service_unittest.cpp:
##########
@@ -665,8 +673,15 @@ TEST(NamingServiceTest, discovery_sanity) {
"/discovery/cancel => Cancel";
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE,
rest_mapping.c_str()));
- ASSERT_EQ(0, server.Start("localhost:8635", nullptr));
-
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ brpc::policy::FLAGS_discovery_api_addr = butil::string_printf(
+ "http://%s/discovery/nodes";, butil::endpoint2str(
+ server.listen_address()).c_str());
+
+ const std::string server_address =
+ butil::endpoint2str(server.listen_address()).c_str();
+ s_nodes_result.replace(s_nodes_result.find("127.0.0.1:8635"),
+ strlen("127.0.0.1:8635"), server_address);
Review Comment:
Fixed in 139cad99f96f5151d6cdf58af747db4036413c6a . The nodes response is
now an immutable template. The test creates a local copy, verifies that the
placeholder is present, substitutes the dynamic endpoint in that copy, and
passes it to the mock service.
##########
test/brpc_naming_service_unittest.cpp:
##########
@@ -665,8 +673,15 @@ TEST(NamingServiceTest, discovery_sanity) {
"/discovery/cancel => Cancel";
ASSERT_EQ(0, server.AddService(&svc, brpc::SERVER_DOESNT_OWN_SERVICE,
rest_mapping.c_str()));
- ASSERT_EQ(0, server.Start("localhost:8635", nullptr));
-
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ brpc::policy::FLAGS_discovery_api_addr = butil::string_printf(
+ "http://%s/discovery/nodes";, butil::endpoint2str(
+ server.listen_address()).c_str());
Review Comment:
Fixed in 139cad99f96f5151d6cdf58af747db4036413c6a . Added
`ResetDiscoveryChannelForTesting()` and reset the process-wide Discovery
channel before and after the test. The Discovery flags are also restored with
RAII guards.
##########
test/brpc_naming_service_unittest.cpp:
##########
@@ -442,15 +449,17 @@ TEST(NamingServiceTest, consul_with_backup_file) {
brpc::Server server;
ConsulNamingServiceImpl svc;
- std::string restful_map(brpc::policy::FLAGS_consul_service_discovery_url);
- restful_map.append("/");
+ std::string restful_map("/v1/health/service/");
restful_map.append(service_name);
restful_map.append(" => ListNames");
ASSERT_EQ(0, server.AddService(&svc,
brpc::SERVER_DOESNT_OWN_SERVICE,
restful_map.c_str()));
- ASSERT_EQ(0, server.Start("localhost:8500", nullptr));
+ ASSERT_EQ(0, server.Start(0, nullptr));
+ brpc::policy::FLAGS_consul_agent_addr = butil::string_printf(
+ "http://%s";, butil::endpoint2str(server.listen_address()).c_str());
Review Comment:
Fixed in 139cad99f96f5151d6cdf58af747db4036413c6a . The test now uses RAII
guards to restore `consul_agent_addr`,
`consul_enable_degrade_to_file_naming_service`, and `health_check_interval`,
including on early assertion returns.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
