Repository: theacodes/nox
Pull request #180 (2019-04-13 15:03:41)

Allow specifying `stdout` to Popen

s0undt3ch: The reason behind this change is that on a particular session, we need access to the output, even if the command failed, for CI reporting reasons.
theacodes: Can you show me an example of when you'd use this and why the existing mechanisms aren't giving your the output you expect?
mostealth: When running in CI you might want to save pylint output to file (to be later parsed). pylint does not support outputting to file hence the only solution I found was to monkey patch subprocess.Popen so that I inject my own stdout/stderr file before calling session.run
s0undt3ch: What @mostealth said, that's our exact same scenario and why I created this PR.

This solution in use on our repo can be seen [here](https://github.com/saltstack/salt/blob/2018.3/noxfile.py#L819-L888)
theacodes: That seems like a great reason. I had a comment about covering some edge cases, but I'm happy to merge this once we get that sorted. :)
s0undt3ch: Rebased and added two new tests based on the comments provided.
theacodes: Okay, I made `silent` and `stdout` mutually exclusive since `silent` doesn't have an effect if `stdout` is passed.

Does this still look good to you, @s0undt3ch? Also, it should be technically possible to override stderr now.  Do you mind adding a test for that?
s0undt3ch: Added tests for custom `stderr`

Patch:
diff --git a/docs/index.rst b/docs/index.rst
index 49ab6bc..d98f92b 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -50,17 +50,17 @@ Projects that use Nox
 
 Nox is lucky to have several wonderful projects that use it and provide feedback and contributions.
 
-- `Urllib3 <https://github.com/urllib3/urllib3>`__
 - `Bezier <https://github.com/dhermes/bezier>`__
-- `Zazo <https://github.com/pradyunsg/zazo>`__
-- `packaging.python.org <https://github.com/pypa/python-packaging-user-guide/>`__
-- `google-cloud-python <https://github.com/googlecloudplatform/google-cloud-python>`__
 - `gapic-generator-python <https://github.com/googleapis/gapic-generator-python>`__
-- `google-resumable-media-python <https://github.com/GoogleCloudPlatform/google-resumable-media-python>`__
 - `Google Assistant SDK <https://github.com/googlesamples/assistant-sdk-python>`__
+- `google-cloud-python <https://github.com/googlecloudplatform/google-cloud-python>`__
+- `google-resumable-media-python <https://github.com/GoogleCloudPlatform/google-resumable-media-python>`__
 - `OpenCensus Python <https://github.com/census-instrumentation/opencensus-python>`__
+- `packaging.python.org <https://github.com/pypa/python-packaging-user-guide/>`__
+- `Salt <https://github.com/saltstack/salt>`__
 - `Subpar <https://github.com/google/subpar>`__
-
+- `Urllib3 <https://github.com/urllib3/urllib3>`__
+- `Zazo <https://github.com/pradyunsg/zazo>`__
 
 Other useful projects
 ---------------------
diff --git a/nox/command.py b/nox/command.py
index 06ef1bf..68b2353 100644
--- a/nox/command.py
+++ b/nox/command.py
@@ -69,7 +69,8 @@ def run(
     path=None,
     success_codes=None,
     log=True,
-    external=False
+    external=False,
+    **popen_kws
 ):
     """Run a command-line program."""
 
@@ -105,7 +106,9 @@ def run(
     env = _clean_env(env)
 
     try:
-        return_code, output = popen([cmd_path] + list(args), silent=silent, env=env)
+        return_code, output = popen(
+            [cmd_path] + list(args), silent=silent, env=env, **popen_kws
+        )
 
         if return_code not in success_codes:
             logger.error(
diff --git a/nox/popen.py b/nox/popen.py
index 82a0776..70db6f6 100644
--- a/nox/popen.py
+++ b/nox/popen.py
@@ -16,13 +16,16 @@ import subprocess
 import sys
 
 
-def popen(args, env=None, silent=False):
-    stdout = None
+def popen(args, env=None, silent=False, stdout=None, stderr=subprocess.STDOUT):
+    if silent and stdout is not None:
+        raise ValueError(
+            "Can not specify silent and stdout; passing a custom stdout always silences the commands output in Nox's log."
+        )
 
     if silent:
         stdout = subprocess.PIPE
 
-    proc = subprocess.Popen(args, env=env, stdout=stdout, stderr=subprocess.STDOUT)
+    proc = subprocess.Popen(args, env=env, stdout=stdout, stderr=stderr)
 
     try:
         out, err = proc.communicate()


Test patch:
diff --git a/tests/test_command.py b/tests/test_command.py
index 0f8f9ff..39d5139 100644
--- a/tests/test_command.py
+++ b/tests/test_command.py
@@ -148,3 +148,95 @@ def test_interrupt():
     with mock.patch("subprocess.Popen", return_value=mock_proc):
         with pytest.raises(KeyboardInterrupt):
             nox.command.run([PYTHON, "-c" "123"])
+
+
+def test_custom_stdout(capsys, tmpdir):
+    with open(str(tmpdir / "out.txt"), "w+b") as stdout:
+        nox.command.run(
+            [
+                PYTHON,
+                "-c",
+                'import sys; sys.stdout.write("out");'
+                'sys.stderr.write("err"); sys.exit(0)',
+            ],
+            stdout=stdout,
+        )
+        out, err = capsys.readouterr()
+        assert not out
+        assert "out" not in err
+        assert "err" not in err
+        stdout.seek(0)
+        tempfile_contents = stdout.read().decode("utf-8")
+        assert "out" in tempfile_contents
+        assert "err" in tempfile_contents
+
+
+def test_custom_stdout_silent_flag(capsys, tmpdir):
+    with open(str(tmpdir / "out.txt"), "w+b") as stdout:
+        with pytest.raises(ValueError, match="silent"):
+            nox.command.run([PYTHON, "-c", 'print("hi")'], stdout=stdout, silent=True)
+
+
+def test_custom_stdout_failed_command(capsys, tmpdir):
+    with open(str(tmpdir / "out.txt"), "w+b") as stdout:
+        with pytest.raises(nox.command.CommandFailed):
+            nox.command.run(
+                [
+                    PYTHON,
+                    "-c",
+                    'import sys; sys.stdout.write("out");'
+                    'sys.stderr.write("err"); sys.exit(1)',
+                ],
+                stdout=stdout,
+            )
+        out, err = capsys.readouterr()
+        assert not out
+        assert "out" not in err
+        assert "err" not in err
+        stdout.seek(0)
+        tempfile_contents = stdout.read().decode("utf-8")
+        assert "out" in tempfile_contents
+        assert "err" in tempfile_contents
+
+
+def test_custom_stderr(capsys, tmpdir):
+    with open(str(tmpdir / "err.txt"), "w+b") as stderr:
+        nox.command.run(
+            [
+                PYTHON,
+                "-c",
+                'import sys; sys.stdout.write("out");'
+                'sys.stderr.write("err"); sys.exit(0)',
+            ],
+            stderr=stderr,
+        )
+        out, err = capsys.readouterr()
+        assert not err
+        assert "out" not in out
+        assert "err" not in out
+        stderr.seek(0)
+        tempfile_contents = stderr.read().decode("utf-8")
+        assert "out" not in tempfile_contents
+        assert "err" in tempfile_contents
+
+
+def test_custom_stderr_failed_command(capsys, tmpdir):
+    with open(str(tmpdir / "out.txt"), "w+b") as stderr:
+        with pytest.raises(nox.command.CommandFailed):
+            nox.command.run(
+                [
+                    PYTHON,
+                    "-c",
+                    'import sys; sys.stdout.write("out");'
+                    'sys.stderr.write("err"); sys.exit(1)',
+                ],
+                stderr=stderr,
+            )
+        out, err = capsys.readouterr()
+        assert not err
+        assert "out" not in out
+        assert "err" not in out
+        stderr.seek(0)
+        tempfile_contents = stderr.read().decode("utf-8")
+        assert "out" not in tempfile_contents
+        assert "err" in tempfile_contents