[buildstream] 08/15: testutils/artifactshare: don't hang on error

[tv...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tvb pushed a commit to branch aevri/enable_spawn_ci_7
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 6661fd56bd5a5758c9bf1c1ddf5dbc24f3fbfcac
Author: Angelos Evripiotis <je...@bloomberg.net>
AuthorDate: Mon Oct 28 15:39:54 2019 +0000

    testutils/artifactshare: don't hang on error
    
    Remove a couple of cases where it's possible to make the main test
    process hang, waiting for something to appear on a queue.
    
    Make it clearer what is available to the server process by
    splitting it out into a non-member function.
    
    Raise a friendlier exception, earlier, if there was a problem starting
    the server process.
---
 tests/testutils/artifactshare.py | 84 +++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 39 deletions(-)

diff --git a/tests/testutils/artifactshare.py b/tests/testutils/artifactshare.py
index 87b0808..7a4eda1 100644
--- a/tests/testutils/artifactshare.py
+++ b/tests/testutils/artifactshare.py
@@ -4,7 +4,7 @@ import signal
 import sys
 from collections import namedtuple
 
-from contextlib import contextmanager
+from contextlib import ExitStack, contextmanager
 from multiprocessing import Process, Queue
 
 from buildstream._cas import CASCache
@@ -50,50 +50,20 @@ class ArtifactShare():
 
         q = Queue()
 
-        self.process = Process(target=self.run, args=(q,))
+        self.process = Process(
+            target=_start_artifact_server,
+            args=(q, self.repodir, self.quota, self.index_only),
+        )
+
         self.process.start()
 
         # Retrieve port from server subprocess
         port = q.get()
 
-        self.repo = 'http://localhost:{}'.format(port)
-
-    # run():
-    #
-    # Run the artifact server.
-    #
-    def run(self, q):
-
-        # Handle SIGTERM by calling sys.exit(0), which will raise a SystemExit exception,
-        # properly executing cleanup code in `finally` clauses and context managers.
-        # This is required to terminate buildbox-casd on SIGTERM.
-        signal.signal(signal.SIGTERM, lambda signalnum, frame: sys.exit(0))
-
-        try:
-            import pytest_cov
-        except ImportError:
-            pass
-        else:
-            pytest_cov.embed.cleanup_on_sigterm()
-
-        try:
-            with create_server(self.repodir,
-                               quota=self.quota,
-                               enable_push=True,
-                               index_only=self.index_only) as server:
-                port = server.add_insecure_port('localhost:0')
-
-                server.start()
-
-                # Send port to parent
-                q.put(port)
+        if port is None:
+            raise Exception("Error occurred when starting artifact server.")
 
-                # Sleep until termination by signal
-                signal.pause()
-
-        except Exception:
-            q.put(None)
-            raise
+        self.repo = 'http://localhost:{}'.format(port)
 
     # has_object():
     #
@@ -183,6 +153,42 @@ class ArtifactShare():
         shutil.rmtree(self.directory)
 
 
+def _start_artifact_server(queue, repodir, quota, index_only):
+    with ExitStack() as stack:
+        try:
+            # Handle SIGTERM by calling sys.exit(0), which will raise a SystemExit exception,
+            # properly executing cleanup code in `finally` clauses and context managers.
+            # This is required to terminate buildbox-casd on SIGTERM.
+            signal.signal(signal.SIGTERM, lambda signalnum, frame: sys.exit(0))
+
+            try:
+                import pytest_cov
+            except ImportError:
+                pass
+            else:
+                pytest_cov.embed.cleanup_on_sigterm()
+
+            server = stack.enter_context(
+                create_server(
+                    repodir,
+                    quota=quota,
+                    enable_push=True,
+                    index_only=index_only,
+                )
+            )
+            port = server.add_insecure_port('localhost:0')
+            server.start()
+        except Exception:
+            queue.put(None)
+            raise
+
+        # Send port to parent
+        queue.put(port)
+
+        # Sleep until termination by signal
+        signal.pause()
+
+
 # create_artifact_share()
 #
 # Create an ArtifactShare for use in a test case