You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by GitBox <gi...@apache.org> on 2022/01/13 17:22:39 UTC

[GitHub] [buildstream] juergbi opened a new pull request #1560: Fix shutdown of test server

juergbi opened a new pull request #1560:
URL: https://github.com/apache/buildstream/pull/1560


   buildbox-casd processes used by test servers were not terminated and kept running after the end of the tests (or crashed because the test deleted the directories used by buildbox-casd).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [buildstream] juergbi commented on a change in pull request #1560: Fix shutdown of test server

Posted by GitBox <gi...@apache.org>.
juergbi commented on a change in pull request #1560:
URL: https://github.com/apache/buildstream/pull/1560#discussion_r784730219



##########
File path: tests/testutils/artifactshare.py
##########
@@ -67,7 +57,17 @@ def run(self, q):
             q.put(port)
 
             # Sleep until termination by signal
-            signal.pause()
+            signal.sigwait([signal.SIGTERM, signal.SIGINT])
+
+            server.stop(0)
+
+        # Save collected coverage data
+        try:
+            from pytest_cov.embed import cleanup
+        except ImportError:
+            pass
+        else:
+            cleanup()

Review comment:
       It would be a bit more concise, however, pylint raises an 'import-error', which can be suppressed but reduces the readability advantage, in my opinion. Also, ignoring potential import errors inside the `cleanup()` function is not ideal, even though this is unlikely to be an issue. I'll leave it as is for now.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [buildstream] juergbi merged pull request #1560: Fix shutdown of test server

Posted by GitBox <gi...@apache.org>.
juergbi merged pull request #1560:
URL: https://github.com/apache/buildstream/pull/1560


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [buildstream] BenjaminSchubert commented on a change in pull request #1560: Fix shutdown of test server

Posted by GitBox <gi...@apache.org>.
BenjaminSchubert commented on a change in pull request #1560:
URL: https://github.com/apache/buildstream/pull/1560#discussion_r784231341



##########
File path: tests/testutils/artifactshare.py
##########
@@ -67,7 +57,17 @@ def run(self, q):
             q.put(port)
 
             # Sleep until termination by signal
-            signal.pause()
+            signal.sigwait([signal.SIGTERM, signal.SIGINT])
+
+            server.stop(0)
+
+        # Save collected coverage data
+        try:
+            from pytest_cov.embed import cleanup
+        except ImportError:
+            pass
+        else:
+            cleanup()

Review comment:
       ```suggestion
           with suppress(ImportError):
               from pytest_cov.embed import cleanup
              cleanup()
   ```
   would that be more readable?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org