Fix the xrpl.net unit test

[a1q123456]

High Level Overview of Change

This PR makes the read function call in handleConnection async, adds a new class TestSink to help debugging, and adds a new target xrpl.tests.helpers to put the helper class in.

Context of Change

Our Trigger job fails almost everyday because of two unit tests. The first issue is that it takes forever for the unit tests to finish, and the second issue is we get a time out error in the xrpl.net test case.

I investigated the second issue and found out that the synchronous function read blocks the connection callback on the client side. As the read function expect some data that is supposed to be sent in the connection callback, we're getting a deadlock here. By making the read function asynchronous, the connection callback will have a chance to be called so that the read function can consume the data eventually.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Performance (increase or change in throughput and/or latency)
• Tests (you added tests for code that already exists, or your new feature included in this PR)
• Documentation update
• Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
• Release

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.4%. Comparing base (b90a843) to head (c6235dd).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff           @@
##           develop   #6241   +/-   ##
=======================================
  Coverage     79.4%   79.4%           
=======================================
  Files          839     839           
  Lines        71625   71625           
  Branches      8237    8236    -1     
=======================================
  Hits         56855   56855           
  Misses       14770   14770           

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bthomee]

Instead of creating a whole new sink, can you use any of the other existing sinks for this? e.g. StreamSink in src/test/unit_test/SuiteJournal.h, TestSink in src/test/server/Server_test.cpp, ...

[a1q123456]

We can't because those things are in xrpld and this test is a standalone executable. We'll be able to get rid of them completely in the future after we fully switch to gtest.

[bthomee]

Suggested change

	if (boost::iequals(name, "Content-Length"))
	{
	res->erase(boost::beast::http::field::content_length);
	res->set(name, value);
	}
	res->set(name, value);

Now that the headers are no longer set before res->body(), shouldn't we set them all here? (I'm not familiar with when Boost sets these fields, so perhaps the erase is still needed if it always adds a Content-Length header by itself.

[bthomee]

Note that I'm a bit surprised the tests succeeded without any of the headers being set (unless Boost sets Content-Length automatically).

Then again, this might be because some tests have already been ported to gtest and others haven't, so here you're just lucky it works?

[a1q123456]

Yeah I think we only have one custom header here and boost always automatically add the content-length header.

[bthomee]

Shouldn't we then keep the erase, as otherwise we will get two headers with the "same" name, e.g. Content-Type and content-type?

Something like this might be better:

                    if (boost::iequals(name, "Content-Length"))
                    {
                        res->erase(boost::beast::http::field::content_length);
                    }
                    res->set(name, value);

[a1q123456]

Yeah I think it still works without explicitly erasing and re-setting. The logs show that the client is still able to receive different content-length headers.

4: Hello World!"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: content-length: 12
4: 
4: Hello World!"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: CONTENT-LENGTH: 12
4: 
4: Hello World!"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: Content-length: 12
4: 
4: Hello World!"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: content-Length: 12
4: 
4: Hello World!"
4: DBG: invokeComplete: Deadline popping: 1
4: [       OK ] HTTPClient.case_insensitive_content_length (10 ms)
4: [ RUN      ] HTTPClient.basic_http_request
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: Content-Length: 18
4: Content-Type: text/plain
4: 
4: Test response body"
4: DBG: invokeComplete: Deadline popping: 1
4: [       OK ] HTTPClient.basic_http_request (0 ms)
4: [ RUN      ] HTTPClient.empty_response
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: Content-Length: 0
4: 
4: "
4: DBG: invokeComplete: Deadline popping: 1
4: [       OK ] HTTPClient.empty_response (0 ms)
4: [ RUN      ] HTTPClient.different_status_codes
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 200 OK
4: Server: TestServer
4: Content-Length: 10
4: 
4: Status 200"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 404 Not Found
4: Server: TestServer
4: Content-Length: 10
4: 
4: Status 404"
4: DBG: invokeComplete: Deadline popping: 1
4: TRC: Fetch: 127.0.0.1
4: TRC: Resolving: 127.0.0.1
4: TRC: Resolve complete.
4: TRC: Connected.
4: TRC: Session started.
4: TRC: Wrote.
4: TRC: Header: "HTTP/1.0 500 Internal Server Error
4: Server: TestServer
4: Content-Length: 10
4: 
4: Status 500"
4: DBG: invokeComplete: Deadline popping: 1
4: [       OK ] HTTPClient.different_status_codes (1 ms)
4: [----------] 4 tests from HTTPClient (14 ms total)
4: 
4: [----------] Global test environment tear-down
4: [==========] 4 tests from 1 test suite ran. (14 ms total)
4: [  PASSED  ] 4 tests.

[a1q123456]

template<class Allocator>
void
basic_fields<Allocator>::
set_element(element& e)
{
    auto it = set_.lower_bound(
        e.name_string(), key_compare{});
    if(it == set_.end() || ! beast::iequals(
        e.name_string(), it->name_string()))
    {
        set_.insert_before(it, e);
        list_.push_back(e);
        return;
    }
    for(;;)
    {
        auto next = it;
        ++next;
        set_.erase(it);
        list_.erase(list_.iterator_to(*it));
        delete_element(*it);
        it = next;
        if(it == set_.end() ||
            ! beast::iequals(e.name_string(), it->name_string()))
            break;
    }
    set_.insert_before(it, e);
    list_.push_back(e);
}

The code in boost also erases the existing element

[kuznetsss]

Not really related to this PR, feel free to ignore.
This could be replaced with:

server.ioc().run_for(std::chrono::seconds{10});