Skip to content

Make DnsFilterTest.RandomizeFirstAnswerTest less brittle #39464

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.

Sign up for GitHub

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

Merged
yanavlasov merged 1 commit into from
May 15, 2025
Merged

Make DnsFilterTest.RandomizeFirstAnswerTest less brittle #39464

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions test/extensions/filters/udp/dns_filter/dns_filter_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1910,12 +1910,6 @@ TEST_F(DnsFilterTest, InvalidShortBufferTest) {
}

TEST_F(DnsFilterTest, RandomizeFirstAnswerTest) {
#if defined(__linux__) && defined(__s390x__)
// Skip on s390x because this test incorrectly depends on the ordering of
// addresses that happens to work on other platforms.
// See https://github.com/envoyproxy/envoy/pull/24330
GTEST_SKIP() << "Skipping RandomizeFirstAnswerTest on s390x";
#endif
InSequence s;

setup(forward_query_off_config);
Expand All @@ -1924,6 +1918,9 @@ TEST_F(DnsFilterTest, RandomizeFirstAnswerTest) {
const std::string query =
Utils::buildQueryForDomain(domain, DNS_RECORD_TYPE_A, DNS_RECORD_CLASS_IN);
ASSERT_FALSE(query.empty());
// Generate a new "random" value different from the original 3 to make the retrieval and insertion
// order in the response_ctx_->answers_ different.
ON_CALL(random_, random()).WillByDefault(Return(5));
sendQueryFromClient("10.0.0.1:1000", query);

response_ctx_ = ResponseValidator::createResponseContext(udp_response_, counters_);
Expand All @@ -1933,10 +1930,16 @@ TEST_F(DnsFilterTest, RandomizeFirstAnswerTest) {
// Although 16 addresses are defined, only 8 are returned
EXPECT_EQ(8, response_ctx_->answers_.size());

// We shuffle the list of addresses when we read the config, and in the case of more than
// 8 defined addresses, we randomize the initial starting index. We should not end up with
// the first answer being the first defined address, or the answers appearing in the same
// order as they are defined.
// DNS filter shuffles the list of addresses when reading the config, and in the case of more than
// 8 defined addresses, it randomizes the initial starting index.
// There is 1/16 chance that the answer will return the initial 8 addresses, however since the
// test is using the same "random" number, the set of addresses in the answer is well defined, and
// should not be the initial 8 addresses.
//
// NOTE: this depends on the std::unordered_multimap order of retrieval, which is unspecified in
// the standard. It is possible that a future version of STL , will produce this exact order when
// retrieving elements starting with index 5 hardcoded in this test. In this case the retrieval
// order can be tweaked by making RNG return a different number above.
const std::list<std::string> defined_order{"10.0.16.1", "10.0.16.2", "10.0.16.3", "10.0.16.4",
"10.0.16.5", "10.0.16.6", "10.0.16.7", "10.0.16.8"};
auto defined_answer_iter = defined_order.begin();
Expand Down