Skip to content

Test framework: fix the old test bugs in llmessage's and llcorehttp's tests #456

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
RyeMutt merged 3 commits into from
Oct 12, 2026
Merged

Test framework: fix the old test bugs in llmessage's and llcorehttp's tests #456

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f71a0fa
llmessage's packing and message builder tests check what they say the…
RyeMutt Oct 12, 2026
ca1d992
llmessage's io, buffer and dispatcher tests stop reading what they ne…
RyeMutt Oct 12, 2026
a4623f5
llcorehttp's tests check what they say they check
RyeMutt Oct 12, 2026
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
6 changes: 3 additions & 3 deletions indra/llcorehttp/tests/bufferarray_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,13 +207,13 @@ TEST_CASE("BufferArray Tests: BufferArray multiple writes - sequential reads", "
UNSCOPED_INFO("nothing written past what was read");
CHECK(buffer[4] == 'X');

// Read some more without repositioning
// Read on from where the last read stopped
memset(buffer, 'X', sizeof(buffer));
len = ba->read(12, buffer, sizeof(buffer));
CHECK((str1_len - 2) == len);
CHECK(strncmp(buffer, str1+2, str1_len-2) == 0);
UNSCOPED_INFO("nothing written past what was read");
CHECK(buffer[str1_len-1] == 'X');
CHECK(buffer[str1_len-2] == 'X');

// release the implicit reference, causing the object to be released
ba->release();
Expand Down Expand Up @@ -244,7 +244,7 @@ TEST_CASE("BufferArray Tests: BufferArray overwrite spanning blocks and appendin
len = ba->write(8, str2, str2_len);
CHECK(str2_len == len);

// Leave position and read verifying content
// A zero-length read at the end of the data reads nothing
memset(buffer, 'X', sizeof(buffer));
len = ba->read(8 + str2_len, buffer, 0);
CHECK(len == 0);
Expand Down
23 changes: 14 additions & 9 deletions indra/llcorehttp/tests/bufferstream_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ typedef BufferArrayStreamBuf::traits_type tst_traits_t;

TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over a NULL BufferArray fails every call", "[BufferStream_Tests]")
{
// create a new ref counted object with an implicit reference
// a stream buffer over no BufferArray
BufferArrayStreamBuf * bsb = new BufferArrayStreamBuf(NULL);

// Not much will work with a NULL
Expand All @@ -58,53 +58,55 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf over a NULL BufferArray fail
CHECK(bsb->xsputn("blah", 4) == 0);
CHECK(bsb->seekoff(0, std::ios_base::beg, std::ios_base::in) == std::streampos(-1));

// release the implicit reference, causing the object to be released
// the stream buffer is not ref counted
delete bsb;
bsb = NULL;
}


TEST_CASE("BufferStream Tests: BufferArrayStream over a NULL BufferArray starts out good", "[BufferStream_Tests]")
{
// create a new ref counted object with an implicit reference
// a stream over no BufferArray
BufferArrayStream * bas = new BufferArrayStream(NULL);

// Not much will work with a NULL here
CHECK_FALSE(bas->eof());
CHECK_FALSE(bas->fail());
CHECK(bas->good());

// release the implicit reference, causing the object to be released
// the stream is not ref counted
delete bas;
bas = NULL;
}


TEST_CASE("BufferStream Tests: BufferArrayStreamBuf construction with empty BufferArray", "[BufferStream_Tests]")
TEST_CASE("BufferStream Tests: BufferArrayStreamBuf takes a reference on an empty BufferArray", "[BufferStream_Tests]")
{
// create a new ref counted BufferArray with implicit reference
BufferArray * ba = new BufferArray;
BufferArrayStreamBuf * bsb = new BufferArrayStreamBuf(ba);
CHECK(ba->getRefCount() == 2);

// I can release my ref on the BA
ba->release();
ba = NULL;

// release the implicit reference, causing the object to be released
// the stream buffer is not ref counted; deleting it drops the BufferArray's last reference
delete bsb;
bsb = NULL;
}


TEST_CASE("BufferStream Tests: BufferArrayStream construction with empty BufferArray", "[BufferStream_Tests]")
TEST_CASE("BufferStream Tests: BufferArrayStream holds a reference on an empty BufferArray while it lives", "[BufferStream_Tests]")
{
// create a new ref counted BufferArray with implicit reference
BufferArray * ba = new BufferArray;

{
// create a new ref counted object with an implicit reference
BufferArrayStream bas(ba);
CHECK(ba->getRefCount() == 2);
}
CHECK(ba->getRefCount() == 1);

// release the implicit reference, causing the object to be released
ba->release();
Expand Down Expand Up @@ -151,9 +153,12 @@ TEST_CASE("BufferStream Tests: BufferArrayStreamBuf reads and appends and seeks
UNSCOPED_INFO("showmanyc() picks up seekoff");
CHECK(bsb->showmanyc() == (c_len + 5 - 4));
CHECK(bsb->seekoff(0, std::ios_base::end, std::ios_base::in) == std::streampos(c_len + 4));
// The seek to the end leaves the read position on the last character, the '!',
// so pbackfail() refuses the '!' and puts back the 'a' before it
CHECK(tst_traits_t::eof() == bsb->pbackfail('!'));
CHECK(bsb->pbackfail('a') == 'a');

// release the implicit reference, causing the object to be released
// the stream buffer is not ref counted; deleting it drops the BufferArray's last reference
delete bsb;
bsb = NULL;
}
Expand Down
104 changes: 85 additions & 19 deletions indra/llcorehttp/tests/httprequest_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@

#include <curl/curl.h>
#include "alregex.h"
#include <chrono>
#include <map>
#include <sstream>
#include <vector>

#include "llcleanup.h"
#include "llproxy.h"
Expand Down Expand Up @@ -820,7 +823,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET to the peer is a
}


TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange with a zero range is answered 200", "[HttpRequest_Tests]")
TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a GET with a byte range sends its Range header and takes a 200 for the whole body", "[HttpRequest_Tests]")
{
ScopedCurlInit ready;

Expand All @@ -835,6 +838,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi
mHandlerCalls = 0;

HttpRequest * req = NULL;
HttpOptions::ptr_t opts;

try
{
Expand All @@ -848,13 +852,22 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi
// create a new ref counted object with an implicit reference
req = new HttpRequest();

// Issue a GET that *can* connect
opts = HttpOptions::ptr_t(new HttpOptions());
opts->setWantHeaders(true);

// Issue a GET for 25 bytes from offset 100. The peer reflects the
// Range header it was sent and, ignoring it, answers 200 with the
// whole body, which has no Content-Range for llcorehttp to check.
mStatus = HttpStatus(200);
handler.mHeadersRequired.push_back(
regex_container_t::value_type(
ALRegex("X-Reflect-range", ALRegex::ICASE),
ALRegex("bytes=100-124", ALRegex::ICASE)));
HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
url_base,
0,
0,
HttpOptions::ptr_t(),
url_base + "reflect/",
100,
25,
opts,
HttpHeaders::ptr_t(),
handlerp);
REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID);
Expand All @@ -873,6 +886,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi

// Okay, request a shutdown of the servicing thread
mStatus = HttpStatus();
handler.mHeadersRequired.clear();
handle = req->requestStopThread(handlerp);
REQUIRE(handle != LLCORE_HTTP_HANDLE_INVALID);

Expand All @@ -897,6 +911,9 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi
}
CHECK(HttpService::isStopped());

// release options
opts.reset();

// release the request object
delete req;
req = NULL;
Expand All @@ -909,6 +926,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: requestGetByteRange wi
catch (...)
{
stop_thread(req);
opts.reset();
delete req;
HttpRequest::destroyService();
throw;
Expand Down Expand Up @@ -945,7 +963,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a PUT with a body to t
// create a new ref counted object with an implicit reference
req = new HttpRequest();

// Issue a GET that *can* connect
// Issue a PUT that *can* connect
static const char * body_text("Now is the time for all good men...");
body->append(body_text, strlen(body_text));
mStatus = HttpStatus(200);
Expand Down Expand Up @@ -1050,7 +1068,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a POST with a body to
// create a new ref counted object with an implicit reference
req = new HttpRequest();

// Issue a GET that *can* connect
// Issue a POST that *can* connect
static const char * body_text("Now is the time for all good men...");
body->append(body_text, strlen(body_text));
mStatus = HttpStatus(200);
Expand Down Expand Up @@ -1883,7 +1901,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: a POST sends default h
ALRegex(".*", ALRegex::ICASE)));
handler.mHeadersDisallowed.push_back(
regex_container_t::value_type(
ALRegex("X-Reflect-transfer_encoding", ALRegex::ICASE),
ALRegex("X-Reflect-transfer-encoding", ALRegex::ICASE),
ALRegex(".*chunked.*", ALRegex::ICASE)));
HttpHandle handle = req->requestPost(HttpRequest::DEFAULT_POLICY_ID,
url_base + "reflect/",
Expand Down Expand Up @@ -2401,7 +2419,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: headers given to a POS
ba = new BufferArray;
ba->append(msg, strlen(msg));

// Issue a default POST
// Issue a POST with modified headers
mStatus = HttpStatus(200);
handler.mHeadersRequired.push_back(
regex_container_t::value_type(
Expand Down Expand Up @@ -2613,7 +2631,7 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: the last of several Co
ba = new BufferArray;
ba->append(msg, strlen(msg));

// Issue a default PUT
// Issue a PUT with several Content-Types
mStatus = HttpStatus(200);
handler.mHeadersRequired.push_back(
regex_container_t::value_type(
Expand Down Expand Up @@ -2936,19 +2954,49 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: BUG-2295 a 206 with a
}
}

TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with good and bad Retry-After headers complete", "[HttpRequest_Tests]")
// Records, by URL, when each response was delivered and the retries it reports
class RetryRecordingHandler : public TestHandler2
{
public:
struct Delivery
{
std::chrono::steady_clock::time_point mAt;
unsigned int mRetries;
unsigned int m503Retries;
};

RetryRecordingHandler(HttpRequestTestData * state,
const std::string & name)
: TestHandler2(state, name)
{}

void onCompleted(HttpHandle handle, HttpResponse * response) override
{
TestHandler2::onCompleted(handle, response);

Delivery delivery{ std::chrono::steady_clock::now(), 0, 0 };
response->getRetries(&delivery.mRetries, &delivery.m503Retries);
mDeliveries[response->getRequestURL()] = delivery;
}

std::map<std::string, Delivery> mDeliveries;
};

TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 are retried once and a good Retry-After holds the retry back", "[HttpRequest_Tests]")
{
ScopedCurlInit ready;

// This tests mainly that the code doesn't fall over if
// various well- and mis-formed Retry-After headers are
// sent along with the response. Direct inspection of
// the parsing result isn't supported.
// Each of the peer's seven 503 answers, /503/0/ to /503/6/, carries
// a different Retry-After, well- or mis-formed, and the code must not
// fall over on any of them. The parsing result can't be inspected,
// but its effect can: only /503/0/'s "2" is a delay llcorehttp takes,
// and it holds that retry back two seconds where the others retry
// after the default backoff of one.

// Handler can be stack-allocated *if* there are no dangling
// references to it after completion of this method.
// Create before memory record as the string copy will bump numbers.
TestHandler2 handler(this, "handler");
RetryRecordingHandler handler(this, "handler");
LLCore::HttpHandler::ptr_t handlerp(&handler, NoOpDeletor);
std::string url_base(get_base_url() + "/503/"); // path to 503 generators
mHandlerCalls = 0;
Expand All @@ -2972,13 +3020,16 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with
opts->setRetries(1); // Retry once only
opts->setUseRetryAfter(true); // Try to parse the retry-after header

// Issue a GET that 503s with valid retry-after
// Issue a GET to each of the peer's 503 answers
mStatus = HttpStatus(503);
int url_limit(6);
const int url_limit(7);
std::vector<std::string> urls;
const auto issued(std::chrono::steady_clock::now());
for (int i(0); i < url_limit; ++i)
{
std::ostringstream url;
url << url_base << i << "/";
urls.push_back(url.str());
HttpHandle handle = req->requestGetByteRange(HttpRequest::DEFAULT_POLICY_ID,
url.str(),
0,
Expand All @@ -3004,6 +3055,21 @@ TEST_CASE_METHOD(HttpRequestTestData, "HttpRequest Tests: GETs answered 503 with
CHECK(count < limit);
CHECK(mHandlerCalls == url_limit);

// Each GET was retried once, as a 503, whatever its Retry-After said,
// and the retry of the one with "Retry-After: 2" came no sooner than that
for (int i(0); i < url_limit; ++i)
{
CAPTURE(i);
const auto delivery(handler.mDeliveries.find(urls[i]));
REQUIRE(delivery != handler.mDeliveries.end());
CHECK(delivery->second.mRetries == 1u);
CHECK(delivery->second.m503Retries == 1u);
if (0 == i)
{
CHECK(delivery->second.mAt - issued >= std::chrono::milliseconds(1900));
}
}

// Okay, request a shutdown of the servicing thread
mStatus = HttpStatus();
mHandlerCalls = 0;
Expand Down
6 changes: 3 additions & 3 deletions indra/llcorehttp/tests/httpstatus_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,18 +133,18 @@ TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: toString says something
CHECK_FALSE(msg.empty());
}

TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: statuses of different types compare unequal", "[HttpStatus_Tests]")
TEST_CASE_METHOD(HttpStatusTestData, "HttpStatus Tests: distinct statuses compare unequal when both succeed or both fail", "[HttpStatus_Tests]")
{
// Make certain equality/inequality tests do not pass
// through the bool conversion. Distinct successful
// and error statuses should compare unequal.
// statuses, and distinct error statuses, should compare unequal.

HttpStatus status1(HttpStatus::LLCORE, HE_SUCCESS);
HttpStatus status2(HttpStatus::EXT_CURL_EASY, HE_SUCCESS);
CHECK(status1 != status2);

status1 = HttpStatus(HttpStatus::LLCORE, HE_REPLY_ERROR);
status1 = HttpStatus(HttpStatus::LLCORE, HE_SHUTTING_DOWN);
status2 = HttpStatus(HttpStatus::LLCORE, HE_SHUTTING_DOWN);

CHECK(status1 != status2);
}
Expand Down
Loading
Loading