From 3741101d70e1fce1bddaaa879233f5b1b65b452d Mon Sep 17 00:00:00 2001
From: Kay0u <pierre@kayou.io>
Date: Fri, 17 Sep 2021 16:17:16 +0200
Subject: [PATCH] Fix a potential bug in call_async_output, update tests

---
 moulinette/utils/process.py |  9 +++-
 test/test_process.py        | 82 ++++++++++++++++++++++++++++++++-----
 2 files changed, 79 insertions(+), 12 deletions(-)

diff --git a/moulinette/utils/process.py b/moulinette/utils/process.py
index 41c43633..e4f2cf31 100644
--- a/moulinette/utils/process.py
+++ b/moulinette/utils/process.py
@@ -88,6 +88,13 @@ def call_async_output(args, callback, **kwargs):
                     break
 
                 callback(message)
+        while True:
+            try:
+                callback, message = log_queue.get_nowait()
+            except queue.Empty:
+                break
+
+            callback(message)
     finally:
         kwargs["stdout"].close()
         kwargs["stderr"].close()
@@ -120,7 +127,7 @@ class LogPipe(threading.Thread):
 
     def run(self):
         """Run the thread, logging everything."""
-        for line in iter(self.pipeReader.readline, ""):
+        for line in iter(self.pipeReader.readline, b""):
             self.queue.put((self.log_callback, line.decode("utf-8").strip("\n")))
 
         self.pipeReader.close()
diff --git a/test/test_process.py b/test/test_process.py
index b3af92ff..2484101a 100644
--- a/test/test_process.py
+++ b/test/test_process.py
@@ -1,6 +1,7 @@
 import os
 from subprocess import CalledProcessError
 
+import mock
 import pytest
 
 from moulinette.utils.process import run_commands
@@ -65,39 +66,71 @@ def test_run_shell_kwargs():
 
 
 def test_call_async_output(test_file):
+
+    mock_callback_stdout = mock.Mock()
+    mock_callback_stderr = mock.Mock()
+
     def stdout_callback(a):
-        assert a == "foo" or a == "bar"
+        mock_callback_stdout(a)
 
     def stderr_callback(a):
-        assert False  # we shouldn't reach this line
+        mock_callback_stderr(a)
 
     callbacks = (lambda l: stdout_callback(l), lambda l: stderr_callback(l))
 
     call_async_output(["cat", str(test_file)], callbacks)
 
+    calls = [mock.call("foo"), mock.call("bar")]
+    mock_callback_stdout.assert_has_calls(calls)
+    mock_callback_stderr.assert_not_called()
+
+    mock_callback_stdout.reset_mock()
+    mock_callback_stderr.reset_mock()
+
     with pytest.raises(TypeError):
         call_async_output(["cat", str(test_file)], 1)
 
-    def callbackA(a):
-        assert a == "foo" or a == "bar"
+    mock_callback_stdout.assert_not_called()
+    mock_callback_stderr.assert_not_called()
 
-    def callbackB(a):
-        assert "cat: doesntexists" in a
+    mock_callback_stdout.reset_mock()
+    mock_callback_stderr.reset_mock()
 
-    callback = (callbackA, callbackB)
+    def callback_stdout(a):
+        mock_callback_stdout(a)
+
+    def callback_stderr(a):
+        mock_callback_stderr(a)
+
+    callback = (callback_stdout, callback_stderr)
     call_async_output(["cat", str(test_file)], callback)
-    call_async_output(["cat", "doesntexists"], callback)
+    calls = [mock.call("foo"), mock.call("bar")]
+    mock_callback_stdout.assert_has_calls(calls)
+    mock_callback_stderr.assert_not_called()
+    mock_callback_stdout.reset_mock()
+    mock_callback_stderr.reset_mock()
+
+    env_var = {"LANG": "C"}
+    call_async_output(["cat", "doesntexists"], callback, env=env_var)
+    calls = [mock.call("cat: doesntexists: No such file or directory")]
+    mock_callback_stdout.assert_not_called()
+    mock_callback_stderr.assert_has_calls(calls)
 
 
 def test_call_async_output_kwargs(test_file, mocker):
+
+    mock_callback_stdout = mock.Mock()
+    mock_callback_stdinfo = mock.Mock()
+    mock_callback_stderr = mock.Mock()
+
     def stdinfo_callback(a):
-        assert False  # we shouldn't reach this line
+        mock_callback_stdinfo(a)
 
     def stdout_callback(a):
-        assert a == "foo" or a == "bar"
+        mock_callback_stdout(a)
 
     def stderr_callback(a):
-        assert False  # we shouldn't reach this line
+        mock_callback_stderr(a)
 
     callbacks = (
         lambda l: stdout_callback(l),
@@ -107,16 +140,43 @@ def test_call_async_output_kwargs(test_file, mocker):
 
     with pytest.raises(ValueError):
         call_async_output(["cat", str(test_file)], callbacks, stdout=None)
+    mock_callback_stdout.assert_not_called()
+    mock_callback_stdinfo.assert_not_called()
+    mock_callback_stderr.assert_not_called()
+
+    mock_callback_stdout.reset_mock()
+    mock_callback_stdinfo.reset_mock()
+    mock_callback_stderr.reset_mock()
+
     with pytest.raises(ValueError):
         call_async_output(["cat", str(test_file)], callbacks, stderr=None)
+    mock_callback_stdout.assert_not_called()
+    mock_callback_stdinfo.assert_not_called()
+    mock_callback_stderr.assert_not_called()
+
+    mock_callback_stdout.reset_mock()
+    mock_callback_stdinfo.reset_mock()
+    mock_callback_stderr.reset_mock()
+
     with pytest.raises(TypeError):
         call_async_output(["cat", str(test_file)], callbacks, stdinfo=None)
+    mock_callback_stdout.assert_not_called()
+    mock_callback_stdinfo.assert_not_called()
+    mock_callback_stderr.assert_not_called()
+
+    mock_callback_stdout.reset_mock()
+    mock_callback_stdinfo.reset_mock()
+    mock_callback_stderr.reset_mock()
 
     dirname = os.path.dirname(str(test_file))
     os.mkdir(os.path.join(dirname, "testcwd"))
     call_async_output(
         ["cat", str(test_file)], callbacks, cwd=os.path.join(dirname, "testcwd")
     )
+    calls = [mock.call("foo"), mock.call("bar")]
+    mock_callback_stdout.assert_has_calls(calls)
+    mock_callback_stdinfo.assert_not_called()
+    mock_callback_stderr.assert_not_called()
 
 
 def test_check_output(test_file):