zas's suggestions

picard/tagger.py

@@ -318,7 +318,7 @@ class Tagger(QtWidgets.QApplication):
             self.updatecheckmanager = UpdateCheckManager(parent=self.window)
 
     def pipe_server(self):
-        IGNORED = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE, ""}
+        IGNORED = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE}
         while self.pipe_running:
             messages = [x for x in self.pipe_handler.read_from_pipe() if x not in IGNORED]
             if messages:

picard/util/pipe.py

@@ -30,7 +30,6 @@ from typing import (
     Iterable,
     List,
     Optional,
-    Set,
     Tuple,
 )
 
@@ -212,13 +211,13 @@ class AbstractPipe(metaclass=ABCMeta):
         """
         raise NotImplementedError()
 
-    def read_from_pipe(self, timeout_secs: Optional[float] = None) -> Set[str]:
+    def read_from_pipe(self, timeout_secs: Optional[float] = None) -> List[str]:
         """
         Common interface for the custom _reader implementations
 
         :param timeout_secs: (Optional[float]) Timeout for the function, by default it fallbacks to self.TIMEOUT_SECS
-        :return: Set of messages or {self.NO_RESPONSE_MESSAGE} (if no messages received)
-        :rtype: Set[str]
+        :return: List of messages or {self.NO_RESPONSE_MESSAGE} (if no messages received)
+        :rtype: List[str]
         """
         if timeout_secs is None:
             timeout_secs = self.TIMEOUT_SECS
@@ -228,15 +227,14 @@ class AbstractPipe(metaclass=ABCMeta):
         try:
             res = reader.result(timeout=timeout_secs)
             if res:
-                out = set([r for r in res.split(self.MESSAGE_TO_IGNORE) if r])
+                out = [r for r in res.split(self.MESSAGE_TO_IGNORE) if r]
                 if out:
-                    log.debug("Read message: %r", out)
                     return out
         except concurrent.futures._base.TimeoutError:
             # hacky way to kill the file-opening loop
             self.send_to_pipe(self.MESSAGE_TO_IGNORE)
 
-        return {self.NO_RESPONSE_MESSAGE}
+        return [self.NO_RESPONSE_MESSAGE]
 
     def send_to_pipe(self, message: str, timeout_secs: Optional[float] = None) -> bool:
         """
@@ -256,7 +254,6 @@ class AbstractPipe(metaclass=ABCMeta):
 
         try:
             if sender.result(timeout=timeout_secs):
-                log.debug("sent successfully: %r", message)
                 return True
         except concurrent.futures._base.TimeoutError:
             log.warning("Couldn't send: %r", message)

test/test_util_pipe.py

@@ -24,12 +24,11 @@ from random import randint
 
 from test.picardtestcase import PicardTestCase
 
-from picard import log
 from picard.util import pipe
 
 
 def pipe_listener(pipe_handler):
-    IGNORED_OUTPUT = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE, ""}
+    IGNORED_OUTPUT = {pipe.Pipe.MESSAGE_TO_IGNORE, pipe.Pipe.NO_RESPONSE_MESSAGE}
     received = ""
 
     while not received:
@@ -38,7 +37,6 @@ def pipe_listener(pipe_handler):
                 received = message
                 break
 
-    log.debug("returning: %r", received)
     return received
 
 
@@ -54,7 +52,7 @@ def pipe_writer(pipe_handler, to_send):
 
 class TestPipe(PicardTestCase):
     # we don't need any strong and secure random numbers, just anything that is different on each run
-    NAME = str(randint(0, 99999999))    # nosec
+    NAME = str(randint(0, 99999999))  # nosec
     VERSION = python_version()
 
     def test_invalid_args(self):
@@ -80,23 +78,19 @@ class TestPipe(PicardTestCase):
         __pool = concurrent.futures.ThreadPoolExecutor()
         for count in range(100):
             for message in to_send:
-                for iteration in range(20):
-                    log.debug("No. %d attempt to send: %r", iteration+1, message)
-                    plistener = __pool.submit(pipe_listener, pipe_listener_handler)
-                    pwriter = __pool.submit(pipe_writer, pipe_writer_handler, message)
-                    to_break = False
-                    try:
-                        self.assertEqual(plistener.result(timeout=6.5), message,
-                                        "Data is sent and read correctly")
-                        log.debug("Sent correctly!")
-                        to_break = True
-                    except concurrent.futures._base.TimeoutError:
-                        pipe_writer_handler.send_to_pipe(pipe_writer_handler.MESSAGE_TO_IGNORE)
+                plistener = __pool.submit(pipe_listener, pipe_listener_handler)
+                pwriter = __pool.submit(pipe_writer, pipe_writer_handler, message)
+                res = []
 
-                    try:
-                        pwriter.result(timeout=0.01)
-                    except concurrent.futures._base.TimeoutError:
-                        pipe_listener_handler.read_from_pipe()
+                # handle the write/read processes
+                try:
+                    res = plistener.result(timeout=6.5)
+                except concurrent.futures._base.TimeoutError:
+                    pipe_writer_handler.send_to_pipe(pipe_writer_handler.MESSAGE_TO_IGNORE)
+                try:
+                    pwriter.result(timeout=0.01)
+                except concurrent.futures._base.TimeoutError:
+                    pipe_listener_handler.read_from_pipe()
 
-                    if to_break:
-                        break
+                self.assertEqual(res, message,
+                                 "Data is sent and read correctly")