From 39161fab259f65c4bcc4b8a10ec38b6c0dd6b639 Mon Sep 17 00:00:00 2001
From: Thiyagu K <thiyaguk@google.com>
Date: Fri, 6 Feb 2026 11:49:55 +0000
Subject: [PATCH 1/2] fix: resolve flakiness in Service retry system tests

- Implement dynamic port allocation (port 0) for mock servers to avoid
EADDRINUSE collisions in CI.
- Increase timeout to 90s for retry logic to accommodate environmental
latency.
- Ensure mock servers are explicitly closed in both success and failure
blocks.
- Fix async race condition in 'non-responsive hosts' test by moving
done() into the callback.
---
 system-test/common.ts | 72 +++++++++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 26 deletions(-)

diff --git a/system-test/common.ts b/system-test/common.ts
index 17bc59b61..0e2d61f6f 100644
--- a/system-test/common.ts
+++ b/system-test/common.ts
@@ -21,6 +21,8 @@ import * as http from 'http';
 import * as common from '../src/nodejs-common/index.js';
 
 describe('Common', () => {
+  // MOCK_HOST_PORT is kept for Service initialization but individual tests
+  // now use dynamic ports to avoid EADDRINUSE collisions in CI.
   const MOCK_HOST_PORT = 8118;
   const MOCK_HOST = `http://localhost:${MOCK_HOST_PORT}`;
 
@@ -42,18 +44,27 @@ describe('Common', () => {
         res.end(mockResponse);
       });
 
-      mockServer.listen(MOCK_HOST_PORT);
-
-      service.request(
-        {
-          uri: '/mock-endpoint',
-        },
-        (err, resp) => {
-          assert.ifError(err);
-          assert.strictEqual(resp, mockResponse);
-          mockServer.close(done);
-        }
-      );
+      // Listen on port 0 to allow the OS to assign a random available port.
+      // This prevents "port already in use" errors if tests run in parallel.
+      mockServer.listen(0, () => {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const port = (mockServer.address() as any).port;
+
+        service.request(
+          {
+            uri: `http://localhost:${port}/mock-endpoint`,
+          },
+          (err, resp) => {
+            try {
+              assert.ifError(err);
+              assert.strictEqual(resp, mockResponse);
+              mockServer.close(done);
+            } catch (e) {
+              mockServer.close(() => done(e));
+            }
+          }
+        );
+      });
     });
 
     it('should retry a request', function (done) {
@@ -70,18 +81,25 @@ describe('Common', () => {
         res.end();
       });
 
-      mockServer.listen(MOCK_HOST_PORT);
-
-      service.request(
-        {
-          uri: '/mock-endpoint-retry',
-        },
-        err => {
-          assert.strictEqual((err! as common.ApiError).code, 408);
-          assert.strictEqual(numRequestAttempts, 4);
-          mockServer.close(done);
-        }
-      );
+      mockServer.listen(0, () => {
+        // eslint-disable-next-line @typescript-eslint/no-explicit-any
+        const port = (mockServer.address() as any).port;
+
+        service.request(
+          {
+            uri: `http://localhost:${port}/mock-endpoint-retry`,
+          },
+          err => {
+            try {
+              assert.strictEqual((err! as common.ApiError).code, 408);
+              assert.strictEqual(numRequestAttempts, 4);
+              mockServer.close(done); // Ensure done is called only after server is closed
+            } catch (e) {
+              mockServer.close(() => done(e)); // Cleanup even if assertion fails
+            }
+          }
+        );
+      });
     });
 
     it('should retry non-responsive hosts', function (done) {
@@ -102,15 +120,17 @@ describe('Common', () => {
 
       service.request(
         {
-          uri: '/mock-endpoint-no-response',
+          // Using port :1 (reserved) ensures an immediate ECONNREFUSED
+          // without risking hitting a real service on the runner.
+          uri: 'http://localhost:1/mock-endpoint-no-response',
         },
         err => {
           assert(err?.message.includes('ECONNREFUSED'));
           const timeResponse = Date.now();
           assert(timeResponse - timeRequest > minExpectedResponseTime);
+          done();
         }
       );
-      done();
     });
   });
 });

From b13bf3a34717e81d0444594d4cf9f966b2bc7d7e Mon Sep 17 00:00:00 2001
From: Thiyagu K <thiyaguk@google.com>
Date: Fri, 6 Feb 2026 12:13:58 +0000
Subject: [PATCH 2/2] add AddressInfo

---
 system-test/common.ts | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/system-test/common.ts b/system-test/common.ts
index 0e2d61f6f..fab71bb72 100644
--- a/system-test/common.ts
+++ b/system-test/common.ts
@@ -47,8 +47,7 @@ describe('Common', () => {
       // Listen on port 0 to allow the OS to assign a random available port.
       // This prevents "port already in use" errors if tests run in parallel.
       mockServer.listen(0, () => {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        const port = (mockServer.address() as any).port;
+        const port = (mockServer.address() as import('net').AddressInfo).port;
 
         service.request(
           {
@@ -82,8 +81,7 @@ describe('Common', () => {
       });
 
       mockServer.listen(0, () => {
-        // eslint-disable-next-line @typescript-eslint/no-explicit-any
-        const port = (mockServer.address() as any).port;
+        const port = (mockServer.address() as import('net').AddressInfo).port;
 
         service.request(
           {