This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new 469eff5f4 KUDU-1457 [8/n] Parameterize tests with ip config
469eff5f4 is described below
commit 469eff5f42f5e44e8950d9060c5203192c5d2220
Author: Ashwani Raina <[email protected]>
AuthorDate: Sat Nov 22 00:33:49 2025 +0530
KUDU-1457 [8/n] Parameterize tests with ip config
This patch enables IP based tests to take constants instead of strings
for paramterization, thereby, adding type-safety and improved compile
time checks. Convert parameter type to readable string to improve
readability. Add helper function to convert IPMode value to its
corresponding string value that can be directly used to set the
ip_config_mode flag.
Change-Id: Ic72efea2045b12300ccf0e80873c405828c15a30
Reviewed-on: http://gerrit.cloudera.org:8080/23707
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Alexey Serbin <[email protected]>
---
src/kudu/master/mini_master-test.cc | 20 +++---
src/kudu/server/webserver-test.cc | 124 +++++++++++++++++++-----------------
src/kudu/util/net/net_util-test.cc | 6 +-
src/kudu/util/net/net_util.cc | 11 ++++
src/kudu/util/net/net_util.h | 5 ++
src/kudu/util/test_util.cc | 6 ++
src/kudu/util/test_util.h | 8 ++-
7 files changed, 106 insertions(+), 74 deletions(-)
diff --git a/src/kudu/master/mini_master-test.cc
b/src/kudu/master/mini_master-test.cc
index 66c6acba7..24a0b870b 100644
--- a/src/kudu/master/mini_master-test.cc
+++ b/src/kudu/master/mini_master-test.cc
@@ -20,7 +20,6 @@
#include <vector>
#include <gflags/gflags_declare.h>
-#include <glog/logging.h>
#include <gtest/gtest.h>
#include "kudu/fs/fs_manager.h"
@@ -29,29 +28,28 @@
#include "kudu/rpc/rpc-test-base.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/path_util.h"
-#include "kudu/util/status.h"
#include "kudu/util/test_macros.h"
#include "kudu/util/test_util.h"
DECLARE_string(ip_config_mode);
namespace kudu {
-namespace master {
using std::string;
using std::unique_ptr;
+namespace master {
+
class MiniMasterTest : public KuduTest,
- public ::testing::WithParamInterface<string> {
+ public ::testing::WithParamInterface<IPMode> {
public:
MiniMasterTest() {
- FLAGS_ip_config_mode = GetParam();
+ mode_ = GetParam();
+ FLAGS_ip_config_mode = IPModeToString(mode_);
}
protected:
- static string get_host() {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- switch (mode) {
+ string get_host() {
+ switch (mode_) {
case IPMode::IPV6:
return "::1";
case IPMode::DUAL:
@@ -60,11 +58,13 @@ class MiniMasterTest : public KuduTest,
return "127.0.0.1";
}
}
+
+ IPMode mode_;
};
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, MiniMasterTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(MiniMasterTest, TestMultiDirMaster) {
SKIP_IF_HOSTNAME_RESOLUTION_FAILURE_EXPECTED();
diff --git a/src/kudu/server/webserver-test.cc
b/src/kudu/server/webserver-test.cc
index 5c28857ac..64b0b0a14 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -80,6 +80,9 @@ TAG_FLAG(test_sensitive_flag, sensitive);
namespace kudu {
+constexpr char kIpv6Localhost[] = "[::1]";
+constexpr char kIpDual[] = "[::]";
+
const bool kIsFips = security::IsFIPSEnabled();
namespace {
@@ -97,29 +100,50 @@ void SetHTPasswdOptions(WebserverOptions* opts) {
&opts->password_file));
}
+string GetWebserverInterface(IPMode mode) {
+ switch (mode) {
+ case IPMode::IPV6:
+ return kIpv6Localhost;
+ case IPMode::DUAL:
+ return kIpDual;
+ case IPMode::IPV4:
+ default:
+ // Return default webserver interface.
+ return FLAGS_webserver_interface;
+ }
+}
+
} // anonymous namespace
class WebserverTest : public KuduTest,
- public ::testing::WithParamInterface<std::string> {
+ public ::testing::WithParamInterface<IPMode> {
public:
- WebserverTest() {
+ WebserverTest()
+ : mode_(GetParam()) {
static_dir_ = GetTestPath("webserver-docroot");
CHECK_OK(env_->CreateDir(static_dir_));
- FLAGS_ip_config_mode = GetParam();
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode_));
+ FLAGS_ip_config_mode = IPModeToString(mode_);
if (mode_ == IPMode::DUAL || mode_ == IPMode::IPV6) {
// The wildcard address is applicable to both 'IPV6' as well as 'DUAL'
modes.
// For 'IPV6' mode, IPV6_V6ONLY is enabled on server socket option that
ensures
// IPv4 connections are rejected by the server.
FLAGS_webserver_interface = "[::]";
}
- }
+}
void SetUp() override {
KuduTest::SetUp();
- FLAGS_ip_config_mode = GetParam();
WebserverOptions opts;
+ switch (mode_) {
+ case IPMode::IPV6:
+ case IPMode::DUAL:
+ opts.bind_interface = "[::]";
+ break;
+ default:
+ // Default value taken from FLAGS_webserver_interface.
+ break;
+ }
opts.port = 0;
opts.doc_root = static_dir_;
opts.enable_doc_root = enable_doc_root();
@@ -189,7 +213,7 @@ class WebserverTest : public KuduTest,
string url_;
string static_dir_;
string cert_path_;
- IPMode mode_;
+ const IPMode mode_;
};
class SslWebserverTest : public WebserverTest {
@@ -209,7 +233,7 @@ class PasswdWebserverTest : public WebserverTest {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, PasswdWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
// Send a HTTP request with no username and password. It should reject
// the request as the .htpasswd is presented to webserver.
@@ -309,7 +333,7 @@ class SpnegoDedicatedKeytabWebserverTest : public
SpnegoWebserverTest {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, SpnegoDedicatedKeytabWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(SpnegoDedicatedKeytabWebserverTest, TestAuthenticated) {
ASSERT_OK(kdc_->Kinit("alice"));
@@ -320,7 +344,7 @@ TEST_P(SpnegoDedicatedKeytabWebserverTest,
TestAuthenticated) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, SpnegoWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
// Tests that execute DoSpnegoCurl() are ignored in MacOS (except the first
test case)
// MacOS heimdal kerberos caches kdc port number somewhere so that all the
test cases
@@ -446,7 +470,7 @@ TEST_P(SpnegoWebserverTest, TestAuthNotRequiredForOptions) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, WebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(WebserverTest, TestIndexPage) {
curl_.set_return_headers(true);
@@ -547,7 +571,7 @@ TEST_P(WebserverTest, TestHttpCompression) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, SslWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(SslWebserverTest, TestSSL) {
// We use a self-signed cert, so we have to trust it manually.
@@ -560,7 +584,7 @@ TEST_P(SslWebserverTest, TestSSL) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, Tls13WebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(Tls13WebserverTest, TestTlsMinVersion) {
FLAGS_trusted_certificate_file = cert_path_;
@@ -765,7 +789,7 @@ class NoAuthnWebserverTest : public WebserverTest {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, NoAuthnWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(NoAuthnWebserverTest, TestUnauthenticatedUser) {
ASSERT_OK(curl_.FetchURL(Substitute("$0/authn", url_), &buf_));
@@ -774,7 +798,7 @@ TEST_P(NoAuthnWebserverTest, TestUnauthenticatedUser) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, AuthnWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
// The following tests are skipped on macOS due to inconsistent behavior of
SPNEGO.
// macOS heimdal kerberos caches the KDC port number, which can cause
subsequent tests to fail.
@@ -851,7 +875,7 @@ class PathParamWebserverTest : public WebserverTest {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, PathParamWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(PathParamWebserverTest, TestPathParameterAtEnd) {
ASSERT_OK(
@@ -914,7 +938,7 @@ class DisabledDocRootWebserverTest : public WebserverTest {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, DisabledDocRootWebserverTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(DisabledDocRootWebserverTest, TestHandlerNotFound) {
Status s = curl_.FetchURL(Substitute("$0/foo", url_), &buf_);
@@ -935,8 +959,14 @@ TEST_P(WebserverTest, TestConnectionReuse) {
ASSERT_EQ(0, curl_.num_connects());
}
-class WebserverAdvertisedAddressesTest : public KuduTest {
+class WebserverAdvertisedAddressesTest : public KuduTest,
+ public
::testing::WithParamInterface<IPMode> {
public:
+ WebserverAdvertisedAddressesTest()
+ : mode_(GetParam()) {
+ FLAGS_ip_config_mode = IPModeToString(mode_);
+ }
+
void SetUp() override {
KuduTest::SetUp();
@@ -962,12 +992,7 @@ class WebserverAdvertisedAddressesTest : public KuduTest {
protected:
// Overridden by subclasses.
virtual string use_webserver_interface() {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- if (mode == IPMode::IPV6) {
- FLAGS_webserver_interface = "[::]";
- }
- return FLAGS_webserver_interface;
+ return GetWebserverInterface(mode_);
}
virtual int32 use_webserver_port() const { return 0; }
virtual string use_advertised_addresses() { return ""; }
@@ -981,19 +1006,13 @@ class WebserverAdvertisedAddressesTest : public KuduTest
{
unique_ptr<Webserver> server_;
std::string expected_webserver_host_;
std::string expected_advertised_host_;
+ const IPMode mode_;
};
-class AdvertisedOnlyWebserverTest : public WebserverAdvertisedAddressesTest,
- public
::testing::WithParamInterface<std::string> {
- public:
- AdvertisedOnlyWebserverTest() {
- FLAGS_ip_config_mode = GetParam();
- }
+class AdvertisedOnlyWebserverTest : public WebserverAdvertisedAddressesTest {
protected:
string use_advertised_addresses() override {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- switch (mode) {
+ switch (mode_) {
case IPMode::IPV4:
expected_advertised_host_ = "1.2.3.4";
return "1.2.3.4:1234";
@@ -1006,17 +1025,10 @@ class AdvertisedOnlyWebserverTest : public
WebserverAdvertisedAddressesTest,
}
};
-class BoundOnlyWebserverTest : public WebserverAdvertisedAddressesTest,
- public
::testing::WithParamInterface<std::string> {
- public:
- BoundOnlyWebserverTest() {
- FLAGS_ip_config_mode = GetParam();
- }
+class BoundOnlyWebserverTest : public WebserverAdvertisedAddressesTest {
protected:
string use_webserver_interface() override {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- switch (mode) {
+ switch (mode_) {
case IPMode::IPV4:
expected_webserver_host_ = expected_advertised_host_ = "127.0.0.1";
return expected_webserver_host_;
@@ -1030,17 +1042,10 @@ class BoundOnlyWebserverTest : public
WebserverAdvertisedAddressesTest,
int32 use_webserver_port() const override { return 9999; }
};
-class BothBoundAndAdvertisedWebserverTest : public
WebserverAdvertisedAddressesTest,
- public
::testing::WithParamInterface<std::string> {
- public:
- BothBoundAndAdvertisedWebserverTest() {
- FLAGS_ip_config_mode = GetParam();
- }
+class BothBoundAndAdvertisedWebserverTest : public
WebserverAdvertisedAddressesTest {
protected:
string use_advertised_addresses() override {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- switch (mode) {
+ switch (mode_) {
case IPMode::IPV4:
expected_advertised_host_ = "1.2.3.4";
return "1.2.3.4:1234";
@@ -1052,9 +1057,7 @@ class BothBoundAndAdvertisedWebserverTest : public
WebserverAdvertisedAddressesT
}
}
string use_webserver_interface() override {
- IPMode mode;
- CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
- switch (mode) {
+ switch (mode_) {
case IPMode::IPV4:
expected_webserver_host_ = "127.0.0.1";
return expected_webserver_host_;
@@ -1071,7 +1074,7 @@ class BothBoundAndAdvertisedWebserverTest : public
WebserverAdvertisedAddressesT
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, AdvertisedOnlyWebserverTest,
- testing::Values("ipv4", "ipv6"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6));
TEST_P(AdvertisedOnlyWebserverTest, OnlyAdvertisedAddresses) {
vector<Sockaddr> bound_addrs;
@@ -1087,7 +1090,7 @@ TEST_P(AdvertisedOnlyWebserverTest,
OnlyAdvertisedAddresses) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, BoundOnlyWebserverTest,
- testing::Values("ipv4", "ipv6"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6));
TEST_P(BoundOnlyWebserverTest, OnlyBoundAddresses) {
vector<Sockaddr> bound_addrs;
@@ -1104,7 +1107,7 @@ TEST_P(BoundOnlyWebserverTest, OnlyBoundAddresses) {
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, BothBoundAndAdvertisedWebserverTest,
- testing::Values("ipv4", "ipv6"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6));
TEST_P(BothBoundAndAdvertisedWebserverTest, BothBoundAndAdvertisedAddresses) {
vector<Sockaddr> bound_addrs;
@@ -1123,17 +1126,18 @@ TEST_P(BothBoundAndAdvertisedWebserverTest,
BothBoundAndAdvertisedAddresses) {
// Various tests for failed webserver startup cases.
class WebserverNegativeTests : public KuduTest,
- public
::testing::WithParamInterface<std::string> {
+ public ::testing::WithParamInterface<IPMode> {
protected:
// Tries to start the webserver, expecting it to fail.
// 'func' is used to set webserver options before starting it.
template<class OptsFunc>
void ExpectFailedStartup(const OptsFunc& func) {
- FLAGS_ip_config_mode = GetParam();
+ FLAGS_ip_config_mode = IPModeToString(GetParam());
WebserverOptions opts;
opts.port = 0;
func(&opts);
+ opts.bind_interface = GetWebserverInterface(GetParam());
Webserver server(opts);
Status s = server.Start();
ASSERT_FALSE(s.ok()) << s.ToString();
@@ -1142,7 +1146,7 @@ class WebserverNegativeTests : public KuduTest,
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, WebserverNegativeTests,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
TEST_P(WebserverNegativeTests, BadCertFile) {
ExpectFailedStartup([](WebserverOptions* opts) {
diff --git a/src/kudu/util/net/net_util-test.cc
b/src/kudu/util/net/net_util-test.cc
index f4d8f48ca..ef77a8227 100644
--- a/src/kudu/util/net/net_util-test.cc
+++ b/src/kudu/util/net/net_util-test.cc
@@ -281,10 +281,10 @@ TEST_F(NetUtilTest, TestParseAddressesWithScheme) {
}
class ResolveAddressTest : public NetUtilTest,
- public ::testing::WithParamInterface<std::string> {
+ public ::testing::WithParamInterface<IPMode> {
public:
ResolveAddressTest() : hostname("localhost") {
- FLAGS_ip_config_mode = GetParam();
+ FLAGS_ip_config_mode = IPModeToString(GetParam());
}
protected:
string hostname;
@@ -293,7 +293,7 @@ protected:
// This is used to run all parameterized tests with different IP modes.
INSTANTIATE_TEST_SUITE_P(Parameters, ResolveAddressTest,
- testing::Values("ipv4", "ipv6", "dual"));
+ testing::Values(IPMode::IPV4, IPMode::IPV6,
IPMode::DUAL));
// Paramterize these tests for ipv4, ipv6 and dual as localhost can resolve to
any of these.
TEST_P(ResolveAddressTest, TestInjectFailureToResolveAddresses) {
diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
index d45290cd6..43ffd49d3 100644
--- a/src/kudu/util/net/net_util.cc
+++ b/src/kudu/util/net/net_util.cc
@@ -183,6 +183,17 @@ Status ParseIPModeFlag(const string& flag_value, IPMode*
mode) {
return Status::OK();
}
+const char* IPModeToString(IPMode mode) {
+ switch (mode) {
+ case IPMode::DUAL:
+ return "dual";
+ case IPMode::IPV6:
+ return "ipv6";
+ default:
+ return "ipv4";
+ }
+}
+
sa_family_t GetIPFamily() {
IPMode mode;
CHECK_OK(ParseIPModeFlag(FLAGS_ip_config_mode, &mode));
diff --git a/src/kudu/util/net/net_util.h b/src/kudu/util/net/net_util.h
index c6499a214..535d90d30 100644
--- a/src/kudu/util/net/net_util.h
+++ b/src/kudu/util/net/net_util.h
@@ -282,6 +282,11 @@ enum class IPMode {
// "ipv4", "ipv6", "dual"
Status ParseIPModeFlag(const std::string& flag_value, IPMode* mode);
+// This is a helper function to return corresponding ip_config_mode string
+// for a given IPMode value:
+// IPMode::IPV4, IPMode::IPV6, IPMode::DUAL
+const char* IPModeToString(IPMode mode);
+
// Return appropriate sa_family_t based on ip_config_mode flag value.
sa_family_t GetIPFamily();
diff --git a/src/kudu/util/test_util.cc b/src/kudu/util/test_util.cc
index bd1c1d361..ab160ab9e 100644
--- a/src/kudu/util/test_util.cc
+++ b/src/kudu/util/test_util.cc
@@ -758,4 +758,10 @@ void CheckPrometheusOutput(const string&
prometheus_output) {
}
}
}
+
+std::ostream& operator<<(std::ostream& os, const IPMode& mode) {
+ os << IPModeToString(mode);
+ return os;
+}
+
} // namespace kudu
diff --git a/src/kudu/util/test_util.h b/src/kudu/util/test_util.h
index 4f882e7ab..8ef1f3629 100644
--- a/src/kudu/util/test_util.h
+++ b/src/kudu/util/test_util.h
@@ -23,15 +23,17 @@
#include <cstdint>
#include <functional>
+#include <iosfwd>
#include <memory>
#include <string>
-#include <vector>
#include <unordered_map>
+#include <vector>
#include <gtest/gtest.h>
#include "kudu/gutil/port.h"
#include "kudu/util/monotime.h"
+#include "kudu/util/net/net_util.h"
#define SKIP_IF_SLOW_NOT_ALLOWED() do { \
if (!AllowSlowTests()) { \
@@ -219,5 +221,9 @@ const std::unordered_map<std::string, std::string>&
GetMasterWebserverEndpoints(
// Prometheus metrics output sanity check. This check is used in both the
tablet_server-test
// and the master-test, so it is placed here.
void CheckPrometheusOutput(const std::string& prometheus_output);
+
+// Beautifies test output if a test scenario fails.
+std::ostream& operator<<(std::ostream& os, const IPMode& mode);
+
} // namespace kudu
#endif
