improve tests and debuggability

cesanta · Dec 30, 2024 · 72f9322 · 72f9322
1 parent f95283e
commit 72f9322
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 30 deletions.
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml
@@ -3,7 +3,7 @@ name: "CodeQL Scanning"
 
 on:
   schedule:
-    - cron: '30 2 * * *' # run at 2:30 AM UTC
+    - cron: '30 23 * * *' # run at 11:30 PM UTC
   # Allow manual runs
   workflow_dispatch:
 

diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml
@@ -1,7 +1,7 @@
 name: Full build and test
 on:
   schedule:
-    - cron: '0 1 * * *' # run at 1 AM UTC
+    - cron: '0 22 * * *' # run at 10 PM UTC
   # Allow manual runs
   workflow_dispatch:
 env:
@@ -117,7 +117,7 @@ jobs:
     name: macos SSL=${{ matrix.ssl }} TFLAGS=${{ matrix.select }}
     env:
       SSL: ${{ matrix.ssl }}
-      TFLAGS: ${{ matrix.select }} -DNO_SNTP_CHECK -Wno-sign-conversion # Workaround for MbedTLS 3.5.0
+      TFLAGS: ${{ matrix.select }} -Wno-sign-conversion # Workaround for MbedTLS 3.5.0
       HOMEBREW_NO_AUTO_UPDATE: 1
     steps:
     - uses: actions/checkout@v4

diff --git a/.github/workflows/quicktest.yml b/.github/workflows/quicktest.yml
@@ -59,7 +59,7 @@ jobs:
     name: macos SSL=${{ matrix.ssl }}
     env:
       SSL: ${{ matrix.ssl }}
-      TFLAGS: -DNO_SNTP_CHECK
+      #TFLAGS: -DNO_SNTP_CHECK
       HOMEBREW_NO_AUTO_UPDATE: 1
     steps:
     - uses: actions/checkout@v4

diff --git a/mongoose.c b/mongoose.c
@@ -3523,7 +3523,7 @@ void mg_mqtt_login(struct mg_connection *c, const struct mg_mqtt_opts *opts) {
     total_len += 2 + (uint32_t) opts->pass.len;
     hdr[7] |= MQTT_HAS_PASSWORD;
   }
-  if (opts->topic.len > 0) { // allow zero-length msgs, message.len is size_t
+  if (opts->topic.len > 0) {  // allow zero-length msgs, message.len is size_t
     total_len += 4 + (uint32_t) opts->topic.len + (uint32_t) opts->message.len;
     hdr[7] |= MQTT_HAS_WILL;
   }
@@ -3569,18 +3569,19 @@ uint16_t mg_mqtt_pub(struct mg_connection *c, const struct mg_mqtt_opts *opts) {
   uint16_t id = opts->retransmit_id;
   uint8_t flags = (uint8_t) (((opts->qos & 3) << 1) | (opts->retain ? 1 : 0));
   size_t len = 2 + opts->topic.len + opts->message.len;
-  MG_DEBUG(("%lu [%.*s] -> [%.*s]", c->id, (int) opts->topic.len,
-            (char *) opts->topic.buf, (int) opts->message.len,
-            (char *) opts->message.buf));
+  MG_DEBUG(("%lu [%.*s] <- [%.*s%c", c->id, (int) opts->topic.len,
+            (char *) opts->topic.buf,
+            (int) (opts->message.len <= 10 ? opts->message.len : 10),
+            (char *) opts->message.buf, opts->message.len <= 10 ? ']' : ' '));
   if (opts->qos > 0) len += 2;
   if (c->is_mqtt5) len += get_props_size(opts->props, opts->num_props);
 
   if (opts->qos > 0 && id != 0) flags |= 1 << 3;
   mg_mqtt_send_header(c, MQTT_CMD_PUBLISH, flags, (uint32_t) len);
   mg_send_u16(c, mg_htons((uint16_t) opts->topic.len));
   mg_send(c, opts->topic.buf, opts->topic.len);
-  if (opts->qos > 0) {    // need to send 'id' field
-    if (id == 0) {  // generate new one if not resending
+  if (opts->qos > 0) {  // need to send 'id' field
+    if (id == 0) {      // generate new one if not resending
       if (++c->mgr->mqtt_id == 0) ++c->mgr->mqtt_id;
       id = c->mgr->mqtt_id;
     }
@@ -3703,8 +3704,10 @@ static void mqtt_cb(struct mg_connection *c, int ev, void *ev_data) {
             }
             break;
           case MQTT_CMD_PUBLISH: {
-            /*MG_DEBUG(("%lu [%.*s] -> [%.*s]", c->id, (int) mm.topic.len,
-                      mm.topic.buf, (int) mm.data.len, mm.data.buf));*/
+            MG_DEBUG(("%lu [%.*s] -> [%.*s%c", c->id, (int) mm.topic.len,
+                      mm.topic.buf,
+                      (int) (mm.data.len <= 10 ? mm.data.len : 10), mm.data.buf,
+                      mm.data.len <= 10 ? ']' : ' '));
             if (mm.qos > 0) {
               uint16_t id = mg_ntohs(mm.id);
               uint32_t remaining_len = sizeof(id);

diff --git a/src/mqtt.c b/src/mqtt.c
@@ -276,7 +276,7 @@ void mg_mqtt_login(struct mg_connection *c, const struct mg_mqtt_opts *opts) {
     total_len += 2 + (uint32_t) opts->pass.len;
     hdr[7] |= MQTT_HAS_PASSWORD;
   }
-  if (opts->topic.len > 0) { // allow zero-length msgs, message.len is size_t
+  if (opts->topic.len > 0) {  // allow zero-length msgs, message.len is size_t
     total_len += 4 + (uint32_t) opts->topic.len + (uint32_t) opts->message.len;
     hdr[7] |= MQTT_HAS_WILL;
   }
@@ -322,18 +322,19 @@ uint16_t mg_mqtt_pub(struct mg_connection *c, const struct mg_mqtt_opts *opts) {
   uint16_t id = opts->retransmit_id;
   uint8_t flags = (uint8_t) (((opts->qos & 3) << 1) | (opts->retain ? 1 : 0));
   size_t len = 2 + opts->topic.len + opts->message.len;
-  MG_DEBUG(("%lu [%.*s] -> [%.*s]", c->id, (int) opts->topic.len,
-            (char *) opts->topic.buf, (int) opts->message.len,
-            (char *) opts->message.buf));
+  MG_DEBUG(("%lu [%.*s] <- [%.*s%c", c->id, (int) opts->topic.len,
+            (char *) opts->topic.buf,
+            (int) (opts->message.len <= 10 ? opts->message.len : 10),
+            (char *) opts->message.buf, opts->message.len <= 10 ? ']' : ' '));
   if (opts->qos > 0) len += 2;
   if (c->is_mqtt5) len += get_props_size(opts->props, opts->num_props);
 
   if (opts->qos > 0 && id != 0) flags |= 1 << 3;
   mg_mqtt_send_header(c, MQTT_CMD_PUBLISH, flags, (uint32_t) len);
   mg_send_u16(c, mg_htons((uint16_t) opts->topic.len));
   mg_send(c, opts->topic.buf, opts->topic.len);
-  if (opts->qos > 0) {    // need to send 'id' field
-    if (id == 0) {  // generate new one if not resending
+  if (opts->qos > 0) {  // need to send 'id' field
+    if (id == 0) {      // generate new one if not resending
       if (++c->mgr->mqtt_id == 0) ++c->mgr->mqtt_id;
       id = c->mgr->mqtt_id;
     }
@@ -456,8 +457,10 @@ static void mqtt_cb(struct mg_connection *c, int ev, void *ev_data) {
             }
             break;
           case MQTT_CMD_PUBLISH: {
-            /*MG_DEBUG(("%lu [%.*s] -> [%.*s]", c->id, (int) mm.topic.len,
-                      mm.topic.buf, (int) mm.data.len, mm.data.buf));*/
+            MG_DEBUG(("%lu [%.*s] -> [%.*s%c", c->id, (int) mm.topic.len,
+                      mm.topic.buf,
+                      (int) (mm.data.len <= 10 ? mm.data.len : 10), mm.data.buf,
+                      mm.data.len <= 10 ? ']' : ' '));
             if (mm.qos > 0) {
               uint16_t id = mg_ntohs(mm.id);
               uint32_t remaining_len = sizeof(id);

diff --git a/test/mip_test.c b/test/mip_test.c
@@ -8,12 +8,16 @@ static int s_num_tests = 0;
 static bool s_sent_fragment = 0;
 static int s_seg_sent = 0;
 
+#define ABORT()                                                 \
+      usleep(500000);  /* 500 ms, GH print reason */            \
+      abort();
+
 #define ASSERT(expr)                                            \
   do {                                                          \
     s_num_tests++;                                              \
     if (!(expr)) {                                              \
       printf("FAILURE %s:%d: %s\n", __FILE__, __LINE__, #expr); \
-      abort();                                                  \
+      ABORT();                                                  \
     }                                                           \
   } while (0)
 

diff --git a/test/unit_test.c b/test/unit_test.c
@@ -5,12 +5,16 @@
 
 static int s_num_tests = 0;
 
+#define ABORT()                                                 \
+      usleep(500000);  /* 500 ms, GH print reason */            \
+      abort();
+
 #define ASSERT(expr)                                            \
   do {                                                          \
     s_num_tests++;                                              \
     if (!(expr)) {                                              \
       printf("FAILURE %s:%d: %s\n", __FILE__, __LINE__, #expr); \
-      abort();                                                  \
+      ABORT();                                                  \
     }                                                           \
   } while (0)
 
@@ -329,7 +333,7 @@ static void sntp_cb(struct mg_connection *c, int ev, void *ev_data) {
   (void) c;
 }
 
-static void test_sntp_server(const char *url) {
+static bool test_sntp_server(const char *url) {
   int64_t ms = 0;
   struct mg_mgr mgr;
   struct mg_connection *c = NULL;
@@ -341,25 +345,29 @@ static void test_sntp_server(const char *url) {
   ASSERT(c->is_udp == 1);
   for (i = 0; i < 60 && ms == 0; i++) mg_mgr_poll(&mgr, 50);
   MG_DEBUG(("server: %s, ms: %lld", url ? url : "(default)", ms));
-#if !defined(NO_SNTP_CHECK)
-  ASSERT(ms > 0);
-#endif
   mg_mgr_free(&mgr);
+  return ms > 0;
 }
 
 static void test_sntp(void) {
+  bool result;
   const unsigned char bad[] =
       "\x55\x02\x00\xeb\x00\x00\x00\x1e\x00\x00\x07\xb6\x3e\xc9\xd6\xa2"
       "\xdb\xde\xea\x30\x91\x86\xb7\x10\xdb\xde\xed\x98\x00\x00\x00\xde"
       "\xdb\xde\xed\x99\x0a\xe2\xc7\x96\xdb\xde\xed\x99\x0a\xe4\x6b\xda";
 
   ASSERT(mg_sntp_parse(bad, sizeof(bad)) < 0);
   ASSERT(mg_sntp_parse(NULL, 0) == -1);
-  // NOTE(cpq): temporarily disabled until Github Actions fix their NTP
-  // port blockage issue, https://github.com/actions/runner-images/issues/5615
-  // test_sntp_server("udp://time.apple.com:123");
-  test_sntp_server("udp://time.windows.com:123");
+  // NOTE(): historical NTP port blockage issue; expect at least one to be
+  // reachable and work. https://github.com/actions/runner-images/issues/5615
+  result = test_sntp_server("udp://time.apple.com:123") ||
+  test_sntp_server("udp://time.windows.com:123") ||
   test_sntp_server(NULL);
+#if defined(NO_SNTP_CHECK)
+  (void) result;
+#else
+  ASSERT(result);
+#endif
 }
 
 struct mqtt_data {
@@ -567,6 +575,7 @@ static void test_mqtt_ver(uint8_t mqtt_version) {
   const char *url = "mqtt://broker.hivemq.com:1883";
   int i, retries;
 
+  MG_DEBUG(("ver: %u", mqtt_version));
   // Connect with options: version, clean session, last will, keepalive
   // time. Don't set retain, some runners are not random
   test_data.flags = 0;