test: Break down huge monolith mock header to improve test compilation performance #11797
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
ahedberg
suggested changes
Jun 29, 2020
test/mocks/server/drain_manager.cc
Outdated
| #include "gmock/gmock.h" | ||
| #include "gtest/gtest.h" | ||
|
|
||
| using testing::_; |
There was a problem hiding this comment.
As I mentioned on the last PR, these should be fully-qualified and inside the innermost working namespace, or removed if not being used.
There was a problem hiding this comment.
Thanks for let me know, now they should be fully qualified and stay inside the innermost namespace.
| MOCK_METHOD(std::string, name, (), (const, override)); | ||
| }; | ||
| } // namespace Configuration | ||
|
|
There was a problem hiding this comment.
Removing the blank lines between closing namespace brackets would be more consistent with the opening namespace brackets.
There was a problem hiding this comment.
Now they're removed
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
ahedberg
reviewed
Jun 30, 2020
test/mocks/server/admin_stream.h
Outdated
| @@ -0,0 +1,32 @@ | |||
| #pragma once | |||
|
|
|||
| #include <chrono> | |||
There was a problem hiding this comment.
Are we scripting the removal of extra #includes or doing it manually? We don't need most of these system headers in the new headers.
There was a problem hiding this comment.
I remove them manually, just forget to remove them. Thanks for pointing out
ahedberg
reviewed
Jun 30, 2020
test/mocks/server/mocks.cc
Outdated
| } // namespace Configuration | ||
| } // namespace Server | ||
| } // namespace Envoy | ||
| namespace Envoy {} |
There was a problem hiding this comment.
We can delete the .cc file.
ahedberg
suggested changes
Jun 30, 2020
test/mocks/server/mocks.h
Outdated
| #include "worker.h" | ||
| #include "worker_factory.h" | ||
|
|
||
| namespace Envoy {} |
There was a problem hiding this comment.
This empty namespace block can be removed.
Signed-off-by: Muge Chen <mugechen@google.com>
ahedberg
suggested changes
Jun 30, 2020
Signed-off-by: Muge Chen <mugechen@google.com>
|
cc @ahedberg
Now it should be fixed. Actually the empty namespace block in mocks.h from previous commits was used to make the formatter happy. now I found I can use no lint instead |
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
Signed-off-by: Muge Chen <mugechen@google.com>
ahedberg
approved these changes
Jul 6, 2020
lizan
approved these changes
Jul 6, 2020
htuch
pushed a commit
that referenced
this pull request
Jul 23, 2020
… up test compilation (#12048) breakdown test/mocks/upstream/mocks.h into different mock classes test/mocks/upstream/mocks.h is a wide-used mock header included by various test files. However it's very huge and most test files only used a small portion of it. Splitting it up into different mock classes will be helpful to reduce compilation time. (similar to #11797 ) Risk Level: low Testing: existing tests Docs Changes: N/A Release Notes: no Related Issues: #10917 Signed-off-by: Muge Chen <mugechen@google.com>
KBaichoo
pushed a commit
to KBaichoo/envoy
that referenced
this pull request
Jul 30, 2020
… up test compilation (envoyproxy#12048) breakdown test/mocks/upstream/mocks.h into different mock classes test/mocks/upstream/mocks.h is a wide-used mock header included by various test files. However it's very huge and most test files only used a small portion of it. Splitting it up into different mock classes will be helpful to reduce compilation time. (similar to envoyproxy#11797 ) Risk Level: low Testing: existing tests Docs Changes: N/A Release Notes: no Related Issues: envoyproxy#10917 Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: Kevin Baichoo <kbaichoo@google.com>
scheler
pushed a commit
to scheler/envoy
that referenced
this pull request
Aug 4, 2020
…n performance (envoyproxy#11797) Commit Message: breakdown `test/mocks/server/mocks.h` into different mock classes Additional Description: `test/mocks/server/mocks.h` is a wide-used mock header included by various test files. However it's very huge and most test files only used a small portion of it. Splitting it up into different mock classes will be helpful to reduce compilation time. Risk Level: low Testing: existing tests Docs Changes: N/A Release Notes: no Related Issues: envoyproxy#10917 Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: scheler <santosh.cheler@appdynamics.com>
chaoqin-li1123
pushed a commit
to chaoqin-li1123/envoy
that referenced
this pull request
Aug 7, 2020
… up test compilation (envoyproxy#12048) breakdown test/mocks/upstream/mocks.h into different mock classes test/mocks/upstream/mocks.h is a wide-used mock header included by various test files. However it's very huge and most test files only used a small portion of it. Splitting it up into different mock classes will be helpful to reduce compilation time. (similar to envoyproxy#11797 ) Risk Level: low Testing: existing tests Docs Changes: N/A Release Notes: no Related Issues: envoyproxy#10917 Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: chaoqinli <chaoqinli@google.com>
htuch
pushed a commit
that referenced
this pull request
Aug 28, 2020
We need to reduce resource consumption of test compilation by simplifying mock library inclusions. #10917 One way to do that is to break the monolithic mock headers into different mock classes and then refactor the code base (like #12053 #12051 #11797) It's hard to refactor them manually. So I wrote headersplit, a tool based on libclang and python to help me divide the monolithic mock header and replace includes after dividing automatically. This tool can also generate building time comparison between before/after refactoring. Risk level: low Testing: Build succeeds. Signed-off-by: Muge Chen <mugechen@google.com>
clarakosi
pushed a commit
to clarakosi/envoy
that referenced
this pull request
Sep 3, 2020
) We need to reduce resource consumption of test compilation by simplifying mock library inclusions. envoyproxy#10917 One way to do that is to break the monolithic mock headers into different mock classes and then refactor the code base (like envoyproxy#12053 envoyproxy#12051 envoyproxy#11797) It's hard to refactor them manually. So I wrote headersplit, a tool based on libclang and python to help me divide the monolithic mock header and replace includes after dividing automatically. This tool can also generate building time comparison between before/after refactoring. Risk level: low Testing: Build succeeds. Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Commit Message: breakdown
test/mocks/server/mocks.hinto different mock classesAdditional Description:
test/mocks/server/mocks.his a wide-used mock header included by various test files. However it's very huge and most test files only used a small portion of it. Splitting it up into different mock classes will be helpful to reduce compilation time.Risk Level: low
Testing: existing tests
Docs Changes: N/A
Release Notes: no
Related Issues: #10917