Second attempt: xds client bootstrap file#20606
Conversation
|
@jtattermusch, this is still showing the same failure as the original PR: Please advise as to how I should fix this. Thanks. |
|
#20621 has been merged, and I re-ran the "Distribution Tests Linux (standalone subset)" test, which is now passing. |
AspirinSJL
left a comment
There was a problem hiding this comment.
Reviewed 32 of 32 files at r1, 3 of 3 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @apolcyn and @markdroth)
test/core/client_channel/xds_bootstrap_test.cc, line 91 at r1 (raw file):
bootstrap.node()->metadata, ::testing::ElementsAre( ::testing::Pair(::testing::StrEq("null"),
Why do we need to reorder the matches? They have to be in alphanumeric order?
markdroth
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! all files reviewed, all discussions resolved (waiting on @apolcyn)
test/core/client_channel/xds_bootstrap_test.cc, line 91 at r1 (raw file):
Previously, AspirinSJL (Juanli Shen) wrote…
Why do we need to reorder the matches? They have to be in alphanumeric order?
ElementsAre() checks the elements in the order in which they are returned by iterating over the container. Since maps are sorted containers, the entries have to be in order.
I could have used UnorderedElementsAre() instead, but being more specific seemed cleaner.
The first commit is a second attempt at #20380, which was reverted in #20593.
The second commit changes
xds_bootstrap_testto actually run in most of our tests, similar to what is done in #20513 forservice_config_test. This change also uncovered some bugs in the test, which are now fixed; I suspect this will also address #20541.This change is