refactor: Minor improvements to HTTP unit tests #36160

1. 
   hodlinator commented at 10:41 AM on September 3, 2026: contributor

   Improves recently added unit tests.

   • Simplify HTTP response check in http_server_socket_tests, it was incorrectly referring to unordered_map
   • Make http_request_state_tests unit test method use Uppercase as per developer-notes.md, shorten enum values for readability
2. DrahtBot added the label Refactoring on Sep 3, 2026
3. 
   DrahtBot commented at 10:41 AM on September 3, 2026: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   <!--006a51241073e994b41acfe9ec718e94-->

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/36160.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline and AI policy for information on the review process.

   Type	Reviewers
   ACK	winterrdog
   Concept ACK	janb84, jeanpablojp

   If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #36124 (http: Make HTTPRequest update state internally by hodlinator)
   • #36033 ([wip,nomerge,rfc] build: Require C++23 compiler by maflcko)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. fanquake requested review from pinheadmz on Sep 3, 2026
5. in src/test/httpserver_tests.cpp:888 in e69331c329

    891 | -    BOOST_CHECK(actual.find("Date: Wed, 11 Dec 2024 00:47:09 GMT\r\n") != std::string::npos);
    892 | +    BOOST_CHECK_EQUAL(actual, "HTTP/1.1 200 OK\r\n"
    893 | +                              "Date: Wed, 11 Dec 2024 00:47:09 GMT\r\n"
    894 | +                              "Content-Length: 7\r\n"
    895 | +                              "Content-Type: text/html; charset=ISO-8859-1\r\n"
    896 | +                              "Connection: close\r\n"
   

   ---

   ---

   

   janb84 commented at 8:12 PM on September 3, 2026:

   NIT: Although the suggested code in the PR is already a great improvement, I would suggest to hoist the expected string to the top. This remove some magic numbers (the reserve of 146 bytes with 10ms sleep is redundant) and the actual.length can be compared with the expected.length. And removes some risk of code drift (between the loop and expectation)

       const std::string expected{"HTTP/1.1 200 OK\r\n"
                                  "Date: Wed, 11 Dec 2024 00:47:09 GMT\r\n"
                                  "Content-Length: 7\r\n"
                                  "Content-Type: text/html; charset=ISO-8859-1\r\n"
                                  "Connection: close\r\n"
                                  "\r\n874140\n"};
       std::string actual;
       // Wait up to one minute for all the bytes to appear in the "send" pipe.
       char buf[0x10000] = {};
       attempts = 6000;
       while (attempts > 0)
       {
           ssize_t bytes_read = mock_client_socket_pipes->send.GetBytes(buf, sizeof(buf), 0);
           if (bytes_read > 0) {
               actual.append(buf, bytes_read);
               if (actual.length() >= expected.length()) {
                   break;
               }
           }
           std::this_thread::sleep_for(10ms);
           --attempts;
       }
       BOOST_CHECK_EQUAL(actual, expected);
   

   ---

   

   hodlinator commented at 10:46 AM on September 4, 2026:

   Thanks for the feedback! Addressed in latest push.

6. 
   janb84 commented at 8:15 PM on September 3, 2026: contributor

   Concept ACK e69331c329877f824ac06a92b00abff116125bad

   Great improvement in readability! using enum matches the existing usage in the other files.

   Found some typo's in the commit messages: In commit message of 8b1c375a85a36118388e88c56bdc2b8543c7fc4f where -> were In commit message of 'e69331c329877f824ac06a92b00abff116125bad' incresae -> increases

7. refactor(test): Simplify HTTP response check

   During the development of the new HTTP server, the headers were initially stored in an unordered_map, but later it was changed to a vector.

   6eea8e313c
8. refactor(test): Make test method use Uppercase, shorten enum values

   The former conforms to developer-notes.md, the latter increases readability.

   f04b0c3162
9. hodlinator force-pushed on Sep 4, 2026
10. 
    winterrdog commented at 7:03 PM on September 4, 2026: contributor

    tACK f04b0c31627adda1d54b7da4507cfe30a63f7e5e

    successfully built and tested on this toolchain: Debian/clang++ 18/x86_64.

11. DrahtBot requested review from janb84 on Sep 4, 2026
12. 
    jeanpablojp commented at 9:58 PM on September 5, 2026: contributor

    Concept ACK

    Confirmed the old checks still pass after swapping two headers in WriteReply, so they were blind rather than tolerant.

13. in src/test/httpserver_tests.cpp:487 in f04b0c3162

     483 | @@ -484,50 +484,51 @@ BOOST_AUTO_TEST_CASE(http_request_state_tests)
     484 |      public:
     485 |          DummyClient() : HTTPRemoteClient{/*id=*/0, /*addr=*/CService(), /*socket=*/CreateSock(0, 0, 0)} {}
     486 |  
     487 | -        void receive(std::string_view s)
     488 | +        void Receive(std::string_view s)
    

    ---

    ---

    

    jeanpablojp commented at 9:58 PM on September 5, 2026:

    nit: Could we call this helper AppendToRecvBuffer? The name Receive hides HTTPRemoteClient's no-argument method, while this helper only appends data to the buffer.

Contributors

hodlinatorDrahtBotjanb84winterrdogjeanpablojp

Review Requested
janb84pinheadmz