Skip to content

test: TODO fixup making enable_half_close private. #14330

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alyssawilk merged 1 commit into from
Dec 9, 2020
Merged

test: TODO fixup making enable_half_close private. #14330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/extensions/access_loggers/grpc/tcp_grpc_access_log_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class TcpGrpcAccessLogIntegrationTest : public Grpc::VersionedGrpcClientIntegrat
public:
TcpGrpcAccessLogIntegrationTest()
: BaseIntegrationTest(ipVersion(), ConfigHelper::tcpProxyConfig()) {
enable_half_close_ = true;
enableHalfClose(true);
}

void createUpstreams() override {
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/filters/http/oauth2/oauth_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class OauthIntegrationTest : public testing::Test, public HttpIntegrationTest {
public:
OauthIntegrationTest()
: HttpIntegrationTest(Http::CodecClient::Type::HTTP2, Network::Address::IpVersion::v4) {
enable_half_close_ = true;
enableHalfClose(true);
}

envoy::service::discovery::v3::DiscoveryResponse genericSecretResponse(absl::string_view name,
Expand Down
4 changes: 2 additions & 2 deletions test/integration/base_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ Network::ClientConnectionPtr BaseIntegrationTest::makeClientConnectionWithOption
fmt::format("tcp://{}:{}", Network::Test::getLoopbackAddressUrlString(version_), port)),
Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(), options));

connection->enableHalfClose(enable_half_close_);
connection->enableHalfClose(enableHalfClose());
return connection;
}

Expand Down Expand Up @@ -223,7 +223,7 @@ BaseIntegrationTest::makeTcpConnection(uint32_t port,
const Network::ConnectionSocket::OptionsSharedPtr& options,
Network::Address::InstanceConstSharedPtr source_address) {
return std::make_unique<IntegrationTcpClient>(*dispatcher_, *mock_buffer_factory_, port, version_,
enable_half_close_, options, source_address);
enableHalfClose(), options, source_address);
}

void BaseIntegrationTest::registerPort(const std::string& key, uint32_t port) {
Expand Down
15 changes: 8 additions & 7 deletions test/integration/base_integration_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,11 +369,14 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
protected:
void setUdpFakeUpstream(bool value) { upstream_config_.udp_fake_upstream_ = value; }
bool initialized() const { return initialized_; }
const FakeUpstreamConfig& upstreamConfig() {
// TODO(alyssawilk) make enable_half_close_ private and remove this.
upstream_config_.enable_half_close_ = enable_half_close_;
return upstream_config_;
}

// Right now half-close is set globally, not separately for upstream and
// downstream.
void enableHalfClose(bool value) { upstream_config_.enable_half_close_ = value; }

bool enableHalfClose() { return upstream_config_.enable_half_close_; }
Comment on lines +375 to +377
Copy link
Member

@dio dio Dec 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if we want to name these two differently (to have different names for getter and setter). But I'm OK with this.


const FakeUpstreamConfig& upstreamConfig() { return upstream_config_; }

std::unique_ptr<Stats::Scope> upstream_stats_store_;

Expand Down Expand Up @@ -442,8 +445,6 @@ class BaseIntegrationTest : protected Logger::Loggable<Logger::Id::testing> {
// This does nothing if autonomous_upstream_ is false
bool autonomous_allow_incomplete_streams_{false};

bool enable_half_close_{false};

// True if test will use a fixed RNG value.
bool deterministic_{};

Expand Down
2 changes: 1 addition & 1 deletion test/integration/cluster_filter_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ class ClusterFilterIntegrationTest : public testing::TestWithParam<Network::Addr
: BaseIntegrationTest(GetParam(), ConfigHelper::tcpProxyConfig()), registration_(factory_) {}

void initialize() override {
enable_half_close_ = true;
enableHalfClose(true);
config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) {
auto* cluster_0 = bootstrap.mutable_static_resources()->mutable_clusters(0);
auto* filter = cluster_0->add_filters();
Expand Down
2 changes: 1 addition & 1 deletion test/integration/filter_manager_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ INSTANTIATE_TEST_SUITE_P(
InjectDataToFilterChainIntegrationTest::testParamsToString);

TEST_P(InjectDataWithTcpProxyFilterIntegrationTest, UsageOfInjectDataMethodsShouldBeUnnoticeable) {
enable_half_close_ = true;
enableHalfClose(true);
initialize();

auto tcp_client = makeTcpConnection(lookupPort("listener_0"));
Expand Down
10 changes: 5 additions & 5 deletions test/integration/tcp_proxy_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ TEST_P(TcpProxyIntegrationTest, NoUpstream) {
lb_endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(1);
});
config_helper_.skipPortUsageValidation();
enable_half_close_ = false;
enableHalfClose(false);
initialize();

IntegrationTcpClientPtr tcp_client = makeTcpConnection(lookupPort("tcp_proxy"));
Expand Down Expand Up @@ -384,7 +384,7 @@ TEST_P(TcpProxyIntegrationTest, ShutdownWithOpenConnections) {
TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithNoData) {
autonomous_upstream_ = true;

enable_half_close_ = false;
enableHalfClose(false);
config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
auto* filter_chain = listener->mutable_filter_chains(0);
Expand All @@ -408,7 +408,7 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithNoData) {

TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) {
config_helper_.setBufferLimits(1024, 1024);
enable_half_close_ = false;
enableHalfClose(false);
config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
auto* filter_chain = listener->mutable_filter_chains(0);
Expand Down Expand Up @@ -441,7 +441,7 @@ TEST_P(TcpProxyIntegrationTest, TestIdletimeoutWithLargeOutstandingData) {
TEST_P(TcpProxyIntegrationTest, TestMaxDownstreamConnectionDurationWithNoData) {
autonomous_upstream_ = true;

enable_half_close_ = false;
enableHalfClose(false);
config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
auto* filter_chain = listener->mutable_filter_chains(0);
Expand All @@ -466,7 +466,7 @@ TEST_P(TcpProxyIntegrationTest, TestMaxDownstreamConnectionDurationWithNoData) {

TEST_P(TcpProxyIntegrationTest, TestMaxDownstreamConnectionDurationWithLargeOutstandingData) {
config_helper_.setBufferLimits(1024, 1024);
enable_half_close_ = false;
enableHalfClose(false);
config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(0);
auto* filter_chain = listener->mutable_filter_chains(0);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/tcp_proxy_integration_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class TcpProxyIntegrationTest : public testing::TestWithParam<TcpProxyIntegratio
public:
TcpProxyIntegrationTest()
: BaseIntegrationTest(GetParam().version, ConfigHelper::tcpProxyConfig()) {
enable_half_close_ = true;
enableHalfClose(true);
}

void initialize() override;
Expand Down
8 changes: 4 additions & 4 deletions test/integration/tcp_tunneling_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class ConnectTerminationIntegrationTest
public:
ConnectTerminationIntegrationTest()
: HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam()) {
enable_half_close_ = true;
enableHalfClose(true);
}

void initialize() override {
Expand Down Expand Up @@ -329,7 +329,7 @@ class TcpTunnelingIntegrationTest : public testing::TestWithParam<Params>,
}

void SetUp() override {
enable_half_close_ = true;
enableHalfClose(true);
setDownstreamProtocol(Http::CodecClient::Type::HTTP2);
setUpstreamProtocol(std::get<1>(GetParam()));

Expand Down Expand Up @@ -456,7 +456,7 @@ TEST_P(TcpTunnelingIntegrationTest, ResetStreamTest) {
if (upstreamProtocol() == FakeHttpConnection::Type::HTTP1) {
return;
}
enable_half_close_ = false;
enableHalfClose(false);
initialize();

// Establish a connection.
Expand All @@ -472,7 +472,7 @@ TEST_P(TcpTunnelingIntegrationTest, ResetStreamTest) {
}

TEST_P(TcpTunnelingIntegrationTest, TestIdletimeoutWithLargeOutstandingData) {
enable_half_close_ = false;
enableHalfClose(false);
config_helper_.setBufferLimits(1024, 1024);
config_helper_.addConfigModifier([&](envoy::config::bootstrap::v3::Bootstrap& bootstrap) -> void {
auto* listener = bootstrap.mutable_static_resources()->mutable_listeners(1);
Expand Down
2 changes: 1 addition & 1 deletion test/integration/uds_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ HttpIntegrationTest::ConnectionCreationFunction UdsListenerIntegrationTest::crea
Network::Utility::resolveUrl(fmt::format("unix://{}", getListenerSocketName())),
Network::Address::InstanceConstSharedPtr(), Network::Test::createRawBufferSocket(),
nullptr));
conn->enableHalfClose(enable_half_close_);
conn->enableHalfClose(enableHalfClose());
return conn;
};
}
Expand Down