test: merge delta CDS integration test into normal CDS#6214
test: merge delta CDS integration test into normal CDS#6214htuch merged 6 commits intoenvoyproxy:masterfrom
Conversation
Signed-off-by: Fred Douglas <fredlas@google.com>
htuch
left a comment
There was a problem hiding this comment.
Nice cleanup. A few questions and needs master merge.
/wait
Signed-off-by: Fred Douglas <fredlas@google.com>
Signed-off-by: Fred Douglas <fredlas@google.com>
htuch
left a comment
There was a problem hiding this comment.
Sorry for the delayed response, I was doing my Inbox -> Gmail migration over the past few days and some GH updates were lost.
/wait:any
|
Tests should be straightforward and clear, even if it means some repetition. If the repetition gets unbearable, then you're forced to get fancier (and harder for newcomers to understand), but until then simple is best. I do not have current plans to add tests that would use different configs, so I would prefer to see the current changes stay simple. Similarly, there are only two roughly similar tests between normal and delta. I don't think that adding if(param) blocks is worth it for what currently exists. |
|
@fredlas this is fine with me,but I would like to get @alyssawilk to also sign-off, since she is the expert on config helper and its architecture. |
|
@fredlas OK, I think we can agree that it's fine to have some replication for delta and non-delta. However, let's continue to discuss the hardcoded bootstrap YAML, as I agree with @alyssawilk that this is a good opportunity to reduce technical debt. |
…a_and_cds Signed-off-by: Fred Douglas <fredlas@google.com>
Description: Refactor requested in #5466 by @htuch. Move all of delta_cds_integration_test.cc into cds_integration_test.cc, with some cute code sharing. Move the config fragment strings into test/config/utility.cc.
Risk Level: none