Skip to content

test: separate AdsIntegrationTest to a library #7581

Merged
zuercher merged 5 commits intoenvoyproxy:masterfrom
asraa:adsintegrationreuse
Jul 16, 2019
Merged

test: separate AdsIntegrationTest to a library #7581
zuercher merged 5 commits intoenvoyproxy:masterfrom
asraa:adsintegrationreuse

Conversation

@asraa
Copy link
Copy Markdown
Contributor

@asraa asraa commented Jul 15, 2019

Moves AdsIntegrationTest to it's own file. I'd like to reuse the class for the xDS fuzzer (#7543), since the setup and utilities are the same.

Risk Level: Low

Signed-off-by: Asra Ali asraa@google.com

Signed-off-by: Asra Ali <asraa@google.com>
Copy link
Copy Markdown
Member

@zuercher zuercher left a comment

Choose a reason for hiding this comment

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

This looks good, I just want to see if we can trim down the dependencies for in the BUILD file to those actually used.

"@envoy_api//envoy/api/v2:eds_cc",
"@envoy_api//envoy/api/v2:lds_cc",
"@envoy_api//envoy/api/v2:rds_cc",
"@envoy_api//envoy/service/discovery/v2:ads_cc",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we trim these dependencies (and the ones for ads_integration_lib) down to the set that each actually uses?

For example, it doesn't look like the the mocks are needed for either of these (you can probably even remove the #include directives) and it doesn't look like the library needs any of the API except ADS.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Removed unnecessary ones, but I think the library does need the API since the return values of build* are envoy::api::v2::Listener/Cluster, right? Technically they're included transitively through the config dump proto though. Not sure if guidelines are generally to explicitly include them.

@zuercher zuercher self-assigned this Jul 15, 2019
asraa added 4 commits July 15, 2019 13:41
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
Signed-off-by: Asra Ali <asraa@google.com>
@zuercher zuercher merged commit 6ca8500 into envoyproxy:master Jul 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants