test(esp_http_client): final review fixes for characterization suite

Runner now runs unfiltered; #error guard couples BASIC_AUTH to custom
transport configs; CMake include and REQUIRES cleanup; doc corrections and
tightened asserts from the final review.

components/esp_http_client/test_apps/main/CMakeLists.txt

@@ -3,7 +3,6 @@ set(mock_transport_dir ${CMAKE_CURRENT_SOURCE_DIR}/../mock_transport)
 idf_component_register(SRC_DIRS "." ${mock_transport_dir}
                     PRIV_INCLUDE_DIRS "."
                                       "../../lib/include"
+                                      ${mock_transport_dir}
                     PRIV_REQUIRES esp_http_client tcp_transport test_utils unity
-                    INCLUDE_DIRS ${mock_transport_dir}
-                    REQUIRES tcp_transport
                     WHOLE_ARCHIVE)

components/esp_http_client/test_apps/main/test_http_client.c

@@ -465,7 +465,7 @@ static const char *mock_http_response_ok =
     "\r\n"
     "{\"status\":\"ok\"}";
 
-esp_err_t _http_event_handler(esp_http_client_event_t *evt)
+static esp_err_t _http_event_handler(esp_http_client_event_t *evt)
 {
     switch (evt->event_id) {
         case HTTP_EVENT_ON_CONNECTED:

components/esp_http_client/test_apps/main/test_http_client_auth.c

@@ -52,6 +52,10 @@
  */
 #if CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT
 
+#if !CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH
+#error "These cases require CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y; keep every sdkconfig.ci.* with CUSTOM_TRANSPORT in sync"
+#endif
+
 static const char *resp_401 =
     "HTTP/1.1 401 Unauthorized\r\n"
     "WWW-Authenticate: Basic realm=\"Test\"\r\n"

components/esp_http_client/test_apps/main/test_http_client_basic.c

@@ -496,6 +496,25 @@ TEST_CASE("Client handles 401 Unauthorized error", "[esp_http_client][basic][p0]
     TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_stats(mock_transport, &stats));
     TEST_ASSERT_EQUAL(2, stats.write_calls);
 
+    /* characterization: master behavior, see refactor spec
+     * The queued second 401 triggers a queue-advance on the retry's write
+     * (previous response fully read, a queued response is still pending -
+     * see test_http_client_mock_transport.h), which resets the capture
+     * buffer just before the retry is written. So this last-request capture
+     * holds only the credential-less retry, not the original request.
+     * Confirms no Authorization header is attached: no auth_type,
+     * username/password, or URL-embedded credentials were ever configured
+     * on this client, so esp_http_client_prepare()'s credential check
+     * (~L800-803) has nothing to attach even though add_auth() unconditionally
+     * schedules the retry. */
+    char req[2048];
+    TEST_ASSERT_EQUAL(ESP_OK, mock_http_transport_get_last_request(mock_transport, req, sizeof(req), NULL));
+    /* Non-vacuity check: confirms the capture actually holds the retried
+     * request line (not empty, not the original request left over from a
+     * missed reset) before trusting the Authorization-absence assert below. */
+    TEST_ASSERT_NOT_NULL(strstr(req, "GET /api/protected"));
+    TEST_ASSERT_NULL(strstr(req, "Authorization"));
+
     ESP_LOGI(TAG, "OK: 401 error handled, credential-less retry capped and reported");
 
     esp_http_client_cleanup(client);

components/esp_http_client/test_apps/main/test_http_client_lifecycle.c

@@ -81,6 +81,11 @@ TEST_CASE("Connection: close forces reconnect on next perform", "[esp_http_clien
     mc.response_data = resp_close;
     esp_transport_handle_t mock = mock_http_transport_create(&mc);
     TEST_ASSERT_NOT_NULL(mock);
+    /* characterization: this queued response is never popped - the forced
+     * reconnect calls mock_close() then mock_connect(), both of which reset
+     * read_offset to 0, so the second perform() re-serves the identical
+     * initial buffer instead of advancing the queue; kept here for
+     * intent-documentation. */
     mock_http_transport_queue_response(mock, resp_close, 0);
 
     esp_http_client_config_t cfg = {

components/esp_http_client/test_apps/main/test_http_client_streaming.c

@@ -55,12 +55,40 @@ TEST_CASE("open/write/fetch_headers/read sequence works", "[esp_http_client][str
 
     const char *body = "abcde";
     TEST_ASSERT_EQUAL(ESP_OK, esp_http_client_open(client, 5));
+    // characterization: master behavior, see refactor spec
+    // esp_http_client_open() (esp_http_client.c ~L1921) sets
+    // state = HTTP_STATE_REQ_COMPLETE_HEADER right after writing the
+    // request line and headers over the transport.
+    TEST_ASSERT_EQUAL(HTTP_STATE_REQ_COMPLETE_HEADER, esp_http_client_get_state(client));
     TEST_ASSERT_EQUAL(5, esp_http_client_write(client, body, 5));
+    // characterization: master behavior, see refactor spec
+    // The public esp_http_client_write() (~L1978) never touches
+    // client->state - it only requires state >= REQ_COMPLETE_HEADER and
+    // writes bytes directly over the transport, so the state observed
+    // here is unchanged from the open() call above.
+    TEST_ASSERT_EQUAL(HTTP_STATE_REQ_COMPLETE_HEADER, esp_http_client_get_state(client));
     TEST_ASSERT_EQUAL(5, esp_http_client_fetch_headers(client));
+    // characterization: master behavior, see refactor spec
+    // esp_http_client_fetch_headers() (~L1667-1697) unconditionally sets
+    // state = HTTP_STATE_REQ_COMPLETE_DATA on entry, then reads/parses
+    // until the response header-parse loop exits, then unconditionally
+    // sets state = HTTP_STATE_RES_ON_DATA_START before returning - it
+    // never stops at RES_COMPLETE_HEADER and does not depend on whether
+    // any body bytes were actually read yet.
+    TEST_ASSERT_EQUAL(HTTP_STATE_RES_ON_DATA_START, esp_http_client_get_state(client));
     TEST_ASSERT_EQUAL(200, esp_http_client_get_status_code(client));
 
     char buf[16] = {0};
     int rd = esp_http_client_read(client, buf, sizeof(buf));
+    // characterization: master behavior, see refactor spec
+    // esp_http_client_read() (~L1435) never assigns client->state at all.
+    // Also, for this canned response the single mock transport read done
+    // inside fetch_headers() above already delivered the whole 43-byte
+    // buffer (headers + 5-byte body) to the parser in one
+    // http_parser_execute() call, so this read() serves the body from the
+    // already-cached response buffer without issuing a second transport
+    // read - the state observed here is unchanged from fetch_headers().
+    TEST_ASSERT_EQUAL(HTTP_STATE_RES_ON_DATA_START, esp_http_client_get_state(client));
     TEST_ASSERT_EQUAL(5, rd);
     TEST_ASSERT_EQUAL_STRING("hello", buf);
     TEST_ASSERT_TRUE(esp_http_client_is_complete_data_received(client));

components/esp_http_client/test_apps/mock_transport/test_http_client_mock_transport.h

@@ -202,8 +202,12 @@ esp_err_t mock_http_transport_queue_response(esp_transport_handle_t t,
  * @brief Retrieve the bytes captured from the most recent request
  *
  * mock_write() appends every written byte (capped at 2048 bytes) into an
- * internal capture buffer, reset at each request boundary. This lets tests
- * assert on the serialized request content (headers, body).
+ * internal capture buffer. The buffer is reset only on a queue-advance: a
+ * write arriving after the current response has been fully read AND a
+ * queued response is still pending (see mock_http_transport_queue_response()).
+ * With the queue empty or exhausted, no reset happens and the capture
+ * concatenates bytes across requests. This lets tests assert on the
+ * serialized request content (headers, body).
  *
  * @param[in] t Mock transport handle
  * @param[out] buf Buffer to receive the captured request bytes, NUL-terminated

components/esp_http_client/test_apps/pytest_stage0_qemu.py

@@ -8,7 +8,4 @@ from pytest_embedded_idf.utils import idf_parametrize
 @pytest.mark.qemu
 @idf_parametrize('target', ['esp32c3'], indirect=['target'])
 def test_http_client_mock(dut: Dut) -> None:
-    dut.run_all_single_board_cases(
-        group=['basic', 'async', 'lifecycle', 'chunked', 'redirect', 'auth', 'streaming', 'error_recovery'],
-        timeout=600,
-    )
+    dut.run_all_single_board_cases(timeout=600)

components/esp_http_client/test_apps/sdkconfig.ci.default

@@ -17,7 +17,3 @@ CONFIG_ESP_HTTP_CLIENT_ENABLE_CUSTOM_TRANSPORT=y
 # (unencrypted credentials without TLS); this is a test-only config, not a
 # client-code change - the mock transport never touches the network.
 CONFIG_ESP_HTTP_CLIENT_ENABLE_BASIC_AUTH=y
-
-# Some of the test apps allocate 2048 size buffers on the stack.
-# Increase the default stack size to accommodate for that
-CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192

components/esp_http_client/test_apps/sdkconfig.defaults

@@ -0,0 +1,3 @@
+# Some of the test apps allocate 2048 size buffers on the stack.
+# Increase the default stack size to accommodate for that
+CONFIG_ESP_MAIN_TASK_STACK_SIZE=8192