Skip to content
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.

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

Windows: enable tests and envoy-static.exe pdb file #13688

Merged
merged 30 commits into from
Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c26ed9c
Workaround problematic googletests with clang-cl compilation
wrowe Oct 9, 2020
d74faff
Activate the clang-cl pipeline with working opt build
wrowe Sep 16, 2020
96b8c29
Tag tests broken due to missing dtor in -c opt --config=clang-cl
sunjayBhatia Oct 21, 2020
a04cd1d
Windows: re-disable tests identifed in #13133
wrowe Oct 28, 2020
d73e918
Address test sequencing problems which cause windows flakes
wrowe Oct 29, 2020
cdb8b44
Demonstrate clang-cl build failures to share with maintainers
wrowe Oct 30, 2020
869b718
Fix bazel syntax
wrowe Oct 30, 2020
5451544
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 3, 2020
747e250
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 10, 2020
e4048af
Drop corrections of EXPECT_ arguments for result values ignored
wrowe Nov 10, 2020
db0120b
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 11, 2020
eaea783
The clang-cl 11.0.0 release resolves this issue
wrowe Nov 11, 2020
bc3d3a4
Unmark tests now passing on clang-cl 11.0.0
wrowe Nov 11, 2020
5c030e8
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 11, 2020
d4884e3
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 17, 2020
27d54a7
Anticipate release of bazel 4.0.0
wrowe Nov 17, 2020
7468926
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 19, 2020
1a7523d
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
sunjayBhatia Nov 20, 2020
788a39e
Reidentify passing and failing/flaky tests for msvc-cl and clang-cl
sunjayBhatia Nov 20, 2020
b49a982
Defer clang-cl pipeline, fix format
wrowe Nov 20, 2020
d794063
Add ability to read text file in test helper
wrowe Nov 20, 2020
840f6fd
Fix copy paste typo
sunjayBhatia Nov 20, 2020
3ffbeba
Enable additional tests
wrowe Nov 20, 2020
d575f73
Reintroduce flaky tag with detail based on CI observation
wrowe Nov 21, 2020
446f5f5
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 21, 2020
f26c01a
Re-flake a few tests observed in CI
wrowe Nov 21, 2020
294dd22
Quick doc fixes prompted by htuch
wrowe Nov 24, 2020
dc68e63
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 24, 2020
5a058de
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
wrowe Nov 25, 2020
779a753
Merge remote-tracking branch 'origin/master' into add-clang-cl-build
sunjayBhatia Nov 30, 2020
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
24 changes: 22 additions & 2 deletions .azure-pipelines/pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ stages:
vmImage: "windows-latest"
steps:
- bash: ci/run_envoy_docker.sh ci/windows_ci_steps.sh
displayName: "Run Windows CI"
displayName: "Run Windows msvc-cl CI"
env:
ENVOY_DOCKER_BUILD_DIR: "$(Build.StagingDirectory)"
ENVOY_RBE: "true"
Expand All @@ -381,9 +381,29 @@ stages:
artifactName: windows.release
condition: always()

- job: docker
- job: clang_cl
dependsOn: ["release"]
timeoutInMinutes: 120
pool:
vmImage: "windows-latest"
steps:
- bash: ci/run_envoy_docker.sh ci/windows_ci_steps.sh
displayName: "Run Windows clang-cl CI"
env:
ENVOY_DOCKER_BUILD_DIR: "$(Build.StagingDirectory)"
ENVOY_RBE: "true"
BAZEL_BUILD_EXTRA_OPTIONS: "--config=remote-ci --config=remote-clang-cl --jobs=$(RbeJobs)"
BAZEL_REMOTE_CACHE: grpcs://remotebuildexecution.googleapis.com
BAZEL_REMOTE_INSTANCE: projects/envoy-ci/instances/default_instance
GCP_SERVICE_ACCOUNT_KEY: $(GcpServiceAccountKey)
- task: PublishBuildArtifacts@1
inputs:
pathtoPublish: "$(Build.StagingDirectory)/envoy"
artifactName: windows.clang-cl
condition: always()

- job: docker
dependsOn: ["release"]
pool:
vmImage: "windows-latest"
steps:
Expand Down
6 changes: 6 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ build:clang-cl --define clang_cl=1
# Override determinism flags (DATE etc) is valid on clang-cl compiler
build:clang-cl --copt="-Wno-macro-redefined"
build:clang-cl --copt="-Wno-builtin-macro-redefined"
# Workaround problematic missing override declarations of mocks
# TODO: resolve this class of problematic mocks, e.g.
Copy link
Member

Choose a reason for hiding this comment

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

Is the plan to do this in this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unclear whether 11.0.0 is going to fix this for us, whether we can fix this in a broad stroke, or whether every mock declaration affected must be updated and was missed in the last googletest update. This is why it was left unassigned.

Copy link
Member

Choose a reason for hiding this comment

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

Can you file a tracking issue?

# ./test/mocks/http/stream.h(16,21): error: 'addCallbacks'
# overrides a member function but is not marked 'override'
# MOCK_METHOD(void, addCallbacks, (StreamCallbacks & callbacks)); │
build:clang-cl --copt="-Wno-inconsistent-missing-override"
build:clang-cl --action_env=USE_CLANG_CL=1

# Defaults to 'auto' - Off for windows, so override to linux behavior
Expand Down
2 changes: 1 addition & 1 deletion bazel/envoy_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def _envoy_linkopts():
"-pagezero_size 10000",
"-image_base 100000000",
],
"@envoy//bazel:clang_cl_opt_build": [
"@envoy//bazel:windows_opt_build": [
Copy link
Contributor

Choose a reason for hiding this comment

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

This expands creation of a .pdb debuginfo symbol file to the opt build of msvc + clang, now that our rbe gcp pool has a larger working set of memory to collect that during the link phase.

"-DEFAULTLIB:ws2_32.lib",
"-DEFAULTLIB:iphlpapi.lib",
"-DEBUG:FULL",
Expand Down
2 changes: 1 addition & 1 deletion bazel/envoy_internal.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def envoy_copts(repository, test = False):
repository + "//bazel:opt_build": [] if test else ["-ggdb3", "-gsplit-dwarf"],
repository + "//bazel:fastbuild_build": [],
repository + "//bazel:dbg_build": ["-ggdb3", "-gsplit-dwarf"],
repository + "//bazel:windows_opt_build": [],
repository + "//bazel:windows_opt_build": [] if test else ["-Z7"],
Copy link
Contributor

Choose a reason for hiding this comment

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

This collects debuginfo symbol info to the .obj files during the opt build of msvc + clang, only for non-test sources. For test sources, do not collect debuginfo for the hundreds of test executables. (We would build fastbuild or dbg individually to troubleshoot a crashing test.)

Copy link
Member

Choose a reason for hiding this comment

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

Can you add this to the .bzl comments so this intuition is kept for posterity?

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that this comment...

Simplify the amount of symbolic debug info for test binaries,
targets listed in order from generic to increasing specificity.

did exactly that?

Copy link
Member

Choose a reason for hiding this comment

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

You might be technically correct here, the best kind of correct. I think this is not really as helpful as something like "do not collect debuginfo for the hundreds of test executables. (We would build fastbuild or dbg individually to troubleshoot a crashing test.)"

Copy link
Contributor

Choose a reason for hiding this comment

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

@htuch have a look at the new iteration and let me know if that's helpful.

repository + "//bazel:windows_fastbuild_build": [],
repository + "//bazel:windows_dbg_build": [],
repository + "//bazel:clang_cl_opt_build": [] if test else ["-Z7", "-fstandalone-debug"],
Expand Down
14 changes: 12 additions & 2 deletions ci/windows_ci_steps.sh
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,13 @@ BAZEL_BUILD_OPTIONS=(
"${BAZEL_BUILD_EXTRA_OPTIONS[@]}"
"${BAZEL_EXTRA_TEST_OPTIONS[@]}")

skip_test_tags="--test_tag_filters=-skip_on_windows,-fails_on_windows,-flaky_on_windows"
fail_test_tags="--test_tag_filters=fails_on_windows,flaky_on_windows"
if [[ "${BAZEL_BUILD_OPTIONS[*]}" =~ "clang-cl" ]]; then
skip_test_tags="${skip_test_tags},-skip_on_windows_clang,-fails_on_windows_clang"
fail_test_tags="${fail_test_tags},fails_on_windows_clang"
fi

Copy link
Contributor

Choose a reason for hiding this comment

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

This extra logic will be discarded after all compile and runtime failures unique to clang-cl have been resolved.

# Also setup some space for building Envoy standalone.
ENVOY_BUILD_DIR="${BUILD_DIR}"/envoy
mkdir -p "${ENVOY_BUILD_DIR}"
Expand All @@ -73,10 +80,13 @@ cp -f bazel-bin/source/exe/envoy-static.exe "${ENVOY_DELIVERY_DIR}/envoy.exe"
tar czf "${ENVOY_BUILD_DIR}"/envoy_binary.tar.gz -C "${ENVOY_DELIVERY_DIR}" envoy.exe

# Test invocations of known-working tests on Windows
bazel "${BAZEL_STARTUP_OPTIONS[@]}" test "${BAZEL_BUILD_OPTIONS[@]}" //test/... --test_tag_filters=-skip_on_windows,-fails_on_windows,-flaky_on_windows --build_tests_only
bazel "${BAZEL_STARTUP_OPTIONS[@]}" test "${BAZEL_BUILD_OPTIONS[@]}" //test/... "${skip_test_tags}" --build_tests_only

# Build tests that are known-flaky or known-failing to ensure no compilation regressions
bazel "${BAZEL_STARTUP_OPTIONS[@]}" build "${BAZEL_BUILD_OPTIONS[@]}" //test/... --test_tag_filters=-skip_on_windows,fails_on_windows,flaky_on_windows --build_tests_only
bazel "${BAZEL_STARTUP_OPTIONS[@]}" build "${BAZEL_BUILD_OPTIONS[@]}" //test/... "${fail_test_tags}" --build_tests_only

# Demonstrate broken linkage of all tests known to fail to build with clang-cl
bazel "${BAZEL_STARTUP_OPTIONS[@]}" build "${BAZEL_BUILD_OPTIONS[@]}" //test/... "--test_tag_filters=skip_on_windows_clang" --build_tests_only --keep-going

# Summarize tests bypasssed to monitor the progress of porting to Windows
echo "Tests bypassed as skip_on_windows: $(bazel query 'kind(".*test rule", attr("tags", "skip_on_windows", //test/...))' 2>/dev/null | sort | wc -l) known unbuildable or inapplicable tests"
Expand Down
4 changes: 2 additions & 2 deletions source/extensions/clusters/redis/redis_cluster.cc
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,8 @@ void RedisCluster::RedisDiscoverySession::startResolveRedis() {
if (parent_.hosts_.empty()) {
const int rand_idx = parent_.random_.random() % discovery_address_list_.size();
auto it = discovery_address_list_.begin();
std::next(it, rand_idx);
host = Upstream::HostSharedPtr{new RedisHost(parent_.info(), "", *it, parent_, true)};
host = Upstream::HostSharedPtr{
new RedisHost(parent_.info(), "", *std::next(it, rand_idx), parent_, true)};
Copy link
Contributor

Choose a reason for hiding this comment

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

@htuch - this is the only non-build, non-test change in this entire patchset. Would you like me to break it out for you as an independently reviewed PR, or are we ok to merge and iterate at this point? It's the remaining example of a discarded no-op result in the //source/... since I re-merged.

Copy link
Member

Choose a reason for hiding this comment

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

No, I was not asking for this. I was pointing out that the test changes alone could have been upstreamed independently. Anyway, NBD.

} else {
const int rand_idx = parent_.random_.random() % parent_.hosts_.size();
host = parent_.hosts_[rand_idx];
Expand Down
2 changes: 1 addition & 1 deletion test/common/config/datasource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ TEST_F(AsyncDataSourceTest, BaseIntervalGreaterThanMaxInterval) {
TestUtility::loadFromYamlAndValidate(yaml, config);
EXPECT_TRUE(config.has_remote());

EXPECT_THROW_WITH_MESSAGE(std::make_unique<Config::DataSource::RemoteAsyncDataProvider>(
EXPECT_THROW_WITH_MESSAGE((void)std::make_unique<Config::DataSource::RemoteAsyncDataProvider>(
cm_, init_manager_, config.remote(), dispatcher_, random_, true,
[&](const std::string&) {}),
EnvoyException,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a wide open question, @htuch - is there someone of the google / googletest participants who could identify why this emits an error under clang-cl, but on no other architectures (such as gcc, linux clang, and not even msvc.)

We could add the explicit flag -Wno-discard-results for clang-cl, but we hadn't done this for other architectures, and I'm wondering if that would be wise. I've fixed a couple of discarded non-expectation results, but in this EXPECT_[NO]THROW googletest pattern, it does get a little wordy.

Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Definitely would rather not have this void cast. I'll follow up on this offline, but one option is to build the cast explicitly into the EXPECT_THROW_WITH_MESSAGE macro, which is something we define.

Copy link
Member

Choose a reason for hiding this comment

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

LMK if this is still an issue on Clang 11.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still an issue with -Wall -Werror; however, it makes sense to keep this potential defect detection in source/... --- but it doesn't seem interesting to track discarded results in test/... sources. Moved the warning suppression to the appropriate tests-only copt logic for clang-cl builds, and left a handful of corrections for source/... --- including documentation of why the results are discarded.

Expand Down
6 changes: 6 additions & 0 deletions test/common/http/http2/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ envoy_cc_test(
name = "codec_impl_test",
srcs = ["codec_impl_test.cc"],
shard_count = 5,
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = CODEC_TEST_DEPS,
)

Expand All @@ -52,6 +54,8 @@ envoy_cc_test(
"--runtime-feature-disable-for-tests=envoy.reloadable_features.new_codec_behavior",
],
shard_count = 5,
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
Copy link
Contributor

Choose a reason for hiding this comment

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

This is what I'm looking for some possible assistance from google folks, @htuch - we are having serious problems deciphering why clang-cl is failing in a manner unique to building opt (/O2) on Windows using clang-cl, and in no other environments or compilers. This particular link failure reads as follows;

ERROR: C:/users/wrowe/dev/envoy/test/common/http/http2/BUILD:40:14: Linking of rule '//test/common/http/http2:codec_impl_test' failed (Exit 1)
lld-link: error: undefined symbol: public: virtual __cdecl Envoy::Http::ConnectionCallbacks::~ConnectionCallbacks(void)
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>, class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>)'::`1'::dtor$69)
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>, class std::tuple<unsigned int, unsigned int, unsigned int, unsigned int>)'::`1'::dtor$76)
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: void __cdecl Envoy::Http::MockServerConnectionCallbacks::`vbase dtor'(void))
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: void __cdecl Envoy::Http::MockConnectionCallbacks::`vbase dtor'(void))>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: virtual __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::~Http2CodecImplTestFixture(void))
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(public: virtual __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::~Http2CodecImplTestFixture(void))
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(void)'::`1'::dtor$18)
>>> referenced by bazel-out/x64_windows-opt/bin/test/common/http/http2/_objs/codec_impl_test/codec_impl_test.obj:(int `public: __cdecl Envoy::Http::Http2::Http2CodecImplTestFixture::Http2CodecImplTestFixture(void)'::`1'::dtor$25)
Target //test/common/http/http2:codec_impl_test failed to build

Any chance you can find some support for us researching what might be happening here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Quick update for @htuch and @lizan - we attempted to verify this error with the llvm 11.0.0 release of clang-cl, however that update breaks bazel. Holding for this 1-line PR to be accepted to allow us to pick up the new clang-cl, and confirm whether the bug remains or has been resolved.

bazelbuild/rules_cc#88

/wait

Copy link
Contributor

Choose a reason for hiding this comment

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

This does appear to be resolved in clang 11.0.0 release.

deps = CODEC_TEST_DEPS,
)

Expand Down Expand Up @@ -128,6 +132,8 @@ envoy_cc_test(
"response_header_corpus/simple_example_huffman",
"response_header_corpus/simple_example_plain",
],
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
":frame_replay_lib",
"//test/common/http/http2:codec_impl_test_util",
Expand Down
8 changes: 4 additions & 4 deletions test/common/network/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,8 @@ envoy_cc_test(
# Used in createDnsResolver to force creation of DnsResolverImpl when running test on macOS.
"--runtime-feature-disable-for-tests=envoy.restart_features.use_apple_api_for_dns_lookups",
],
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
deps = [
"//include/envoy/event:dispatcher_interface",
"//include/envoy/network:address_interface",
Expand Down Expand Up @@ -275,10 +277,8 @@ envoy_cc_test(
envoy_cc_test(
name = "udp_listener_impl_batch_writer_test",
srcs = ["udp_listener_impl_batch_writer_test.cc"],
tags = [
# Skipping as quiche quic_gso_batch_writer.h does not exist on Windows
"skip_on_windows",
],
# Skipping as quiche quic_gso_batch_writer.h does not exist on Windows
tags = ["skip_on_windows"],
deps = [
":udp_listener_impl_test_base_lib",
"//source/common/event:dispatcher_lib",
Expand Down
1 change: 0 additions & 1 deletion test/common/network/cidr_range_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ TEST(TruncateIpAddressAndLength, Various) {
{{"ffff::ffff", 128}, {"ffff::ffff", 128}},
{{"ffff::ffff", 999}, {"ffff::ffff", 128}},
};
test_cases.size();
htuch marked this conversation as resolved.
Show resolved Hide resolved
for (const auto& kv : test_cases) {
InstanceConstSharedPtr inPtr = Utility::parseInternetAddress(kv.first.first);
EXPECT_NE(inPtr, nullptr) << kv.first.first;
Expand Down
6 changes: 2 additions & 4 deletions test/common/network/listener_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -412,13 +412,11 @@ TEST_P(TcpListenerImplTest, SetListenerRejectFractionIntermediate) {
{
testing::InSequence s1;
EXPECT_CALL(random_generator, random()).WillOnce(Return(std::numeric_limits<uint64_t>::max()));
EXPECT_CALL(listener_callbacks, onAccept_(_));
EXPECT_CALL(listener_callbacks, onAccept_(_)).WillOnce([&] { dispatcher_->exit(); });
}
{
testing::InSequence s2;
EXPECT_CALL(connection_callbacks, onEvent(ConnectionEvent::Connected)).WillOnce([&] {
dispatcher_->exit();
});
EXPECT_CALL(connection_callbacks, onEvent(ConnectionEvent::Connected));
EXPECT_CALL(connection_callbacks, onEvent(ConnectionEvent::RemoteClose)).Times(0);
}

Expand Down
4 changes: 2 additions & 2 deletions test/common/network/socket_option_factory_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ class SocketOptionFactoryTest : public testing::Test {
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{[this]() {
// Before injecting OsSysCallsImpl, make sure validateIpv{4,6}Supported is called so the static
// bool is initialized without requiring to mock ::socket and ::close. :( :(
std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
(void)std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
(void)std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
return &os_sys_calls_mock_;
}()};

Expand Down
4 changes: 2 additions & 2 deletions test/common/network/socket_option_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ class SocketOptionTest : public testing::Test {
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls_{[this]() {
// Before injecting OsSysCallsImpl, make sure validateIpv{4,6}Supported is called so the static
// bool is initialized without requiring to mock ::socket and ::close.
std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
(void)std::make_unique<Address::Ipv4Instance>("1.2.3.4", 5678);
(void)std::make_unique<Address::Ipv6Instance>("::1:2:3:4", 5678);
htuch marked this conversation as resolved.
Show resolved Hide resolved
return &os_sys_calls_;
}()};

Expand Down
2 changes: 2 additions & 0 deletions test/common/router/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,8 @@ envoy_cc_test(
envoy_cc_test(
name = "upstream_request_test",
srcs = ["upstream_request_test.cc"],
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/common/router:router_lib",
"//test/mocks/router:router_filter_interface",
Expand Down
4 changes: 2 additions & 2 deletions test/common/runtime/runtime_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,8 @@ TEST_F(DiskLoaderImplTest, MultipleAdminLayersFail) {
layer->mutable_admin_layer();
}
EXPECT_THROW_WITH_MESSAGE(
std::make_unique<LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_),
(void)std::make_unique<LoaderImpl>(dispatcher_, tls_, layered_runtime, local_info_, store_,
generator_, validation_visitor_, *api_),
EnvoyException,
"Too many admin layers specified in LayeredRuntime, at most one may be specified");
}
Expand Down
2 changes: 2 additions & 0 deletions test/common/tcp_proxy/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ envoy_cc_test(
envoy_cc_test(
name = "upstream_test",
srcs = ["upstream_test.cc"],
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/common/tcp_proxy",
"//test/mocks/http:http_mocks",
Expand Down
4 changes: 2 additions & 2 deletions test/common/upstream/upstream_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1018,8 +1018,8 @@ TEST_F(StrictDnsClusterImplTest, CustomResolverFails) {
singleton_manager_, tls_, validation_visitor_, *api_);

EXPECT_THROW_WITH_MESSAGE(
std::make_unique<StrictDnsClusterImpl>(cluster_config, runtime_, dns_resolver_,
factory_context, std::move(scope), false),
(void)std::make_unique<StrictDnsClusterImpl>(cluster_config, runtime_, dns_resolver_,
factory_context, std::move(scope), false),
EnvoyException, "STRICT_DNS clusters must NOT have a custom resolver name set");
}

Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/adaptive_concurrency/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ envoy_extension_cc_test(
"adaptive_concurrency_filter_integration_test.h",
],
extension_name = "envoy.filters.http.adaptive_concurrency",
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
deps = [
"//source/extensions/filters/http/adaptive_concurrency:config",
"//source/extensions/filters/http/fault:config",
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/common/compressor/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,6 @@ envoy_cc_benchmark_binary(
envoy_benchmark_test(
name = "compressor_filter_speed_test_benchmark_test",
benchmark_binary = "compressor_filter_speed_test",
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
)
2 changes: 2 additions & 0 deletions test/extensions/filters/http/grpc_json_transcoder/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ envoy_extension_cc_test(
"//test/proto:bookstore_proto_descriptor",
],
extension_name = "envoy.filters.http.grpc_json_transcoder",
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
deps = [
"//source/common/grpc:codec_lib",
"//source/common/http:header_map_lib",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ TEST_F(HeaderToMetadataTest, RejectInvalidRule) {

TEST_F(HeaderToMetadataTest, PerRouteEmtpyRules) {
envoy::extensions::filters::http::header_to_metadata::v3::Config config_proto;
EXPECT_THROW(std::make_shared<Config>(config_proto, true), EnvoyException);
EXPECT_THROW((void)std::make_shared<Config>(config_proto, true), EnvoyException);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/http/lua/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ envoy_extension_cc_test(
name = "lua_filter_test",
srcs = ["lua_filter_test.cc"],
extension_name = "envoy.filters.http.lua",
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
deps = [
"//source/common/stream_info:stream_info_lib",
"//source/extensions/filters/http/lua:lua_filter_lib",
Expand Down
6 changes: 5 additions & 1 deletion test/extensions/filters/listener/http_inspector/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@ envoy_extension_cc_test(
name = "http_inspector_test",
srcs = ["http_inspector_test.cc"],
extension_name = "envoy.filters.listener.http_inspector",
#TODO(davinci26): The test passes on Windows *but* http inspector
# TODO(davinci26): The test passes on Windows *but* http inspector
# *used* to rely on Event::FileTriggerType::Edge and we got away with it
# because we mock the dispatcher. Need to verify that the scenario is
# actually working.
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/common/common:hex_lib",
"//source/common/http:utility_lib",
Expand All @@ -35,6 +37,8 @@ envoy_extension_cc_test(
name = "http_inspector_config_test",
srcs = ["http_inspector_config_test.cc"],
extension_name = "envoy.filters.listener.http_inspector",
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/extensions/filters/listener:well_known_names",
"//source/extensions/filters/listener/http_inspector:config",
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/listener/proxy_protocol/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ envoy_extension_cc_test(
name = "proxy_protocol_test",
srcs = ["proxy_protocol_test.cc"],
extension_name = "envoy.filters.listener.proxy_protocol",
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/common/buffer:buffer_lib",
"//source/common/event:dispatcher_includes",
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/listener/tls_inspector/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ envoy_package()
envoy_cc_test(
name = "tls_inspector_test",
srcs = ["tls_inspector_test.cc"],
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
":tls_utility_lib",
"//source/common/http:utility_lib",
Expand Down
6 changes: 3 additions & 3 deletions test/extensions/filters/network/dubbo_proxy/metadata_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,21 @@ TEST(MessageMetadataTest, Fields) {
metadata.setInvocationInfo(invo);
EXPECT_TRUE(metadata.hasInvocationInfo());

EXPECT_THROW(metadata.timeout().value(), absl::bad_optional_access);
EXPECT_THROW((void)metadata.timeout().value(), absl::bad_optional_access);
metadata.setTimeout(3);
EXPECT_TRUE(metadata.timeout().has_value());

invo->setMethodName("method");
EXPECT_EQ("method", invo->methodName());

EXPECT_FALSE(invo->serviceVersion().has_value());
EXPECT_THROW(invo->serviceVersion().value(), absl::bad_optional_access);
EXPECT_THROW((void)invo->serviceVersion().value(), absl::bad_optional_access);
invo->setServiceVersion("1.0.0");
EXPECT_TRUE(invo->serviceVersion().has_value());
EXPECT_EQ("1.0.0", invo->serviceVersion().value());

EXPECT_FALSE(invo->serviceGroup().has_value());
EXPECT_THROW(invo->serviceGroup().value(), absl::bad_optional_access);
EXPECT_THROW((void)invo->serviceGroup().value(), absl::bad_optional_access);
invo->setServiceGroup("group");
EXPECT_TRUE(invo->serviceGroup().has_value());
EXPECT_EQ("group", invo->serviceGroup().value());
Expand Down
2 changes: 2 additions & 0 deletions test/extensions/filters/network/rocketmq_proxy/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ envoy_extension_cc_test(
name = "config_test",
srcs = ["config_test.cc"],
extension_name = "envoy.filters.network.rocketmq_proxy",
# TODO(envoyproxy/windows-dev): Resolve missing pure virtual destructor bug
tags = ["skip_on_windows_clang"],
deps = [
"//source/extensions/filters/network/rocketmq_proxy:config",
"//test/mocks/local_info:local_info_mocks",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ envoy_extension_cc_test(
"//test/config/integration/certs",
],
extension_name = "envoy.filters.network.sni_dynamic_forward_proxy",
# TODO(envoyproxy/windows-dev): Resolve test failing when compiled with clang-cl opt build on Windows
tags = ["fails_on_windows_clang"],
deps = [
"//source/extensions/clusters/dynamic_forward_proxy:cluster",
"//source/extensions/filters/listener/tls_inspector:config",
Expand Down
5 changes: 2 additions & 3 deletions test/extensions/grpc_credentials/file_based_metadata/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@ licenses(["notice"]) # Apache 2
envoy_package()

envoy_cc_test(
name = "file_based_metadata_grpc_credentials_test",
srcs = ["file_based_metadata_grpc_credentials_test.cc"],
name = "integration_test",
srcs = ["integration_test.cc"],
data = ["//test/config/integration/certs"],
tags = ["flaky_on_windows"],
deps = [
"//source/extensions/grpc_credentials:well_known_names",
"//source/extensions/grpc_credentials/file_based_metadata:config",
Expand Down
Loading