-
Notifications
You must be signed in to change notification settings - Fork 5.3k
tools/test: adding tools for splitting up mock headers #12150
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
Changes from all commits
27876b3
f33ed82
9579352
4106ddc
d13fc8f
270933d
364d7b2
b812242
900eb5e
89c47dd
6b94843
03f5b7a
25ba2d3
e31448a
e3d5220
688e47c
810cc3f
8e21940
e5dc56a
98d28dc
29f73b6
45c39a0
9c6c7dd
a63d79c
a1bfc6a
1157bcf
1ece0c5
8a77cd4
7a1d616
59c6e1c
c40bd31
c48cd2b
2ce96cd
78f571f
7ab2763
6040230
4278a4c
1bba190
0f7ca0f
891f8b3
66be1ac
470f3c8
2bc5aa2
31e6111
1922301
85fc71c
04da966
28833ef
49d6781
a7b40e8
6c5f650
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,6 +45,11 @@ function exclude_testdata() { | |
| grep -v tools/testdata/check_format/ | ||
| } | ||
|
|
||
| # Do not run clang-tidy on envoy_headersplit testdata files. | ||
| function exclude_testdata() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that you redefined the exclude_testdata function and effectively undid the tools/testdata/check_format exclusion a few lines up.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! Thanks for fix it |
||
| grep -v tools/envoy_headersplit/ | ||
| } | ||
|
|
||
| # Exclude files in third_party which are temporary forks from other OSS projects. | ||
| function exclude_third_party() { | ||
| grep -v third_party/ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| load("@rules_python//python:defs.bzl", "py_binary", "py_test") | ||
| load("@headersplit_pip3//:requirements.bzl", "requirement") | ||
| load( | ||
| "//bazel:envoy_build_system.bzl", | ||
| "envoy_package", | ||
| ) | ||
|
|
||
| licenses(["notice"]) # Apache 2 | ||
|
|
||
| envoy_package() | ||
|
|
||
| py_binary( | ||
| name = "headersplit", | ||
| srcs = [ | ||
| "headersplit.py", | ||
| ], | ||
| python_version = "PY3", | ||
| srcs_version = "PY3", | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| requirement("clang"), | ||
| ], | ||
| ) | ||
|
|
||
| py_binary( | ||
| name = "replace_includes", | ||
| srcs = [ | ||
| "replace_includes.py", | ||
| ], | ||
| python_version = "PY3", | ||
| srcs_version = "PY3", | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| ":headersplit", | ||
| ], | ||
| ) | ||
|
|
||
| py_test( | ||
| name = "headersplit_test", | ||
| srcs = [ | ||
| "headersplit_test.py", | ||
| ], | ||
| data = glob(["code_corpus/**"]), | ||
| python_version = "PY3", | ||
| srcs_version = "PY3", | ||
| tags = ["no-sandbox"], # TODO (foreseeable): make this test run under sandbox | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| requirement("clang"), | ||
foreseeable marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ":headersplit", | ||
| ], | ||
| ) | ||
|
|
||
| py_test( | ||
| name = "replace_includes_test", | ||
| srcs = [ | ||
| "replace_includes_test.py", | ||
| ], | ||
| data = glob(["code_corpus/**"]), | ||
| python_version = "PY3", | ||
| srcs_version = "PY3", | ||
| tags = ["no-sandbox"], # TODO (foreseeable): make this test run under sandbox | ||
| visibility = ["//visibility:public"], | ||
| deps = [ | ||
| ":replace_includes", | ||
| ], | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # Envoy Header Split | ||
| Tool for spliting monolithic header files in Envoy to speed up compilation | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How often do you think Envoy developers should be running this? Is this a once a year kind of cleanup?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the period could be several months or a year. It should be long enough until we get some monolithic mock headers appeared in Envoy repository. |
||
|
|
||
| Steps to divide Envoy mock headers: | ||
|
|
||
| 1. Run `headersplit.py` to divide the monolithic mock header into different classes | ||
foreseeable marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Example (to split monolithic mock header test/mocks/network/mocks.h): | ||
|
|
||
| ``` | ||
| cd ${ENVOY_SRCDIR}/test/mocks/network/ | ||
| python3 ${ENVOY_SRCDIR}/tools/envoy_headersplit/headersplit.py -i mocks.cc -d mocks.h | ||
| ``` | ||
|
|
||
| 2. Remove unused `#includes` from the new mock headers, and write Bazel dependencies for the newly divided mock classes. (this step needs to be done manually) | ||
|
|
||
foreseeable marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 3. Run `replace_includes.py` to replace superfluous `#includes` in Envoy directory after dividing. It will also modify the corresponding Bazel `BUILD` file. | ||
|
|
||
| Example (to replace `#includes` after dividing mock header test/mocks/network/mocks.h): | ||
|
|
||
| ``` | ||
| cd ${ENVOY_SRCDIR} | ||
| python3 ${ENVOY_SRCDIR}/tools/envoy_headersplit/replace_includes.py -m network | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| #include "envoy/split" | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| namespace { | ||
|
|
||
| class Foo {}; | ||
|
|
||
| class Bar { | ||
| Foo getFoo(); | ||
| }; | ||
|
|
||
| class FooBar : Foo, Bar {}; | ||
|
|
||
| class DeadBeaf { | ||
| public: | ||
| int val(); | ||
| FooBar foobar; | ||
| }; | ||
| } // namespace |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| #include "envoy/split" | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| class Foo {}; | ||
|
|
||
| class Bar { | ||
| Foo getFoo(); | ||
| }; | ||
|
|
||
| class FooBar : Foo, Bar {}; | ||
|
|
||
| class DeadBeaf { | ||
| public: | ||
| int val(); | ||
| FooBar foobar; | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| #include "class_defn.h" | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| namespace { | ||
| Foo Bar::getFoo() { | ||
| Foo foo; | ||
| return foo; | ||
| } | ||
|
|
||
| int DeadBeaf::val() { return 42; } | ||
|
|
||
| DeadBeaf::DeadBeaf() = default; | ||
| } // namespace |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| #include "fail_mocks.h" | ||
|
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| #include <string> | ||
|
|
||
| #include "envoy/admin/v3/server_info.pb.h" | ||
| #include "envoy/config/core/v3/base.pb.h" | ||
|
|
||
| #include "common/singleton/manager_impl.h" | ||
|
|
||
| #include "gmock/gmock.h" | ||
| #include "gtest/gtest.h" | ||
|
|
||
| using testing::_; | ||
| using testing::Invoke; | ||
| using testing::Return; | ||
| using testing::ReturnPointee; | ||
| using testing::ReturnRef; | ||
| using testing::SaveArg; | ||
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
|
|
||
| MockConfigTracker::MockConfigTracker() { | ||
| ON_CALL(*this, add_(_, _)) | ||
| .WillByDefault(Invoke([this](const std::string& key, Cb callback) -> EntryOwner* { | ||
| EXPECT_TRUE(config_tracker_callbacks_.find(key) == config_tracker_callbacks_.end()); | ||
| config_tracker_callbacks_[key] = callback; | ||
| return new MockEntryOwner(); | ||
| })); | ||
| } | ||
| MockConfigTracker::~MockConfigTracker() = default; | ||
|
|
||
| MockListenerComponentFactory::MockListenerComponentFactory() | ||
| : socket_(std::make_shared<NiceMock<Network::MockListenSocket>>()) { | ||
| ON_CALL(*this, createListenSocket(_, _, _, _)) | ||
| .WillByDefault(Invoke([&](Network::Address::InstanceConstSharedPtr, Network::Socket::Type, | ||
| const Network::Socket::OptionsSharedPtr& options, | ||
| const ListenSocketCreationParams&) -> Network::SocketSharedPtr { | ||
| if (!Network::Socket::applyOptions(options, *socket_, | ||
| envoy::config::core::v3::SocketOption::STATE_PREBIND)) { | ||
| throw EnvoyException("MockListenerComponentFactory: Setting socket options failed"); | ||
| } | ||
| return socket_; | ||
| })); | ||
| } | ||
| MockListenerComponentFactory::~MockListenerComponentFactory() = default; | ||
|
|
||
| } // namespace Server | ||
| } // namespace Envoy |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| #pragma once | ||
foreseeable marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // NOLINT(namespace-envoy) | ||
|
|
||
| #include <chrono> | ||
| #include <cstdint> | ||
| #include <list> | ||
| #include <string> | ||
|
|
||
| namespace Envoy { | ||
| namespace Server { | ||
|
|
||
| class MockConfigTracker : public ConfigTracker { | ||
| public: | ||
| MockConfigTracker(); | ||
| ~MockConfigTracker() override; | ||
|
|
||
| struct MockEntryOwner : public EntryOwner {}; | ||
|
|
||
| MOCK_METHOD(EntryOwner*, add_, (std::string, Cb)); | ||
|
|
||
| // Server::ConfigTracker | ||
| MOCK_METHOD(const CbsMap&, getCallbacksMap, (), (const)); | ||
| EntryOwnerPtr add(const std::string& key, Cb callback) override { | ||
| return EntryOwnerPtr{add_(key, std::move(callback))}; | ||
| } | ||
|
|
||
| absl::node_hash_map<std::string, Cb> config_tracker_callbacks_; | ||
| }; | ||
|
|
||
| class MockListenerComponentFactory : public ListenerComponentFactory { | ||
| public: | ||
| MockListenerComponentFactory(); | ||
| ~MockListenerComponentFactory() override; | ||
|
|
||
| DrainManagerPtr | ||
| createDrainManager(envoy::config::listener::v3::Listener::DrainType drain_type) override { | ||
| return DrainManagerPtr{createDrainManager_(drain_type)}; | ||
| } | ||
| LdsApiPtr createLdsApi(const envoy::config::core::v3::ConfigSource& lds_config) override { | ||
| return LdsApiPtr{createLdsApi_(lds_config)}; | ||
| } | ||
|
|
||
| MOCK_METHOD(LdsApi*, createLdsApi_, (const envoy::config::core::v3::ConfigSource& lds_config)); | ||
| MOCK_METHOD(std::vector<Network::FilterFactoryCb>, createNetworkFilterFactoryList, | ||
| (const Protobuf::RepeatedPtrField<envoy::config::listener::v3::Filter>& filters, | ||
| Configuration::FilterChainFactoryContext& filter_chain_factory_context)); | ||
| MOCK_METHOD(std::vector<Network::ListenerFilterFactoryCb>, createListenerFilterFactoryList, | ||
| (const Protobuf::RepeatedPtrField<envoy::config::listener::v3::ListenerFilter>&, | ||
| Configuration::ListenerFactoryContext& context)); | ||
| MOCK_METHOD(std::vector<Network::UdpListenerFilterFactoryCb>, createUdpListenerFilterFactoryList, | ||
| (const Protobuf::RepeatedPtrField<envoy::config::listener::v3::ListenerFilter>&, | ||
| Configuration::ListenerFactoryContext& context)); | ||
| MOCK_METHOD(Network::SocketSharedPtr, createListenSocket, | ||
| (Network::Address::InstanceConstSharedPtr address, Network::Socket::Type socket_type, | ||
| const Network::Socket::OptionsSharedPtr& options, | ||
| const ListenSocketCreationParams& params)); | ||
| MOCK_METHOD(DrainManager*, createDrainManager_, | ||
| (envoy::config::listener::v3::Listener::DrainType drain_type)); | ||
| MOCK_METHOD(uint64_t, nextListenerTag, ()); | ||
|
|
||
| std::shared_ptr<Network::MockListenSocket> socket_; | ||
| }; | ||
| } // namespace Server | ||
| } // namespace Envoy | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| envoy_cc_test( | ||
| name = "async_client_impl_test", | ||
| srcs = ["async_client_impl_test.cc"], | ||
| deps = [ | ||
| ":common_lib", | ||
| "//source/common/buffer:buffer_lib", | ||
| "//source/common/http:async_client_lib", | ||
| "//source/common/http:context_lib", | ||
| "//source/common/http:headers_lib", | ||
| "//source/common/http:utility_lib", | ||
| "//source/extensions/upstreams/http/generic:config", | ||
| "//test/mocks:common_lib", | ||
| "//test/mocks/buffer:buffer_mocks", | ||
| "//test/mocks/http:http_mocks", | ||
| "//test/mocks/local_info:local_info_mocks", | ||
| "//test/mocks/router:router_mocks", | ||
| "//test/mocks/runtime:runtime_mocks", | ||
| "//test/mocks/stats:stats_mocks", | ||
| "//test/mocks/upstream:upstream_mocks", | ||
| "//test/test_common:test_time_lib", | ||
| "@envoy_api//envoy/config/core/v3:pkg_cc_proto", | ||
| "@envoy_api//envoy/config/route/v3:pkg_cc_proto", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also to Ashley's advice, I think code corpus examples might want to avoid depending on Envoy implementation and APIs, since they need to be maintained. Again, not the high order bit, but worth considering if you need to add any extra test cases. |
||
| ], | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| // NOLINT(namespace-envoy) | ||
| #include <chrono> | ||
| #include <cstdint> | ||
| #include <memory> | ||
| #include <string> | ||
|
|
||
| #include "envoy/config/core/v3/base.pb.h" | ||
| #include "envoy/config/route/v3/route_components.pb.h" | ||
|
|
||
| #include "common/buffer/buffer_impl.h" | ||
| #include "common/http/async_client_impl.h" | ||
| #include "common/http/context_impl.h" | ||
| #include "common/http/headers.h" | ||
| #include "common/http/utility.h" | ||
|
|
||
| #include "test/common/http/common.h" | ||
| #include "test/mocks/buffer/mocks.h" | ||
| #include "test/mocks/common.h" | ||
| #include "test/mocks/http/mocks.h" | ||
| #include "test/mocks/local_info/mocks.h" | ||
| #include "test/mocks/router/mocks.h" | ||
| #include "test/mocks/runtime/mocks.h" | ||
| #include "test/mocks/stats/mocks.h" | ||
| #include "test/mocks/upstream/mocks.h" | ||
| #include "test/test_common/printers.h" | ||
|
|
||
| ....useless stuff... | ||
|
|
||
| NiceMock<Upstream::MockClusterManager> | ||
| cm_; | ||
|
|
||
| ... uninteresting stuff.. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // your first c++ program | ||
| // NOLINT(namespace-envoy) | ||
| #include <iostream> | ||
|
|
||
| // random strings | ||
|
|
||
| #include "foo/bar" | ||
|
|
||
| class test { | ||
| test() { std::cout << "Hello World" << std::endl; } | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to confirm the pip3 trick is so you can bring in Clang as a Python dependency during test via requirements.txt? @lizan @PiotrSikora is there going to be some Wasm dependency that eventually gives us this more directly? I think what is here right now is fine for now though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes I have to install the
clangmodule so that I can use them at test.