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 <ermu...@microsoft.com> Authored: Wed Mar 7 16:29:24 2018 -0800 Committer: Andrew Schwartzmeyer <and...@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"));