Improve tests (#836)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: 3 people

Makefile

@@ -13,9 +13,9 @@ docker-build-binaries:
 
 .PHONY: test
 test:
-	docker build -t nginx-opentracing-test/nginx -f test/Dockerfile-test . --build-arg NGINX_VERSION=$(NGINX_VERSION)
-	docker build -t nginx-opentracing-test/backend -f test/Dockerfile-backend ./test
-	docker build -t nginx-opentracing-test/grpc-backend -f test/environment/grpc/Dockerfile ./test/environment/grpc
+	docker build -t nginx-opentracing-test/nginx -f test/Dockerfile-test . --build-arg NGINX_VERSION=$(NGINX_VERSION) --load
+	docker build -t nginx-opentracing-test/backend -f test/Dockerfile-backend ./test --load
+	docker build -t nginx-opentracing-test/grpc-backend -f test/environment/grpc/Dockerfile ./test/environment/grpc --load
 	cd test && LOG_DIR=$(CURDIR)/test/test-log PYTHONPATH=environment/grpc python3 nginx_opentracing_test.py
 
 .PHONY: clean

test/environment/docker-compose.yaml

@@ -1,6 +1,7 @@
 services:
 
   nginx:
+    container_name: nginx
     image: nginx-opentracing-test/nginx
     networks:
       testnet:
@@ -30,6 +31,7 @@ services:
       - daemon off;
 
   backend:
+    container_name: backend
     image: nginx-opentracing-test/backend
     networks:
       testnet:
@@ -43,6 +45,7 @@ services:
       - "5001:5001"
 
   php_fpm:
+    container_name: php_fpm
     image: php:8-fpm
     networks:
       testnet:
@@ -56,6 +59,7 @@ services:
       - "9000:9000"
 
   grpc_backend:
+    container_name: grpc_backend
     image: nginx-opentracing-test/grpc-backend
     networks:
       testnet:

test/nginx_opentracing_test.py

@@ -47,24 +47,47 @@ def setUp(self):
             stderr=subprocess.PIPE,
         )
         self.client = get_docker_client()
-        timeout = time.time() + 120
-        while len(self.client.containers.list()) != 4:
-            if time.time() > timeout:
-                raise TimeoutError()
-            time.sleep(0.001)
+        self.wait_for_containers()
 
         # Wait so that backend can come up.
         # TODO: replace with something better
         time.sleep(5)
 
         self.conn = http.client.HTTPConnection("localhost", 8080, timeout=5)
-        self.grpcConn = grpc.insecure_channel("localhost:8081")
-        try:
-            grpc.channel_ready_future(self.grpcConn).result(timeout=10)
-        except grpc.FutureTimeoutError:
-            sys.exit("Error connecting to server")
+
+        # Only create the gRPC connection when needed, not during setup
+        self.grpcConn = None
         self.running = True
 
+    def wait_for_containers(self, expected_containers=None):
+        if expected_containers is None:
+            expected_containers = ["nginx", "backend", "php_fpm", "grpc_backend"]
+
+        timeout = time.time() + 120
+        while True:
+            try:
+                containers = {
+                    c.name: c.status for c in self.client.containers.list(all=True)
+                }
+
+                ready = all(
+                    containers.get(expected) == "running"
+                    for expected in expected_containers
+                )
+
+                if ready:
+                    break
+
+                if time.time() > timeout:
+                    raise TimeoutError(
+                        f"Containers not ready in time. Current containers: {containers}"
+                    )
+                time.sleep(0.5)
+            except docker.errors.APIError as e:
+                if time.time() > timeout:
+                    raise TimeoutError(f"Docker API error: {str(e)}") from e
+                time.sleep(0.5)
+
     def _logEnvironment(self):
         logdir = os.environ["LOG_DIR"]
         test_log = os.path.join(logdir, self._testMethodName)
@@ -91,6 +114,16 @@ def _logEnvironment(self):
             os.path.join(test_log, "nginx-error.log"),
         )
 
+    def _setup_grpc(self):
+        """Set up gRPC connection only when needed"""
+        if self.grpcConn is None:
+            self.grpcConn = grpc.insecure_channel("localhost:8081")
+            try:
+                grpc.channel_ready_future(self.grpcConn).result(timeout=10)
+            except grpc.FutureTimeoutError as err:
+                raise RuntimeError("Error connecting to gRPC server") from err
+        return self.grpcConn
+
     def tearDown(self):
         self._stopDocker()
         logdir = None
@@ -101,9 +134,13 @@ def tearDown(self):
         os.chdir(self.testdir)
         self.client.close()
         self.conn.close()
-        self.grpcConn.close()
 
     def _stopDocker(self):
+        # Close any active gRPC connection before Docker operations that might fork
+        if self.grpcConn is not None:
+            self.grpcConn.close()
+            self.grpcConn = None
+
         if not self.running:
             return
         self.running = False
@@ -210,11 +247,16 @@ def testFastcgiPropagation(self):
         self.assertEqual(len(self.nginx_traces), 2)
 
     def testGrpcPropagation(self):
-        app = app_service.AppStub(self.grpcConn)
-        app.CheckTraceHeader(app_messages.Empty())
-
+        # Set up gRPC just for this test
+        grpc_conn = self._setup_grpc()
+        app = app_service.AppStub(grpc_conn)
+        try:
+            app.CheckTraceHeader(app_messages.Empty())
+        finally:
+            if self.grpcConn is not None:
+                self.grpcConn.close()
+                self.grpcConn = None
         self._stopEnvironment()
-
         self.assertEqual(len(self.nginx_traces), 2)