Skip to content

build: don't depend on test_pch unless using pch #18466

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
mattklein123 merged 6 commits into from
Oct 7, 2021
Merged

build: don't depend on test_pch unless using pch #18466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ac13550
build: don't depend on test_pch unless using pch
mattklein123 Oct 6, 2021
982f857
Merge remote-tracking branch 'origin/main' into pch_fix
mattklein123 Oct 6, 2021
a8ce93d
fix
mattklein123 Oct 6, 2021
9fe2ae7
Merge remote-tracking branch 'origin/main' into pch_fix
mattklein123 Oct 6, 2021
68dc80c
Merge branch 'main' into pch_fix
mattklein123 Oct 6, 2021
137c880
fix
mattklein123 Oct 6, 2021
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
6 changes: 4 additions & 2 deletions bazel/envoy_test.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,12 @@ def envoy_cc_test(
linkstatic = envoy_linkstatic(),
malloc = tcmalloc_external_dep(repository),
deps = envoy_stdlib_deps() + deps + [envoy_external_dep_path(dep) for dep in external_deps + ["googletest"]] + [
repository + "//test:test_pch",
repository + "//test:main",
repository + "//test/test_common:test_version_linkstamp",
],
] + select({
Copy link
Copy Markdown
Member

@keith keith Oct 6, 2021

Choose a reason for hiding this comment

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

Needs to be a list

Copy link
Copy Markdown
Member Author

@mattklein123 mattklein123 Oct 6, 2021

Choose a reason for hiding this comment

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

Sorry what do you mean? You need brackets around the select? Or inside the select?

Copy link
Copy Markdown
Member Author

@mattklein123 mattklein123 Oct 6, 2021

Choose a reason for hiding this comment

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

(I think we have brackets inside the select?)

Copy link
Copy Markdown
Member

@keith keith Oct 6, 2021

Choose a reason for hiding this comment

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

ah ok yea sorry that works too, i've seen it the other way in this repo so i just assumed

repository + "//bazel:clang_pch_build": [repository + "//test:test_pch"],
"//conditions:default": [],
}),
# from https://github.com/google/googletest/blob/6e1970e2376c14bf658eb88f655a054030353f9f/googlemock/src/gmock.cc#L51
# 2 - by default, mocks act as StrictMocks.
args = args + ["--gmock_default_mock_behavior=2"],
Expand Down
1 change: 1 addition & 0 deletions test/extensions/common/dynamic_forward_proxy/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ envoy_cc_test(
"//source/extensions/common/dynamic_forward_proxy:dns_cache_manager_impl",
"//test/mocks/network:network_mocks",
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/server:factory_context_mocks",
Copy link
Copy Markdown
Member

@ggreenway ggreenway Oct 6, 2021

Choose a reason for hiding this comment

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

Did you intend for these two BUILD file changes to be in this PR?

Copy link
Copy Markdown
Member Author

@mattklein123 mattklein123 Oct 6, 2021

Choose a reason for hiding this comment

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

Yes, it's required because we no longer basically build the world for each test. I think there is an issue with how much test_pch brings in that we need to also fix.

"//test/mocks/thread_local:thread_local_mocks",
"//test/test_common:registry_lib",
"//test/test_common:simulated_time_system_lib",
Expand Down
1 change: 1 addition & 0 deletions test/extensions/filters/network/ratelimit/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ envoy_extension_cc_test(
"//test/mocks/network:network_mocks",
"//test/mocks/ratelimit:ratelimit_mocks",
"//test/mocks/runtime:runtime_mocks",
"//test/mocks/server:factory_context_mocks",
"//test/mocks/stream_info:stream_info_mocks",
"//test/mocks/tracing:tracing_mocks",
"@envoy_api//envoy/extensions/filters/network/ratelimit/v3:pkg_cc_proto",
Expand Down
17 changes: 10 additions & 7 deletions test/per_file_coverage.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,33 @@
# directory:coverage_percent
# for existing directories with low coverage.
declare -a KNOWN_LOW_COVERAGE=(
"source/common:96.1" # Raise when QUIC coverage goes up
"source/common:96.0" # Raise when QUIC coverage goes up
"source/common/api:79.8"
"source/common/api/posix:78.5"
"source/common/common/posix:92.7"
"source/common/config:96.5"
"source/common/config/xds_mux:94.5"
"source/common/crypto:0.0"
"source/common/event:94.1" # Emulated edge events guards don't report LCOV
"source/common/filesystem/posix:95.5"
"source/common/http:96.5"
"source/common/http:96.3"
"source/common/http/http2:96.4"
"source/common/json:90.1"
"source/common/matcher:94.2"
"source/common/network:94.8" # Flaky, `activateFileEvents`, `startSecureTransport` and `ioctl` do not always report LCOV
"source/common/protobuf:95.3"
"source/common/quic:91.8"
"source/common/secret:96.3"
"source/common/router:96.5"
"source/common/secret:94.9"
"source/common/signal:86.9" # Death tests don't report LCOV
"source/common/singleton:95.7"
"source/common/tcp:94.6"
"source/common/thread:0.0" # Death tests don't report LCOV
"source/common/tracing:96.1"
"source/common/upstream:96.5"
"source/common/upstream:96.2"
"source/common/watchdog:58.6" # Death tests don't report LCOV
"source/exe:92.6"
"source/extensions/common:96.0"
"source/extensions/common:95.9"
"source/extensions/common/tap:94.2"
"source/extensions/common/wasm:95.3" # flaky: be careful adjusting
"source/extensions/common/wasm/ext:92.0"
Expand Down Expand Up @@ -66,8 +69,8 @@ declare -a KNOWN_LOW_COVERAGE=(
"source/extensions/tracers/opencensus:94.8"
"source/extensions/tracers/xray:96.2"
"source/extensions/tracers/zipkin:96.1"
"source/extensions/transport_sockets:95.4"
"source/extensions/transport_sockets/tls:94.6"
"source/extensions/transport_sockets:95.3"
"source/extensions/transport_sockets/tls:94.5"
"source/extensions/transport_sockets/tls/cert_validator:96.0"
"source/extensions/transport_sockets/tls/ocsp:96.5"
"source/extensions/transport_sockets/tls/private_key:77.8"
Expand Down