WIP: Flexible instantiation #64
Conversation
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 101 121 +20
=====================================
+ Hits 101 121 +20
Continue to review full report at Codecov.
|
src/stdio_mgr/stdio_mgr.py
Outdated
| @@ -95,62 +101,32 @@ def getvalue(self): | |||
| return self._stream.getvalue().decode(self.encoding) | |||
|
|
|||
|
|
|||
| @attr.s(slots=False) | |||
| class TeeStdin(TextIOWrapper): | |||
| class Tee(TextIOWrapper): | |||
There was a problem hiding this comment.
@jayvdb I would want (at least) two implementations of teeing, ultimately, in order to accurately mimic the CLI experience: tee-on-read for stdin and tee-on-write for stderr. That way, everything that would get pushed to a real user's console, across in/out/err, would get stored in out in the same order that that user would see it in "real life".
I had originally only implemented TeeStdin b/c I didn't have specific need for teeing stderr in my initial use-cases for it.
There was a problem hiding this comment.
Sounds good, but beyond the scope of this. Raised #65 about a similar feature.
However it does suggest that Tee should be _Tee, and other parts of this WIP probably should be private (even if they are desirable in coala-utils; that shouldnt be a consideration here)
There was a problem hiding this comment.
<nod>, agreed, out of scope here.
But yes, exactly, if you think it's not a crazy feature to have, then it would be good to keep that additional teeing eventuality in mind during separation of the mocking and teeing of stdin.
| @@ -23,6 +23,6 @@ jobs: | |||
| script: | |||
| - python --version | |||
| - pip list | |||
| - pytest --cov=src -p no:warnings | |||
| - pytest --cov=src -p no:warnings -s | |||
There was a problem hiding this comment.
this -s can be removed, because of the commented out assertion in StdioTuple.__new__.
the alternatives include:
- not creating
_INITIAL_SYS_STREAMSand re-add the assertion, which requires the caller to disable any incompatible wrappers before using stdio-mgr - detecting wrappers like pytest, and finding where they hid the real sys streams.
- detecting incompatible wrappers in
__new__but only emit a warning, and let the user encounter real breakages somewhere else, possibly quite confusing. (this appears to be theioapproach, as we saw withbuffer=StringIO()not breaking during instantiation) - do no checking; user beware.
There was a problem hiding this comment.
Disabling pytest capturing from within the test suite is given a small note at the bottom of https://docs.pytest.org/en/latest/capture.html .
There was also talk of it being possible using markers pytest-dev/pytest#1599 (comment)
And it seems class scope disabling can be achieved using the pytest plugin manager pytest-dev/pytest#1599 (comment)
(If we are going to expect users to uncapture pytest, https://docs.pytest.org/en/latest/capture.html needs a rewrite to assist them.)
|
|
||
| Upon exiting the context, the original stream objects are restored | ||
| within :mod:`sys`, and the temporary streams are closed. | ||
| class FakeStdioTuple(StdioTuple, _MultiCloseContextManager): |
There was a problem hiding this comment.
the _MultiCloseContextManager here has no effect. It is mostly thought experiment atm, and should be removed unless there is some benefit in keeping it.
There was a problem hiding this comment.
Ah, the important part is that dual-inheritance is possible, so others could do it if they wanted to use _MultiCloseContextManager as a mixin.
| @@ -380,3 +427,4 @@ def __exit__(self, exc_type, exc_value, traceback): | |||
|
|
|||
|
|
|||
| stdio_mgr = StdioManager | |||
| _INITIAL_SYS_STREAMS = StdioTuple([sys.stdin, sys.stdout, sys.stderr]) | |||
There was a problem hiding this comment.
These should be sys.__stdin__, sys.__stdout__ & sys.__stderr__, which should be untouched and we can bypass/undo any wrapping done to sys.std*
There was a problem hiding this comment.
Ah, I hadn't thought about using those, but yeah -- definitely helpful.
It's not a carte-blanche thing, though. We can reference back to those in some cases -- but in general, there may be situations where we might want to restore to the prior sys.stdfoo, instead of the canonical sys.__stdfoo__, to avoid munging a surrounding stdio mock/wrap.
I think testing sys.stdfoo is sys.__stdfoo__ should be an unambiguous sign that we're working inside someone else's mocking/wrapping environment, though. Oh, well, not completely -- if something rummages about in the innards of sys.stdfoo instead of replacing the object references, then is comparison might still give True.
That actually may be an argument against trying to directly reach within sys.stdfoo to work with the internal buffer... if .stdfoo is .__stdfoo__, then we'd also be tinkering with the "canonical backup".
There was a problem hiding this comment.
... if
.stdfoois.__stdfoo__, then we'd also be tinkering with the "canonical backup".
Yes, this is the dangerous territory that #78 is going in -- IMO the 'dont touch' is cause for concern, lots of it, especially in multi-threading as the tinkering being done isnt atomic, and locking is going to be hard. #78 may need to be opt-in for a while.
Adds
StdioManagerinstantiation argstreams