mesos git commit: Fixed `HTTPTest.QueryEncodeDecode`.

[an...@apache.org]

Repository: mesos
Updated Branches:
  refs/heads/master 0a4a900e5 -> 66bae088d


Fixed `HTTPTest.QueryEncodeDecode`.

This commit fixes MESOS-8551.

Stout's `hashmap` is a wrapper around `std::unordered_map`. The
Standard Template Library places no restriction on the iteration order
for the elements of an `unordered_map`. Because of this, different
implementations of `unordered_map` can and do iterate the elements in
different orders. The implementation of `http::query::encode` uses the
iteration order of `std::unordered_map` to encode the elements.

The test case has two items in the `unordered_map`, which makes it
possible for `encode` to return either of two different orders. The
solution in this fix tests that the return value from `encode` matches
either of the possible orders. This was not previously an issue
because the implementations of GCC and Clang were consistent; however,
MSVC happens to encode in the order. This is not incorrect, just
different.

This patch also added a second test to ensure that either possible
output of `encode` would correctly decode.

Review: https://reviews.apache.org/r/65577/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/66bae088
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/66bae088
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/66bae088

Branch: refs/heads/master
Commit: 66bae088de61ae0cc3f9c5fac6f4b6f9695a27d1
Parents: 0a4a900
Author: Eric Mumau <er...@microsoft.com>
Authored: Wed Mar 7 16:29:24 2018 -0800
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
Committed: Wed Mar 7 16:50:57 2018 -0800

----------------------------------------------------------------------
 3rdparty/libprocess/src/tests/http_tests.cpp | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/66bae088/3rdparty/libprocess/src/tests/http_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/src/tests/http_tests.cpp b/3rdparty/libprocess/src/tests/http_tests.cpp
index 4a7e81e..4661f2e 100644
--- a/3rdparty/libprocess/src/tests/http_tests.cpp
+++ b/3rdparty/libprocess/src/tests/http_tests.cpp
@@ -1437,11 +1437,7 @@ TEST(HTTPConnectionTest, RequestStreaming)
 }
 
 
-// TODO(hausdorff): This test seems to create inconsistent (though not
-// incorrect) results across platforms. Fix and enable the test on Windows. In
-// particular, the encoding in the 3rd example puts the first variable into the
-// query string before the second, but we expect the reverse. See MESOS-5814.
-TEST_P_TEMP_DISABLED_ON_WINDOWS(HTTPTest, QueryEncodeDecode)
+TEST_P(HTTPTest, QueryEncodeDecode)
 {
   // If we use Type<a, b> directly inside a macro without surrounding
   // parenthesis the comma will be eaten by the macro rather than the
@@ -1454,9 +1450,13 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(HTTPTest, QueryEncodeDecode)
   EXPECT_EQ("foo=bar",
             http::query::encode(HashmapStringString({{"foo", "bar"}})));
 
-  EXPECT_EQ("c%7E%2Fasdf=%25asdf&a()=b%2520",
-            http::query::encode(
-                HashmapStringString({{"a()", "b%20"}, {"c~/asdf", "%asdf"}})));
+  // Because `http::query::encode` is implemented with
+  // `std::unsorted_map`, it can return two possible strings since the
+  // STL does not require a particular element iteration order.
+  const string encoded = http::query::encode(
+      HashmapStringString({{"a()", "b%20"}, {"c~/asdf", "%asdf"}}));
+  EXPECT_TRUE(encoded == "c%7E%2Fasdf=%25asdf&a()=b%2520" ||
+              encoded == "a()=b%2520&c%7E%2Fasdf=%25asdf");
 
   EXPECT_EQ("d",
             http::query::encode(HashmapStringString({{"d", ""}})));
@@ -1471,9 +1471,16 @@ TEST_P_TEMP_DISABLED_ON_WINDOWS(HTTPTest, QueryEncodeDecode)
   EXPECT_SOME_EQ(HashmapStringString({{"foo", "bar"}}),
                  http::query::decode("foo=bar"));
 
+
+  // Again, because the iteration order of `std::unsorted_map` is
+  // unspecified, we must test that `http::query::decode` can
+  // correctly decode both encoded orderings.
   EXPECT_SOME_EQ(HashmapStringString({{"a()", "b%20"}, {"c~/asdf", "%asdf"}}),
                  http::query::decode("c%7E%2Fasdf=%25asdf&a()=b%2520"));
 
+  EXPECT_SOME_EQ(HashmapStringString({{"a()", "b%20"}, {"c~/asdf", "%asdf"}}),
+                 http::query::decode("a()=b%2520&c%7E%2Fasdf=%25asdf"));
+
   EXPECT_SOME_EQ(HashmapStringString({{"d", ""}}),
                  http::query::decode("d"));