Skip to content

Commit

Permalink
Deal with even more flaky tests! (#928)
Browse files Browse the repository at this point in the history
  • Loading branch information
BillyONeal authored Oct 18, 2018
1 parent 328646a commit 95409c5
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 38 deletions.
3 changes: 3 additions & 0 deletions Release/include/cpprest/http_msg.h
Original file line number Diff line number Diff line change
Expand Up @@ -878,6 +878,9 @@ class _http_request final : public http::details::http_msg_base, public std::ena
http::method m_method;

// Tracks whether or not a response has already been started for this message.
// 0 = No reply sent
// 1 = Usual reply sent
// 2 = Reply aborted by another thread; e.g. server shutdown
pplx::details::atomic_long m_initiated_response;

std::unique_ptr<http::details::_http_server_context> m_server_context;
Expand Down
17 changes: 12 additions & 5 deletions Release/src/http/listener/http_listener_msg.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pplx::task<void> details::_http_request::_reply_impl(http_response response)
pplx::task<void> details::_http_request::_reply_if_not_already(status_code status)
{
const long expected = 0;
const long desired = 1;
const long desired = 2;
if (pplx::details::atomic_compare_exchange(m_initiated_response, desired, expected) == expected)
{
return _reply_impl(http_response(status));
Expand All @@ -68,11 +68,18 @@ pplx::task<void> details::_http_request::_reply_if_not_already(status_code statu

pplx::task<void> details::_http_request::reply(const http_response &response)
{
if(pplx::details::atomic_increment(m_initiated_response) != 1l)
{
throw http_exception(U("Error: trying to send multiple responses to an HTTP request"));
const long expected = 0;
const long desired = 1;
switch (pplx::details::atomic_compare_exchange(m_initiated_response, desired, expected)) {
case 0:
return _reply_impl(response); // success
case 1:
throw http_exception(U("Error: trying to send multiple responses to an HTTP request"));
case 2:
return pplx::task_from_result(); // already handled
default:
abort();
}
return _reply_impl(response);
}

}} // namespace web::http
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ TEST_FIXTURE(uri_address, body_types)
p_request->reply(200);
});
http_asserts::assert_response_equals(client.request(msg).get(), status_codes::OK);

// string - no content type.
msg = http_request(method);
msg.set_body(std::move(str_move_body));
Expand Down Expand Up @@ -143,7 +143,7 @@ TEST(set_body_string_with_charset)
{
http_request request;
VERIFY_THROWS(request.set_body(
::utility::conversions::to_utf16string("body_data"),
::utility::conversions::to_utf16string("body_data"),
::utility::conversions::to_utf16string("text/plain;charset=utf-16")), std::invalid_argument);
}

Expand Down Expand Up @@ -236,7 +236,7 @@ TEST_FIXTURE(uri_address, set_body_with_charset)
http_request msg(methods::PUT);
msg.set_body("datadatadata", "text/plain;charset=us-ascii");
VERIFY_THROWS(msg.set_body(
::utility::conversions::to_utf16string("datadatadata"),
::utility::conversions::to_utf16string("datadatadata"),
::utility::conversions::to_utf16string("text/plain;charset=us-ascii")), std::invalid_argument);
}

Expand Down Expand Up @@ -321,4 +321,4 @@ TEST_FIXTURE(uri_address, reuse_request)

}

}}}}
}}}}
2 changes: 1 addition & 1 deletion Release/tests/functional/http/client/compression_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1108,7 +1108,7 @@ SUITE(compression_tests)
}
else
{
memcpy(vv.data(), v.data(), v.size());
std::copy(v.begin(), v.end(), vv.begin());
got = v.size();
}
VERIFY_ARE_EQUAL(buffer_size, got);
Expand Down
56 changes: 28 additions & 28 deletions Release/tests/functional/http/client/progress_handler_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ TEST_FIXTURE(uri_address, set_progress_handler_no_bodies)

http_request msg(mtd);
msg.set_progress_handler(
[&](message_direction::direction direction, utility::size64_t so_far)
{
[&](message_direction::direction direction, utility::size64_t so_far)
{
calls += 1;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
});

test_http_server::scoped_server scoped(m_uri);
Expand Down Expand Up @@ -123,19 +123,19 @@ TEST_FIXTURE(uri_address, set_progress_handler_download)

http_client client(m_uri, config);
const method mtd = methods::GET;

utility::size64_t upsize = 4711u, downsize = 4711u;
int calls = 0;

http_request msg(mtd);
msg.set_progress_handler(
[&](message_direction::direction direction, utility::size64_t so_far)
{
[&](message_direction::direction direction, utility::size64_t so_far)
{
calls += 1;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
});

const size_t repeats = 6000;
Expand Down Expand Up @@ -179,19 +179,19 @@ TEST_FIXTURE(uri_address, set_progress_handler_upload_and_download)
const size_t repeats = 5500;
for (size_t i = 0; i < repeats; ++i)
data.append(U("abcdefghihklmnopqrstuvwxyz"));

utility::size64_t upsize = 4711u, downsize = 4711u;
int calls = 0;

http_request msg(mtd);
msg.set_progress_handler(
[&](message_direction::direction direction, utility::size64_t so_far)
{
[&](message_direction::direction direction, utility::size64_t so_far)
{
calls += 1;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
});

msg.set_body(data);
Expand Down Expand Up @@ -274,27 +274,27 @@ TEST_FIXTURE(uri_address, set_progress_handler_request_timeout)
const size_t repeats = 5500;
for (size_t i = 0; i < repeats; ++i)
data.append(U("abcdefghihklmnopqrstuvwxyz"));

utility::size64_t upsize = 4711u, downsize = 4711u;
int calls = 0;

http_request msg(mtd);
// We should never see this handler called for download, but for upload should still happen, since
// there's a server (just not a very responsive one) and we're sending data to it.
msg.set_progress_handler(
[&](message_direction::direction direction, utility::size64_t so_far)
{
[&](message_direction::direction direction, utility::size64_t so_far)
{
calls += 1;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
if (direction == message_direction::upload)
upsize = so_far;
else
downsize = so_far;
});

msg.set_body(data);
auto t = scoped.server()->next_request();
auto response = client.request(msg);

#ifdef __APPLE__
// CodePlex 295
VERIFY_THROWS(response.get(), http_exception);
Expand Down

0 comments on commit 95409c5

Please sign in to comment.