-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
test: Split huge monolith mock header(test/upstream/mocks.h) to speed up test compilation #12048
Conversation
Signed-off-by: Muge Chen <mugechen@google.com>
\cc @ahedberg @bbarenblat |
@junr03 can you take a look at this as non-Googler? Thanks! |
Yep, will take a look with fresh eyes on monday |
Friendly ping @junr03 |
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.
Thanks for your patience, specially because reviewing this PR is actually not time consuming.
One question: is the intention to break apart the mock target here, and then update usage in a subsequent PR? Is that why this PR still has the massive target that now includes all the broken up targets?
One request: can you update the title of the PR to mention that this is for the upstream mocks, similar to how we did for the header inclusion reduction PRs?
Thanks!
Yes I will then update usage in subsequent PRs. The title has been changed, thanks for your suggestion. |
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.
Great to see this work progress, should greatly help build time reduction in aggregate!
… 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>
… 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>
…/upstream) (#12774) Commit Message: refactor header inclusion to speed up building Additional Description: This is the follow up PR for dividing the monolithic mock header test/mocks/upstream/mocks.h #12048 We refactored mock class include directives, this will reduce the target size and building time. Signed-off-by: Muge Chen <mugechen@google.com>
…/upstream) (envoyproxy#12774) Commit Message: refactor header inclusion to speed up building Additional Description: This is the follow up PR for dividing the monolithic mock header test/mocks/upstream/mocks.h envoyproxy#12048 We refactored mock class include directives, this will reduce the target size and building time. Signed-off-by: Muge Chen <mugechen@google.com> Signed-off-by: Clara Andrew-Wani <candrewwani@gmail.com>
Signed-off-by: Muge Chen mugechen@google.com
Commit Message: breakdown
test/mocks/upstream/mocks.h
into different mock classesAdditional Description:
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