Support of pytest capsys

[shepherdjay]

First off great plugin, we were writing a bunch of our own logic and boiler plate for our nagios style tests and I am considering moving over to this plugin to avoid copy paste errors.

The one issue I've had is converting our tests, our current modules use pytest's capsys fixture. To compare the expected output of a check with test data. Sample from one of these tests below.

[.....]
mock_get.return_value.text = test_data
expected_msg = "OK :load average is 2% and max is 10% | " + \
               "load_average=2;75;85;100; load_max=10;75;85;100;\n"
with pytest.raises(SystemExit) as e:
    cpdmain(test_args)
assert capsys.readouterr().out == expected_msg

What's interesting is none of the pytest fixtures for capturing stdout seem to capture nagiosplugins output. More interesting is that the output does appear in the test results summary like this

test_load.py::test_main FAILED [ 56%]LOAD OK - avg is 2 | avg=2;75;85;0;100 max=10;75;85;0;100

As far as I can tell the reason appears to be related to the Runtime class execute method where it prints with the file keyword argument set to self.stdout which itself is just sys.stdout

Since writing to stdout is the default behavior of print I'm curious if it being explicitly specified is to solve a specific problem or if this can be removed to allow test frameworks like pytest to capture the output?

[mpounsett]

I'm not the original author, just the current maintainer, so I can't speak with authority about historical reasons for using Runtime.stdout, but I imagine it's to allow anything using the library to be able to redirect output easily.

That said, I can't see how an explicit print to sys.stdout is any different from an implicit print to stdout by default. I don't think removing it is going to fix your problem.

Can you provide some simple-as-possible example code that demonstrates the problem?

[shepherdjay]

The simplest example is to use the Hello World check from the documentation with this pytest code. In this case, the exact code from the tutorial is placed in a file called example_issue24.py

from example_issue24 import main as libmain
import pytest

def test_helloworld(capsys):
    with pytest.raises(SystemExit):
        libmain()
    assert capsys.readouterr().out == 'WORLD OK\n'

The test will fail with the assert not being matched. Specifically with the below

 AssertionError: assert '' == 'WORLD OK\n'

However, if you uninstall nagiosplugin and install this forked version where the only change is to remove the file parameter from Runtime.execute the test passes.

pip install git+https://github.com/shepherdjay/nagiosplugin.git@617a04ff378c1c8045213d264995d83ec4e9f5ad

Its possible that there is a small issue with the fixture I can try to raise an issue with the pytest team as well.

[shepherdjay]

Pytest issue raised https://github.com/pytest-dev/pytest/issues/6676

[RonnyPfannschmidt]

the basic issue is that pytest output redirection has to replace sys.stdout/stderr to do "isolated" redirection

taking a alias simply negates that as far as i can tell, the TestRuntime also replaces the stdout with a stringio, i believe its fine to do the same from pytest based tests as well

[shepherdjay]

@mpounsett A possible solution that came out of the pytest issue is to set Runtime.stdout to None - When passed to the file parameter it uses the default std.out that pytest is redirecting while still allowing it to be overridden for anyone doing so.

I tested this on my issue24 branch I have and all the tox tests passed. https://travis-ci.com/shepherdjay/nagiosplugin/builds/147585561

I can open a pull request if you like; my current branch added a .travis.yml for testing but I can remove that or include it.

[mpounsett]

I did some digging into the origin of using sys.stdout there. It was first introduced in 202a19f1f5ef34479335f75392ee1a2e4d5b0b96 where a basic print() was replace with a sys.stdout.write() call in order to fix something cross-platform. Later it changed to print(..., file=sys.stdout).

I think I'd like to both understand why print(..., file=sys.stdout) behaves any differently from print(..., file=None) when the latter should still print to "the current sys.stdout" according to the Python docs. I'd also like to make sure that tests aren't broken on Windows.

[RonnyPfannschmidt]

@mpounsett there is a time lag difference and aliasing

#untested
my_alias = sys.stdout
with redirect_stdout(something_else):
   print(..., file=my_alias) # goes to the original stdout
   print(..., file=None) # goes to the current sys.stdout aka something_else

[shepherdjay]

@mpounsett as far as the windows tests - I got this semi working in Travis using python 3.7 as well but both on my Windows 10 machine and Travis several tests fail for permissions issues. These tests fail on the most recent release as well as the build that removes the alias. Not sure if this is helpful or not for you but here are the builds.

Issue24 - https://travis-ci.com/shepherdjay/nagiosplugin/builds/147641097 Current Master - https://travis-ci.com/shepherdjay/nagiosplugin/builds/147640379

In my config I did specify windows was allowed to fail given that it is still experimental in Travis.

[mpounsett]

Sorry, @shepherdjay .. I seem to have let this drop of my radar.

If you want to send that PR (without the travis config) I'll apply it.

Adding the travis stuff would also be great, but I think that should be a separate PR/issue.