bskinn / stdio-mgr

Context manager for mocking/wrapping stdin/stdout/stderr
MIT License
14 stars 4 forks source link

Testing against console libraries #71

Open jayvdb opened 5 years ago

jayvdb commented 5 years ago

colorama < 0.4 is incompatible, as its StreamWrapper only gained context manager features in 0.4 https://github.com/tartley/colorama/commit/2f4b564a6586709db2fed20e5d1f5856b1a482d2 and it pushes instances of its StreamWrapper into sys.std*

We could potentially work around this by detecting and patching its objects, but it probably isnt worth the effort. coala was forced to use < 0.4 because of radon, but they have bumped their min dependency, so we will too. Not many projects will be still wanting colorama < 0.4.

So this is probably best addressed with documentation of known incompatibilities.

However, it would be worth checking out other console libraries, and creating tests for each.

Stdlib:

Other:

jayvdb commented 5 years ago

Ah , the context manager problem exists with at least pytest 3-5.0.1 also, but only becomes a problem when we do not replace their streams - i.e. https://github.com/bskinn/stdio-mgr/issues/4 - and assume the sys handles are context managers.

pytest 3

/usr/local/lib/python3.7/site-packages/stdio_mgr/stdio_mgr.py:410: in __enter__
    super().__enter__()
/usr/local/lib/python3.7/site-packages/stdio_mgr/stdio_mgr.py:302: in __enter__
    all(map(stack.enter_context, self))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <contextlib.ExitStack object at 0x7fa2f73fed10>
cm = <_pytest.capture.EncodedFile object at 0x7fa2f7681e90>

    def enter_context(self, cm):
        """Enters the supplied context manager.

            If successful, also pushes its __exit__ method as a callback and
            returns the result of the __enter__ method.
            """
        # We look up the special methods on the type to match the with
        # statement.
        _cm_type = type(cm)
>       _exit = _cm_type.__exit__
E       AttributeError: type object 'EncodedFile' has no attribute '__exit__'

/usr/local/lib/python3.7/contextlib.py:426: AttributeError

pytest 5

src/stdio_mgr/stdio_mgr.py:410: in __enter__
    super().__enter__()
src/stdio_mgr/stdio_mgr.py:302: in __enter__
    all(map(stack.enter_context, self))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <contextlib.ExitStack object at 0x7f532cfd1390>, cm = <_pytest.capture.DontReadFromInput object at 0x7f5339d2e908>

    def enter_context(self, cm):
        """Enters the supplied context manager.

        If successful, also pushes its __exit__ method as a callback and
        returns the result of the __enter__ method.
        """
        # We look up the special methods on the type to match the with
        # statement.
        _cm_type = type(cm)
>       _exit = _cm_type.__exit__
E       AttributeError: type object 'DontReadFromInput' has no attribute '__exit__'

/usr/lib64/python3.7/contextlib.py:425: AttributeError

That is happening because the stdin wrapper is deferring attributes to the buffer https://github.com/pytest-dev/pytest/blob/2c402f4bd9cff2c6faeccb86a97364e1fa122a16/src/_pytest/capture.py#L442

bskinn commented 5 years ago

So -- the solution for the pytest problem is to exclude any unmocked sys.std... from the ExitStack?

jayvdb commented 5 years ago

That, or processing each item in the tuple and catching AttributeError, which means it isnt a context manager, and concluding that it also isnt closable.

Tests against a reasonable sample of console capturing/wrapping libraries is needed to ensure ongoing compatibility, and generic enough that it is easy to use the tests to debug compatibility problems with new libraries people want to use stdio-mgr with.

bskinn commented 5 years ago

Should we curate somewhere a list of libraries to add compat tests for?

tqdm comes to mind as possibly needing compat test.

bskinn commented 5 years ago

Seems like colorama is going to be kind of annoying to add as a localized test, what with how it's an import-once-to-wrap-sys.stdxyz thing

...unless pytest will restore the sys.stdxyz to defaults between each test function?

I guess one way to handle it would be to coerce a colorama test to always be the last one executed, say with pytest-ordering?

jayvdb commented 5 years ago

We would need wrap the import in a context manager / fixture that only restores the three streams afterwards.

jayvdb commented 5 years ago

Detecting when the sys variables have been modified, and reporting on what has happened, would help devs see that new streams were opened and need to be closed (and eradicated) before they exit the context handler. As you mentioned, we also could avoid closing streams that have unexpectedly appeared, but that only hides the fact those new streams havent been cleaned up.

I encountered https://github.com/bskinn/stdio-mgr/issues/73 when playing with coala, because the existing testing approach there was hiding the problem that logging wasnt being cleaned up between each 'cli' test.

This touches on the same problem area as https://github.com/bskinn/stdio-mgr/issues/66 .

bskinn commented 5 years ago

Detecting when the sys variables have been modified, and reporting on what has happened, would help devs see that new streams were opened and need to be closed (and eradicated) before they exit the context handler.

Would it make sense to implement some sort of 'active monitoring mode', perhaps tied into logging (cf #43), that somehow could sniff out whenever anything modified the sys.stdx?

It would probably require some major hackery around the sys attribute access system, though, in order to track with precision when and from what source modifications are made? I don't have good enough a mental model of the logic flow to know if it would be enough, say, to just have machinery in the __enter__ and __exit__ of StdioManager (or whatever related class(es)).

jayvdb commented 5 years ago

https://travis-ci.org/bskinn/stdio-mgr/jobs/581947934 shows the errors wrt stdlib doctest with re-using their existing fake streams.

I created a list at the top of the issue, and also added click

jayvdb commented 5 years ago

Added readline/pyreadline/anyreadline, as I have seen wonderfully complicated bugs caused by this.

jayvdb commented 5 years ago

And a sure way to break something: https://github.com/ipopov13/py3console

And https://www.crystax.net/ is another python on Android.

bskinn commented 4 years ago

better-exceptions seems like it would potentially wreak merry havoc at multiple points. Added to main issue description.